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

Compilation error when using NLOHMANN_JSON_SERIALIZE_ENUM ordered_json on libc++ #2491

Closed
3 tasks done
alexezeder opened this issue Dec 2, 2020 · 20 comments · Fixed by #2825
Closed
3 tasks done

Compilation error when using NLOHMANN_JSON_SERIALIZE_ENUM ordered_json on libc++ #2491

alexezeder opened this issue Dec 2, 2020 · 20 comments · Fixed by #2825
Assignees
Labels
kind: bug solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@alexezeder
Copy link

alexezeder commented Dec 2, 2020

Usage of NLOHMANN_JSON_SERIALIZE_ENUM with nlohmann::ordered_json results in a compilation error for libc++ with -std=c++17 and greater.

What is the issue you have?

The NLOHMANN_JSON_SERIALIZE_ENUM expands to the custom to_json and from_json functions. Both of these functions have the following code:

static const std::pair<Enum, BasicJsonType> m[] = /* macro argument */;

And they work fine with using GCC or using (Clang + libstdc++) or using (Clang + libc++) but trying to convert enum value to nlohmann::json, but when (Clang + libc++) is used and enum value is converting to nlohmann::ordered_json then there is a compilation error.

Entire compiler output with error
In file included from <source>:1:
In file included from /opt/compiler-explorer/libs/nlohmann_json/trunk/single_include/nlohmann/json.hpp:37:
In file included from /opt/compiler-explorer/clang-11.0.0/bin/../include/c++/v1/algorithm:642:
/opt/compiler-explorer/clang-11.0.0/bin/../include/c++/v1/utility:514:58: error: no member named 'value' in 'std::__1::is_copy_assignable<nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>>>'
                        is_copy_assignable<second_type>::value,
                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
/opt/compiler-explorer/libs/nlohmann_json/trunk/single_include/nlohmann/json.hpp:3856:17: note: in instantiation of template class 'std::__1::pair<const std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>>>' requested here
    -> decltype(from_json(j, val), void())
                ^
/opt/compiler-explorer/libs/nlohmann_json/trunk/single_include/nlohmann/json.hpp:4451:17: note: while substituting deduced template arguments into function template 'operator()' [with BasicJsonType = nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>>, T = nlohmann::ordered_map<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>>, std::__1::less<void>, std::__1::allocator<std::__1::pair<const std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>>>>> *]
    -> decltype(::nlohmann::from_json(std::forward<BasicJsonType>(j), val), void())
                ^
/opt/compiler-explorer/libs/nlohmann_json/trunk/single_include/nlohmann/json.hpp:3094:37: note: while substituting deduced template arguments into function template 'from_json' [with BasicJsonType = const nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>> &, ValueType = nlohmann::ordered_map<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>>, std::__1::less<void>, std::__1::allocator<std::__1::pair<const std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>>>>> *]
using from_json_function = decltype(T::from_json(std::declval<Args>()...));
                                    ^
/opt/compiler-explorer/libs/nlohmann_json/trunk/single_include/nlohmann/json.hpp:2911:33: note: in instantiation of template type alias 'from_json_function' requested here
struct detector<Default, void_t<Op<Args...>>, Op, Args...>
                                ^
/opt/compiler-explorer/libs/nlohmann_json/trunk/single_include/nlohmann/json.hpp:2921:1: note: during template argument deduction for class template partial specialization 'detector<Default, void_t<Op<Args...>>, Op, Args...>' [with Default = nlohmann::detail::nonesuch, Op = from_json_function, Args = <nlohmann::adl_serializer<nlohmann::ordered_map<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>>, std::__1::less<void>, std::__1::allocator<std::__1::pair<const std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>>>>> *, void>, const nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>> &, nlohmann::ordered_map<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>>, std::__1::less<void>, std::__1::allocator<std::__1::pair<const std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>>>>> *&>]
using detected_t = typename detector<nonesuch, void, Op, Args...>::type;
^
/opt/compiler-explorer/libs/nlohmann_json/trunk/single_include/nlohmann/json.hpp:2921:1: note: (skipping 22 contexts in backtrace; use -ftemplate-backtrace-limit=0 to see all)
<source>:12:50: note: in instantiation of template class 'std::__1::pair<Enum1, nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>>>' requested here
    static const std::pair<Enum1, BasicJsonType> m[] = { 
                                                 ^
/opt/compiler-explorer/libs/nlohmann_json/trunk/single_include/nlohmann/json.hpp:4420:16: note: in instantiation of function template specialization 'to_json<nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>>>' requested here
        return to_json(j, std::forward<T>(val));
               ^
/opt/compiler-explorer/libs/nlohmann_json/trunk/single_include/nlohmann/json.hpp:4470:9: note: in instantiation of function template specialization 'nlohmann::detail::to_json_fn::operator()<nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>>, Enum1>' requested here
        ::nlohmann::to_json(j, std::forward<ValueType>(val));
        ^
/opt/compiler-explorer/libs/nlohmann_json/trunk/single_include/nlohmann/json.hpp:18014:28: note: in instantiation of function template specialization 'nlohmann::adl_serializer<Enum1, void>::to_json<nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>>, Enum1>' requested here
        JSONSerializer<U>::to_json(*this, std::forward<CompatibleType>(val));
                           ^
<source>:19:32: note: in instantiation of function template specialization 'nlohmann::basic_json<nlohmann::ordered_map, std::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, bool, long, unsigned long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char>>>::basic_json<Enum1, Enum1, 0>' requested here
    nlohmann::ordered_json j = Enum1::Value1;
                               ^

This probably depends on C++ standard version in libc++, for example with -std=c++11 and -std=c++14 it works fine.

Please describe the steps to reproduce the issue.

You can reproduce the issue by looking at this useful example on Compiler Explorer with Clang 11.0.0 and -std=c++17.
Note that NLOHMANN_JSON_SERIALIZE_ENUM is expanded already there and a bit simplified, if this is not necessary then here is also an example without NLOHMANN_JSON_SERIALIZE_ENUM macro expansion.

Example with only std::is_copy_assignable<nlohmann::ordered_json>::value usage that also fails.

Clang 11.0.0 with -std=c++14 compiles this code on Compiler Explorer.

Can you provide a small but working code example?

#include "nlohmann/json.hpp"

enum class Enum1
{
    Value1,
    Value2
};

NLOHMANN_JSON_SERIALIZE_ENUM( Enum1,
{
    {Enum1::Value1,   "Enum1::Value1"},
    {Enum1::Value2,   "Enum1::Value2"},
})

int main()
{
    nlohmann::ordered_json j = Enum1::Value1;
}

What is the expected behavior?

It should compile

And what is the actual behavior instead?

Compilation error

Which compiler and operating system are you using?

  • Compiler: Clang 11 + libc++-11 on PC, Clang on Compiler Explorer
  • Operating system: Ubuntu 20.10

Which version of the library did you use?

  • latest release version 3.9.1
  • "trunk" on Compiler Explorer

If you experience a compilation error: can you compile and run the unit tests?

  • yes
@YarikTH
Copy link
Contributor

YarikTH commented Dec 12, 2020

Additional info. I refuse to comprehend it, but forced instantination of std::is_copy_constructible<nlohmann::ordered_json> 'fixes' this problem. Looks like a bug in libc++.
https://godbolt.org/z/bsdT1z
B̶t̶w̶ ̶i̶t̶ ̶s̶e̶e̶m̶s̶ ̶t̶h̶a̶t̶ ̶#̶2̶5̶1̶9̶ ̶h̶a̶s̶ ̶a̶l̶m̶o̶s̶t̶ ̶t̶h̶e̶ ̶s̶a̶m̶e̶ ̶i̶s̶s̶u̶e̶,̶ ̶b̶u̶t̶ ̶w̶i̶t̶h̶ ̶Q̶T̶ ̶i̶n̶s̶t̶e̶a̶d̶ ̶o̶f̶ ̶l̶i̶b̶c̶+̶+̶.̶ ̶M̶a̶y̶b̶e̶ ̶w̶o̶r̶k̶a̶r̶o̶u̶n̶d̶ ̶w̶i̶l̶l̶ ̶b̶e̶ ̶t̶h̶e̶ ̶s̶a̶m̶e̶?̶

@nlohmann
Copy link
Owner

Good catch, but mega confusing...

@YarikTH
Copy link
Contributor

YarikTH commented Dec 14, 2020

Posted bug in llvm bugtracker. Maybe someone who understands type-traits implementation will answer.
https://bugs.llvm.org/show_bug.cgi?id=48507

@karzhenkov
Copy link
Contributor

This is definitively a Clang compiler bug (not to be mixed with Qt bug from #2519 :). As @YarikTH pointed out, an unrelated declaration can change the compiler behavior. Consider also the following:

using J = nlohmann::ordered_json;
using magic = decltype(std::declval<J&>() = std::declval<J&>());
bool check = std::is_copy_assignable<J>::value;

Assuming clang -std=c++17 -stdlib=libc++, the above code will not be compiled if the "magic" is removed.
(see https://godbolt.org/z/qozhj7)

@YarikTH
Copy link
Contributor

YarikTH commented Dec 16, 2020

I minimized the library code as much as I can. Still 400 lines of complicated template code, but at least it's a good starting point to investigate further. It's too much to paste here, so here are the links:

Godbolt: https://godbolt.org/z/zf17vT
GitHub: https://github.com/YarikTH/json/blob/feature/libcxx_debug/single_include/nlohmann/json.hpp

@nlohmann
Copy link
Owner

Is there anything we can do for this library at the moment?

@nlohmann nlohmann added the state: help needed the issue needs help to proceed label Dec 30, 2020
@YarikTH
Copy link
Contributor

YarikTH commented Dec 30, 2020

I'm stuck with 400 lines of templated spaghetti code and because I neither nlohmann::json maintainer, nor llvm specialist I have no idea how to dig it further. I see no meaningful workarounds of the problem on the nlohmann::json side. So maybe it's time to say that it's obviously llvm bug and call it a day.

@karzhenkov
Copy link
Contributor

Maybe some kind of "magic" in nlohmann::detail will be enough for now?

@alexezeder
Copy link
Author

I think that some workaround can be added unless it is something forbidden for the library. I mean, I found only one workaround (for MSVC) in code with a proper comment...
IMHO, even though it's an LLVM bug, it doesn't mean that the library should require that kind of workarounds on the client-side

@YarikTH
Copy link
Contributor

YarikTH commented Dec 31, 2020

Ok. If the common decision is to add a workaround, then I have it for you:
Code: https://godbolt.org/z/WdWeTY

#include "nlohmann/json.hpp"

enum class Enum1
{
    Value1,
    Value2
};

// Work around llvm bug described in #2491
#if JSON_USING_WORKAROUND //JSON_HAS_CPP_17
#   define JSON_LIBCXX_STD17_WORKAROUND( BasicJsonType ) \
    static_assert( std::is_copy_constructible<BasicJsonType>::value )
#else
#   define JSON_LIBCXX_STD17_WORKAROUND( BasicJsonType ) static_assert(true, "")
#endif

/*!
@brief macro to briefly define a mapping between an enum and JSON
@def NLOHMANN_JSON_SERIALIZE_ENUM
@since version 3.4.0
*/
#define NLOHMANN_JSON_SERIALIZE_ENUM_2(ENUM_TYPE, ...)                                            \
    template<typename BasicJsonType>                                                            \
    inline void to_json(BasicJsonType& j, const ENUM_TYPE& e)                                   \
    {                                                                                           \
        static_assert(std::is_enum<ENUM_TYPE>::value, #ENUM_TYPE " must be an enum!");          \
        JSON_LIBCXX_STD17_WORKAROUND(BasicJsonType);                                            \
        static const std::pair<ENUM_TYPE, BasicJsonType> m[] = __VA_ARGS__;                     \
        auto it = std::find_if(std::begin(m), std::end(m),                                      \
                               [e](const std::pair<ENUM_TYPE, BasicJsonType>& ej_pair) -> bool  \
        {                                                                                       \
            return ej_pair.first == e;                                                          \
        });                                                                                     \
        j = ((it != std::end(m)) ? it : std::begin(m))->second;                                 \
    }                                                                                           \
    template<typename BasicJsonType>                                                            \
    inline void from_json(const BasicJsonType& j, ENUM_TYPE& e)                                 \
    {                                                                                           \
        static_assert(std::is_enum<ENUM_TYPE>::value, #ENUM_TYPE " must be an enum!");          \
        JSON_LIBCXX_STD17_WORKAROUND(BasicJsonType);                                            \
        static const std::pair<ENUM_TYPE, BasicJsonType> m[] = __VA_ARGS__;                     \
        auto it = std::find_if(std::begin(m), std::end(m),                                      \
                               [&j](const std::pair<ENUM_TYPE, BasicJsonType>& ej_pair) -> bool \
        {                                                                                       \
            return ej_pair.second == j;                                                         \
        });                                                                                     \
        e = ((it != std::end(m)) ? it : std::begin(m))->first;                                  \
    }

NLOHMANN_JSON_SERIALIZE_ENUM_2( Enum1, {
    {Enum1::Value1, "Value1"},
    {Enum1::Value2, "Value2"}} )

int main()
{
    nlohmann::ordered_json j = Enum1::Value1;
}

@alexezeder
Copy link
Author

alexezeder commented Dec 31, 2020

I minimized the library code as much as I can. Still 400 lines of complicated template code, but at least it's a good starting point to investigate further. It's too much to paste here, so here are the links:

Godbolt: https://godbolt.org/z/zf17vT
GitHub: https://github.com/YarikTH/json/blob/feature/libcxx_debug/single_include/nlohmann/json.hpp

Thanks to your work here, @YarikTH, I was able to make a deeper investigation and found a bit smaller code example (69 lines) that also fails. But the example that I'm about to present also fails on libstdc++ too, GCC standard library implementation. While the original 400+ lines version fails only with libc++. Probably it's due to a reduced number of included files because I was only removing code, but not adding.

Here are two demonstrations on Compiler Explorer with the same code:

  1. https://godbolt.org/z/fhTf7j
  2. https://godbolt.org/z/jbeTje

The first one just shows that Clang now also fails with libstdc++, while GCC does not fail, as well as MSVC with its own STD library.

The second demonstrates that by using some workarounds a compilation error can be eliminated on Clang. There are 2 workarounds, but they have one thing in common - they manipulate somehow with std::pair, the first workaround just replaces the usage of std::pair with std::tuple in the code, while the second one explicitly instantiates std::pair with used template parameters.

I also posted this on LLVM bug tracker.

@YarikTH
Copy link
Contributor

YarikTH commented Jan 4, 2021

I have another idea. If replacing of std::pair with std::tuple is a viable strategy, then we can change ordered_map to work with std::vector<std::tuple<const Key, T>>. But I'm not sure is it a good idea to change ordered_map like this just as a workaround for libc++ c++17 bug in one specific use-case.

@YarikTH
Copy link
Contributor

YarikTH commented Jan 5, 2021

Dug in a little. Altering of internal type for ordered_map is impossible becase of

    using object_t = ObjectType<StringType,
          basic_json,
          object_comparator_t,
          AllocatorType<std::pair<const StringType,
          basic_json>>>;

Allocator type is hardcoded into basic_json type. So my genius idea is not viable in the current codebase.

@nlohmann
Copy link
Owner

I think it would make sense to add a CI step that uses libc++, right?

@alexezeder
Copy link
Author

AFAIK, macOS uses some kind of libc++, so this problem would have been detected if the macOS pipeline had C++17 standard check, and there was also a unit-test for ordered_json from PR.
But Ubuntu pipeline can also have Clang build with libc++ with all supported standards. I don't really understand the difference between Clang and Apple Clang, maybe there is even Apple libc++ exists, who knows...
I think these changes may help to uncover more bugs.

@nlohmann
Copy link
Owner

@alexezeder The CI now compiles the tests on macOS with Clang using C++11, C++14, C++17, and C++20. I have not seen a failed test though.

@ldionne
Copy link
Contributor

ldionne commented Jun 17, 2021

@alexezeder The CI now compiles the tests on macOS with Clang using C++11, C++14, C++17, and C++20. I have not seen a failed test though.

The simple test case I added in #2825 fails with the latest version of the library without my fixes. Try this on develop and then with my fix:

cat <<EOF | clang++ -xc++ - -std=c++17 -fsyntax-only -I include
#include <nlohmann/json.hpp>
#include <type_traits>
static_assert(std::is_copy_assignable<nlohmann::ordered_json>::value, "");
EOF

@alexezeder
Copy link
Author

Yeah, the test should be added first to get this error.
My PR adds a test for this case, looks like the one from @ldionne does it too.

@ldionne
Copy link
Contributor

ldionne commented Jun 17, 2021

I just realized there was already a PR for this even before mine. @nlohmann Can this be fixed ASAP? I've gotten numerous reports of problems related to this over the past 6 months/year, some external and some internal at the company I work at. Everybody was obviously pointing to libc++ even though the problem is in this library. It's a pretty vexing issue for users in a widely used library, and it would be great to prioritize it as such.

To clarify the situation regarding libc++:

  1. On macOS, Clang uses libc++ by default. On Linux, it uses libstdc++ by default (I believe).
  2. If you are using Apple's Clang, then you are using libc++ automatically.

So adding any CI that uses AppleClang on macOS should provide coverage for this. You should make sure to compile your library as C++17 though, since the issue is apparently invisible when compiling as C++20.

@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Jul 16, 2021
@nlohmann
Copy link
Owner

@ldionne I would be happy to help! I added some remarks to #2825.

@nlohmann nlohmann removed the state: help needed the issue needs help to proceed label Jul 22, 2021
@nlohmann nlohmann self-assigned this Jul 22, 2021
@nlohmann nlohmann added this to the Release 3.9.2 milestone Jul 22, 2021
harry75369 pushed a commit to harry75369/json that referenced this issue Aug 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
5 participants