MSVC: unexpected behaviour converting from json to a rvalue reference of a variant containing a type that can be constructed from a string

[karcherm]

Description

Converting a json object to an rvalue of a variant containing a type that can be constructed from a string and json itself, will result in construction of a variant with that string-constructible type, and crash if the json object does not contain a string. This even happens on a variant containing only nlohmann::json, as that type is both "constructible from string" and "nlohmann::json itself".

I suspect this is actually an MSVC compiler bug, but a workaround in the json library would be nice, if possible.

Reproduction steps

Given

using var = std::variant<nlohmann::json>;
std::vector<var> vars;

run

vars.push_back(nlohmann::json(1));

Expected vs. actual results

Actual: the call vars.push_back(json(1)) results in an assertion failure

Expected: a variant containing a json object containing the number 1 is pushed into the vector

The expected result occurs, because the json object does not contain a string. This happens because the type var is constructible from a string, and if MSVC requires a conversion from json to var &&, it prefers the route via template<typename T> T json::operator T() const over the route of creating a temporary variant being passed to push_back.

Minimal code example

#include <variant>
#include <vector>
#include <nlohmann/json.hpp>

int main()
{
    std::vector<std::variant<nlohmann::json>> v;
    v.push_back(nlohmann::json(1));
}

Furthermore, I set up a demonstration for MSVC picking the unwanted conversion path without nlohmann::json on Compiler Explorer. I did not use the actual JSON library, because Compiler Explorer doesn't support external libraries on Windows. I tested locally using MSVC that the issue happens with the acutal JSON library in exactly the same way. See
https://godbolt.org/z/xP8r1nYT8 .

Error messages

The miminal example shown bove terminates with an assertion failure (`JSON_THROW`) with this backtrace:

nlohmann::detail::from_json<nlohman::json, std::variant<nlohmann::json>, 0>(const nlohman::json&, std::variant<nlohmann::json>&)
nlohmann::detail::from_json_fn::operator()<nlohmann::json, std::variant<nlohmann::json>&>(nlohmann::json&, std::variant<nlohmann::json>&)
nlohmann::adl_serializer<std::variant<nlohmann::json>, void>::from_json<const nlohmann::json &, std::variant<nlohmann::json>>(const nlohmann::json&, std::variant<nlohmann::json>>&)
nlohmann::json::get_impl<std::variant<nlohmann::json>, 0>(nlohmann::detail::priority_tag<0>)
nlohmann::json::get<std::variant<nlohmann::json>, std::variant<nlohmann::json>>()
nlohmann::json::operator<std::variant<nlohmann::json>, 0> std::variant<nlohmann::json>()
main()

Compiler and operating system

Visual Studio 17.6.5 running on Microsoft Window 11 25H2

Library version

3.10.5

Validation

• The bug also occurs if the latest version from the develop branch is used.
• I can successfully compile and run the unit tests.

[abhay0132]

Hi, I’d like to take a look at this issue and try to reproduce it locally.
I’ll update here if I find anything.

[safocl]

Furthermore, I set up a demonstration for MSVC picking the unwanted conversion path without nlohmann::json on Compiler Explorer. I did not use the actual JSON library, because Compiler Explorer doesn't support external libraries on Windows. I tested locally using MSVC that the issue happens with the acutal JSON library in exactly the same way. See
https://godbolt.org/z/xP8r1nYT8 .

https://eel.is/c++draft/variant.variant#variant.ctor-note-2

[Note 2: variant<string, string> v("abc");
is ill-formed, as both alternative types have an equally viable constructor for the argument.
— end note]

[gregmarr]

This looks to be an example of why the implicit conversions are bad, and you should use JSON_USE_IMPLICIT_CONVERSIONS set to 0.

https://github.com/nlohmann/json?tab=readme-ov-file#implicit-conversions

[karcherm]

https://eel.is/c++draft/variant.variant#variant.ctor-note-2

[Note 2: variant<string, string> v("abc");
is ill-formed, as both alternative types have an equally viable constructor for the argument.
— end note]

I didn't look up the details, but neither MSVC, clang or gcc refuse to compile the program, which would be the correct response for an ill-formed program. Note that the standard specifically provides the category "ill-formed, no diagnostic required", which is not applied here. Well, actually, Notes are not normative, but this note relies on https://eel.is/c++draft/over.match.best#general-3 :

If there is exactly one viable function that is a better function than all other viable functions, then it is the one selected by overload resolution; otherwise the call is ill-formed.

This is a diagnosable rule according to https://eel.is/c++draft/intro.compliance#general-1 :

The set of diagnosable rules consists of all syntactic and semantic rules in this document except for those rules containing an explicit notation that “no diagnostic is required” or which are described as resulting in “undefined behavior”.

Which should result in a diagnostic (which may formally be a warning or an error), according to https://eel.is/c++draft/intro.compliance#general-2.3

Otherwise, if a program contains

• a violation of any diagnosable rule,
• ...
  a conforming implementation shall issue at least one diagnostic message.

Even with an increased warning level (-Wextra -Wall for clang/gcc, /W4 for MSVC), neither implementation prints any error or warning: https://godbolt.org/z/Eze6q1ohE

So it doesn't look like my program is ill-formed.

This looks to be an example of why the implicit conversions are bad, and you should use JSON_USE_IMPLICIT_CONVERSIONS set to 0.

https://github.com/nlohmann/json?tab=readme-ov-file#implicit-conversions

That's a fair point. I will check whether JSON_USE_IMPLICIT_CONVERSION = 0 will be applicable to the code base we encountered the issue in.

[safocl]

Furthermore, I set up a demonstration for MSVC picking the unwanted conversion path without nlohmann::json on Compiler Explorer. I did not use the actual JSON library, because Compiler Explorer doesn't support external libraries on Windows. I tested locally using MSVC that the issue happens with the acutal JSON library in exactly the same way. See
https://godbolt.org/z/xP8r1nYT8 .

This example uses the default ctor std::variant<std::string, Args...>, which constructs std::string:

template<typename ...Args> operator std::variant<std::string, Args...>() const { return {}; }

why JSON is printed for GCC and CLANG, I don't know

[safocl]

https://godbolt.org/z/5boqWad79 -- direct initialization fixed this behavior.

[safocl]

I think MSVC doesn't satisfy https://eel.is/c++draft/over.match.best#over.best.ics.general-4.4

4 However, if the target is
(4.1)
the first parameter of a constructor or
[...]
and the constructor or user-defined conversion function is a candidate by
[...]
(4.4) [over.match.copy], [over.match.conv], or [over.match.ref] (in all cases), or
[...]

user-defined conversion sequences are not considered.

[safocl]

https://godbolt.org/z/c6zcxYdsn -- simplified example

[safocl]

other example with consteval functions:
https://godbolt.org/z/Kh9KevezK

[safocl]

Furthermore, I set up a demonstration for MSVC picking the unwanted conversion path without nlohmann::json on Compiler Explorer. I did not use the actual JSON library, because Compiler Explorer doesn't support external libraries on Windows. I tested locally using MSVC that the issue happens with the acutal JSON library in exactly the same way. See
https://godbolt.org/z/xP8r1nYT8 .

https://godbolt.org/z/E748r1rbj -- this changed example results in ambiguous
(added non-const annotated operator Type() )

[karcherm]

Interestingly, when using actual nlohmann::json, if the object is const in main, I still get "JSON" all the time using GCC, but all three calls are ambiguous according to clang. With JSON_USE_IMPLICIT_CONVERSIONS set to zero, I always get the variant constructor I intend to get.

Without looking at the C++ conversion rules or the std::variant implementation, I intuitively expect that if I have an object of type T, which is one of the choices of a union type, and I want to create an instance of that union type, I get that specific variant of the union type without any value conversions. Probably the best way to deal with the actual issue I have with nlohman::kson is to explicitly disable the (in this case) overly aggressive implicit conversions.

[gregmarr]

Yes, they are very aggressive, and harmful at times. We have JSON_USE_IMPLICIT_CONVERSIONS because we don't want to break code by default without a major version bump, and we haven't done that yet.