from_json declaration order and exceptions

[andbleo]

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.

[nlohmann]

I can confirm you observation, but I also have no idea. Maybe a language lawyer like @gregmarr or @theodelrieu knows more about this.

[theodelrieu]

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 every noexcept(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]

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 the from_json_fn::operator()().

This method has a noexcept(noexcept(to_json(bla, bla))) construct. The thing is, there is no method void 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 of nlohmann::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]

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 of nlohmann::detail::from_json_fn::call,
whose body only consists of a static_assert. Note that this overload is marked noexcept.
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 marked noexcept, j.at("test").get<A>() is noexcept 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 calls from_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:

• remove every noexcept in the library code
• remove the fallback overload of from_json_fn::call
• don't touch the code, and add an entry in the documentation

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]

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]

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]

@theodelrieu Can we close this?

[theodelrieu]

We should an entry in the documentation about the definition order. Other than that, I don't think we can do anything.

[nlohmann]

Hm. What would be a proper line to add to the documentation?

[theodelrieu]

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) before to_json(B), look at issue 561 for more details.

[nlohmann]

Thanks a lot!