Skip to content

Parse to custom class from unordered_json breaks on G++11.2.0 with C++20 #3312

Closed
#3427
Closed
Parse to custom class from unordered_json breaks on G++11.2.0 with C++20#3312
#3427
Assignees
nlohmann
Labels
kind: bugrelease item: 🐛 bug fixsolution: proposed fixa fix for the issue has been proposed and waits for confirmation
Milestone
Release 3.11.0

Description

@NeroBurner
NeroBurner
opened on Feb 1, 2022

What is the issue you have?

Parsing a custom class with a std::string member in it using nlohmann::ordered_json with C++20 starting with v3.10.4 results in a compilation error.

Using v3.10.3 compiles fine

Probably related issue: #3207

Please describe the steps to reproduce the issue.

Godbolt link with example showing that g++ 11.2.0 fails to compile when C++20 is enabled

https://godbolt.org/z/MsYe45qP9

  1. Add custom class containing a std::string (custom class with just an int works)
  2. Add custom from_json function for custom class
  3. convert from ordered_json to class (normal unordered json works)
  4. get compilation error

Example code, which I'd like to add to unittests, but don't know where. (for the error message I added it to the bottom of unit-deserialization.cpp)

struct MyClass {
    std::string name;
};

void from_json(const nlohmann::json &j, MyClass &obj)
{
    j.at("name").get_to(obj.name);
}

TEST_CASE("custom class parsing")
{
    SECTION("parse class directly")
    {
        nlohmann::ordered_json j = nlohmann::ordered_json::parse("{\"name\": \"class\"}");
        MyClass obj;
        j.get_to(obj);
        CHECK(obj.name == "class");
    }
}

Can you provide a small but working code example?

see above

What is the expected behavior?

Compile without error

And what is the actual behavior instead?

Compillation error:

In file included from /home/nero/repos/external/nlohmann-json/test/src/unit-deserialization.cpp:32:
/home/nero/repos/external/nlohmann-json/single_include/nlohmann/json.hpp: In instantiation of 'void nlohmann::detail::from_json(const BasicJsonType&, ConstructibleStringType&) [with BasicJsonType = nlohmann::basic_json<nlohmann::ordered_map>; ConstructibleStringType = MyClass; typename std::enable_if<(nlohmann::detail::is_constructible_string_type<BasicJsonType, ConstructibleStringType>::value && (! std::is_same<typename BasicJsonType::string_t, ConstructibleStringType>::value)), int>::type <anonymous> = 0]':
/home/nero/repos/external/nlohmann-json/single_include/nlohmann/json.hpp:4273:25:   required from 'decltype (nlohmann::detail::from_json(j, forward<T>(val))) nlohmann::detail::from_json_fn::operator()(const BasicJsonType&, T&&) const [with BasicJsonType = nlohmann::basic_json<nlohmann::ordered_map>; T = MyClass&; decltype (nlohmann::detail::from_json(j, forward<T>(val))) = void]'
/home/nero/repos/external/nlohmann-json/single_include/nlohmann/json.hpp:4934:30:   required from 'static decltype ((nlohmann::{anonymous}::from_json(forward<BasicJsonType>(j), val), void())) nlohmann::adl_serializer<T, SFINAE>::from_json(BasicJsonType&&, TargetType&) [with BasicJsonType = const nlohmann::basic_json<nlohmann::ordered_map>&; TargetType = MyClass; ValueType = MyClass; <template-parameter-1-2> = void; decltype ((nlohmann::{anonymous}::from_json(forward<BasicJsonType>(j), val), void())) = void]'
/home/nero/repos/external/nlohmann-json/single_include/nlohmann/json.hpp:18955:45:   required from 'ValueType& nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer, BinaryType>::get_to(ValueType&) const [with ValueType = MyClass; typename std::enable_if<((! nlohmann::detail::is_basic_json<BasicJsonType>::value) && nlohmann::detail::has_from_json<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer, BinaryType>, ValueType>::value), int>::type <anonymous> = 0; ObjectType = nlohmann::ordered_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; BinaryType = std::vector<unsigned char>]'
/home/nero/repos/external/nlohmann-json/test/src/unit-deserialization.cpp:1143:17:   required from here
/home/nero/repos/external/nlohmann-json/single_include/nlohmann/json.hpp:3914:7: error: no match for 'operator=' (operand types are 'MyClass' and 'const string_t' {aka 'const std::__cxx11::basic_string<char>'})
 3914 |     s = *j.template get_ptr<const typename BasicJsonType::string_t*>();
      |     ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/nero/repos/external/nlohmann-json/test/src/unit-deserialization.cpp:1127:8: note: candidate: 'MyClass& MyClass::operator=(const MyClass&)'
 1127 | struct MyClass {
      |        ^~~~~~~
/home/nero/repos/external/nlohmann-json/test/src/unit-deserialization.cpp:1127:8: note:   no known conversion for argument 1 from 'const string_t' {aka 'const std::__cxx11::basic_string<char>'} to 'const MyClass&'
/home/nero/repos/external/nlohmann-json/test/src/unit-deserialization.cpp:1127:8: note: candidate: 'MyClass& MyClass::operator=(MyClass&&)'
/home/nero/repos/external/nlohmann-json/test/src/unit-deserialization.cpp:1127:8: note:   no known conversion for argument 1 from 'const string_t' {aka 'const std::__cxx11::basic_string<char>'} to 'MyClass&&'

Error message when compiling with Visual Studio 16 2019 in C++20 mode:

Error    C2679    binary '=': no operator found which takes a right-hand operand of type 'const std::basic_string<char,std::char_traits<char>,std::allocator<char>>' (or there is no acceptable conversion)    test    C:\Software\vcpkg\installed\x64-windows\include\nlohmann\detail\conversions\from_json.hpp    121

Which compiler and operating system are you using?

  • Compiler: g++ 11.2.0 (only when setting C++20 mode)
  • Operating system: Ubuntu 21.10

Also on Visual Studio 16 2019 in C++20 mode

Which version of the library did you use?

  • latest release version 3.10.5
  • other release: 3.10.4
  • other release: 3.10.3 (not affected by bug, working)
  • the develop branch

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

  • yes
  • no - please copy/paste the error message below
"test-comparison" start time: Feb 01 15:05 UTC
Output:
----------------------------------------------------------
[doctest] doctest version is "2.4.6"
[doctest] run with "--help" for options
===============================================================================
/home/nero/repos/external/nlohmann-json/test/src/unit-comparison.cpp:46:
TEST CASE:  lexicographical comparison operators
  values
  comparison: less

/home/nero/repos/external/nlohmann-json/test/src/unit-comparison.cpp:213: ERROR: CHECK( (j_values[i] < j_values[j]) == expected[i][j] ) is NOT correct!
  values: CHECK( false == 1 )
  logged: i := 12
          j := 13
          j_values[i] := [1,2,3]
          j_values[j] := ["one","two","three"]

/home/nero/repos/external/nlohmann-json/test/src/unit-comparison.cpp:213: ERROR: CHECK( (j_values[i] < j_values[j]) == expected[i][j] ) is NOT correct!
  values: CHECK( true == 0 )
  logged: i := 13
          j := 12
          j_values[i] := ["one","two","three"]
          j_values[j] := [1,2,3]

===============================================================================
[doctest] test cases:    1 |    0 passed | 1 failed | 0 skipped
[doctest] assertions: 2220 | 2218 passed | 2 failed |
[doctest] Status: FAILURE!

edit: the unittest test-comparison fails in C++20 mode, as reported in issue: #3207

Activity

  1. NeroBurner
    added
    kind: bug
    on Feb 1, 2022

  2. gregmarr commented on Feb 2, 2022

    @gregmarr
    gregmarr
    on Feb 2, 2022
    Contributor

    Can you try this:

    template<typename BasicJsonType>
void from_json(const BasicJsonType &j, MyClass &obj)
{
    j.at("name").get_to(obj.name);
}

  3. NeroBurner commented on Feb 3, 2022

    @NeroBurner
    NeroBurner
    on Feb 3, 2022
    Author

    @gregmarr the workaround works! Although I don't want to use the templated function, as I'd have to move all my from_json() implementations from the cpp file into the hpp header files. But thanks for taking the time and providing a workaround 🙇

  4. gregmarr commented on Feb 3, 2022

    @gregmarr
    gregmarr
    on Feb 3, 2022
    Contributor

    Are you currently putting extern void from_json(const nlohmann::json &j, MyClass &obj); in your header files? If not, and it's currently working with nlohmann::json, then you don't need to move the implementations to get the templated version to work with nlohmann::unordered_json.

    If you are doing that, you can just add a second version just like the first:

    void from_json(const nlohmann::unordered_json &j, MyClass &obj)
{
    j.at("name").get_to(obj.name);
}

    If you don't want to duplicate the whole body, you can do something like this:

    template<typename BasicJsonType>
void from_json_internal(const BasicJsonType &j, MyClass &obj)
{
    j.at("name").get_to(obj.name);
}

void from_json(const nlohmann::json &j, MyClass &obj)
{
    from_json_internal(j, obj);
}

void from_json(const nlohmann::unordered_json &j, MyClass &obj)
{
    from_json_internal(j, obj);
}

    @nlohmann Do we need to update the guides to tell people to use the templatized version if they're going to be using the other json variants?

  5. nlohmann commented on Feb 5, 2022

    @nlohmann
    nlohmann
    on Feb 5, 2022
    Owner

    @nlohmann Do we need to update the guides to tell people to use the templatized version if they're going to be using the other json variants?

    Yes, I think this was forgotten when ordered_json was introduced.

  6. NeroBurner commented on Feb 5, 2022

    @NeroBurner
    NeroBurner
    on Feb 5, 2022
    Author

    so it was just a bug, that I was able to use it like that until now? (before C++20)

  7. gregmarr commented on Feb 7, 2022

    @gregmarr
    gregmarr
    on Feb 7, 2022
    Contributor

    @NeroBurner Not sure why it worked, there may have been some temporaries being created that probably aren't desired.

    @nlohmann It's not just ordered_json, it's any variation in the template parameters.

  8. NeroBurner commented on Feb 7, 2022

    @NeroBurner
    NeroBurner
    on Feb 7, 2022
    Author

    that's unfortunate, but you're probably right. Thanks for the workarounds. Should I close issue, or do you want to keep it open until documentation is updated for people to find this open issue?

  9. gregmarr commented on Feb 7, 2022

    @gregmarr
    gregmarr
    on Feb 7, 2022
    Contributor

    I would leave it open for the docs. Thanks.

  10. NeroBurner commented on Feb 7, 2022

    @NeroBurner
    NeroBurner
    on Feb 7, 2022
    Author

    Out of curiosity I did a git bisect and found the following commit 0e694b4 (between v3.10.3 and v3.10.4) to introduce this "non-bug"

    0e694b4060ed55df980eaaebc2398b0ff24530d4 is the first bad commit
commit 0e694b4060ed55df980eaaebc2398b0ff24530d4
Author: Théo DELRIEU <theo.delrieu@tanker.io>
Date:   Thu Oct 14 19:19:46 2021 +0200

    fix std::filesystem::path regression (#3073)
    
    * meta: rework is_compatible/is_constructible_string_type
    
    These type traits performed an incorrect and insufficient check.
    
    Converting to a std::filesystem::path used to work by accident thanks to
    these brittle constraints, but the clean-up performed in #3020 broke them.
    
    * support std::filesystem::path
    
    Fixes #3070

 include/nlohmann/detail/conversions/from_json.hpp |  18 ++-
 include/nlohmann/detail/conversions/to_json.hpp   |  13 ++
 include/nlohmann/detail/meta/type_traits.hpp      |  39 ++----
 single_include/nlohmann/json.hpp                  |  71 ++++++-----
 test/src/unit-regression2.cpp                     | 139 ++++++++++++----------
 5 files changed, 161 insertions(+), 119 deletions(-)
  11. falbrechtskirchinger
    added 3 commits that reference this issue on Apr 6, 2022

    Further constrain from_json() for strings

    67cedc0

    Further constrain from_json() for strings

    debcb29

    Fix constraints on from_json() for strings

    ebab61e
  12. nlohmann
    added
    solution: proposed fixa fix for the issue has been proposed and waits for confirmation
    on Apr 7, 2022
  13. nlohmann
    added this to the Release 3.11.0 milestone on Apr 8, 2022
  14. nlohmann
    self-assigned this
    on Apr 8, 2022
  15. nlohmann
    added
    release item: 🐛 bug fix
    on Apr 8, 2022
  16. nlohmann
    closed this as completedin #3427on Apr 8, 2022
  17. nlohmann
    added a commit that references this issue on Apr 8, 2022

    Fix constraints on from_json() for strings (#3427)

    261cc4e

  18. NeroBurner commented on Apr 8, 2022

    @NeroBurner
    NeroBurner
    on Apr 8, 2022
    Author

    Awesome!! Thank you so much for fixing this. I thought it won't be fixed because it was unintentional behaviour and creates temporary copies of the json object if the function doesn't match. But now it is fixed! 🎉
    When possible I'll do the right fix with the templated from_json functions, but it is great to consciously keep the inefficient source code and still be able to update nlohmann_json

    Thank you! 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

kind: bugrelease item: 🐛 bug fixsolution: proposed fixa fix for the issue has been proposed and waits for confirmation

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions