Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is it possible to use exceptions istead of assertions? #1056

Closed
lesf0 opened this issue Apr 16, 2018 · 4 comments
Closed

Is it possible to use exceptions istead of assertions? #1056

lesf0 opened this issue Apr 16, 2018 · 4 comments
Labels
solution: wontfix the issue will not be fixed (either it is impossible or deemed out of scope)

Comments

@lesf0
Copy link

lesf0 commented Apr 16, 2018

Let's consider the following code

#include "json.hpp"
#include <iostream>

int main()
{
	const auto data = R"({"foo":42})"_json;
	
	try {
		std::cout << data["bar"] << std::endl;
	} catch(...) {
		std::cerr << "json error" << std::endl;
	}
}

I expect this code to output json error, but instead it shuts down with the following error:

json.hpp:12936: const value_type& nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::operator[](T*) const [with T = const char; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; JSONSerializer = nlohmann::adl_serializer; nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::const_reference = const nlohmann::basic_json<>&; nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::value_type = nlohmann::basic_json<>]: Assertion `m_value.object->find(key) != m_value.object->end()' failed.

I know I can disable assertion by NDEBUG flag, but then it prints out null in my example (and I'm almost sure this is an UB) instead of expected exception.

I've checked out json.hpp code and found 98 calls to assert, so I don't think above is the only example leading to application shutdown which can't be handled. In my real app I deal with user provided data, so it's quite bad for me. Is it possible to just replace those assertions with exception throws? Or, if assertions are necessary for some cases, is it possible to have something like template parameter specifying assertion handler, which would be defaulted to assert?

I can create a PR by myself, but first I want to know which option is preferred.

@nlohmann
Copy link
Owner

You can use the at function just like for std::map.

@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Apr 16, 2018
@lesf0
Copy link
Author

lesf0 commented Apr 16, 2018

I know I can, but unlike std::map this library allows const [] operator compilation, so I think it should be either disallowed or properly asserted to avoid possible problems.

@nlohmann
Copy link
Owner

This has been discussed several times. The operator[] will stay as is.

@nlohmann nlohmann added solution: wontfix the issue will not be fixed (either it is impossible or deemed out of scope) and removed solution: proposed fix a fix for the issue has been proposed and waits for confirmation labels Apr 16, 2018
@nlohmann
Copy link
Owner

On assertions, please note the first entry in the README: https://github.com/nlohmann/json#notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solution: wontfix the issue will not be fixed (either it is impossible or deemed out of scope)
Projects
None yet
Development

No branches or pull requests

2 participants