Closed
Description
I am running into an issue where if a from_json function I wrote does a conversion that uses another from_json function I wrote, whether or not I can catch exceptions from nlohmann::json depends on the order of the from_json functions. Repro code below:
#include <exception>
#include <fstream>
#include <iostream>
#include "json.hpp"
struct A {
int value;
};
struct B {
A a;
};
void from_json(const nlohmann::json& j, B& b) {
b.a = j.at("test").get<A>();
}
// If this is moved before the from_json for B, I can catch the exception below
void from_json(const nlohmann::json& j, A& a) {
a.value = j.at("value").get<int>();
}
int main() {
try {
std::ifstream stream("test.json");
nlohmann::json j;
stream >> j;
B b = j;
} catch (const std::exception& e) {
std::cerr << "caught exception: " << e.what() << std::endl;
}
return 0;
}
My test.json file is below:
{
"test": {
"value": "hello"
}
}
In the above example, the output is:
libc++abi.dylib: terminating with uncaught exception of type std::domain_error: type must be number, but is string
Abort trap: 6
If I switch the order of the from_json functions, the output is as expected:
caught exception: type must be number, but is string
I am pretty new to C++11 and C++ exceptions, so sorry if the reason for this is obvious.
My platform/compiler is OSX 10.11.6 with AppleClang 8.0.0.8000042 and I tested this using json version 2.1.1.
Metadata
Metadata
Assignees
Projects
Milestone
Relationships
Development
No branches or pull requests
Activity
nlohmann commentedon Apr 23, 2017
I can confirm you observation, but I also have no idea. Maybe a language lawyer like @gregmarr or @theodelrieu knows more about this.
theodelrieu commentedon Apr 26, 2017
I'm far away from being a language lawyer, I will take a look, this looks really weird.
EDIT: This is related to the
noexcept
operator, I removed everynoexcept(noexcept(some_method))
from the library code, and it works.I don't have a lot of times these days, but I'll try to investigate more. Meanwhile, if someone had a problem related to
noexcept
and function declaration order, that would help a lot!theodelrieu commentedon Apr 26, 2017
Well well well, there is something fishy, looks like a dark corner of C++.
I have no idea why, but it seems that during the evaluation of the first
from_json
method's body, the compiler instantiates a lot of templates, one being thefrom_json_fn::operator()()
.This method has a
noexcept(noexcept(to_json(bla, bla)))
construct. The thing is, there is no methodvoid to_json(json const& , B&)
defined, it is defined later in the translation unit. However, it stills compiles, no idea why.I printed the value of the
noexcept(noexcept())
clause in the definition ofnlohmann::detail::from_json_fn::operator()()
, and the output is ....true
.That's weird, and if you add the prototype
void from_json(json const&, A&);
at the top of your test file, the program will not crash.The quick way of fixing this, is removing the
noexcept(noexcept())
everywhere.I will try to reproduce the problem with less code, and see if someone can explain what really happens.
theodelrieu commentedon May 9, 2017
I posted a question on SO last week.
The answer is not really clear, except if you're fluent in standardese (which I'm not).
I'll try to explain what happens (I may have some details wrong, don't hit me)
When the compiler sees the line
b.a = j.at("test").get<A>();
, it chooses the fallback overload ofnlohmann::detail::from_json_fn::call
,whose body only consists of a
static_assert
. Note that this overload is markednoexcept
.The main overload could not be chosen, since
from_json(json, A)
could not be found by the compiler at that point(it is defined above, but the compiler cannot see it, according to lookup rules)
static_assert
doesn't trigger because only prototypes are looked up at this point.Since
from_json_fn::call(json, A)
is markednoexcept
,j.at("test").get<A>()
isnoexcept
too.However, the compiler seems to perform another instantiation once
from_json(json, A)
is defined.Thus, your code calls the first overload of
from_json_fn::call
, which callsfrom_json(json, A)
.But, the
noexcept
-ness remains, and that's why your program crashes. If I understood the SO answer correctly, your code leads to undefined behaviour.There are 3 things we can do to resolve that issue:
noexcept
in the library codefrom_json_fn::call
The first solution would make your code work, but it still is undefined behaviour. And people caring about
noexcept
won't be happy.The second would make your code fail at compile-time, but the
static_assert
would disappear, it won't be long before we hear complaints from users.I think the last one is the only viable solution, you should either declare your prototypes before defining your methods, or put the definition in the correct order.
Hope I was clear enough.
andbleo commentedon May 9, 2017
My preference would be for something like this to cause a compiler error or warning, but I don't understand this library or the C++ standard well enough to properly evaluate the options you provided. Regardless, adding some docs explaining this issue would be great.
stale commentedon Oct 25, 2017
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
nlohmann commentedon Oct 27, 2017
@theodelrieu Can we close this?
theodelrieu commentedon Oct 27, 2017
We should an entry in the documentation about the definition order. Other than that, I don't think we can do anything.
nlohmann commentedon Oct 27, 2017
Hm. What would be a proper line to add to the documentation?
theodelrieu commentedon Oct 27, 2017
Something along those lines:
Be careful with about the definition order of from/to_json. If a type B has a member of type A, you MUST define
to_json(A)
beforeto_json(B)
, look at issue 561 for more details.📝 comment to address #561
nlohmann commentedon Oct 27, 2017
Thanks a lot!