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

Cannot assign from ordered_json vector<CustomStruct> to value in not ordered json #2528

Closed
2 of 5 tasks
kotori2 opened this issue Dec 13, 2020 · 4 comments · Fixed by #4597
Closed
2 of 5 tasks

Cannot assign from ordered_json vector<CustomStruct> to value in not ordered json #2528

kotori2 opened this issue Dec 13, 2020 · 4 comments · Fixed by #4597
Labels
kind: bug solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@kotori2
Copy link

kotori2 commented Dec 13, 2020

What is the issue you have?

I created a custom struct in unordered_json with vector, then assign it to an ordered json, it will throw error.

Please describe the steps to reproduce the issue.

Check the snippet below.
Actually it works if both of them are ordered json or unordered json, or I removed the vector.

Can you provide a small but working code example?

I defined my own NLOHMANN_DEFINE_TYPE_INTRUSIVE just to conveniently define them in unordered json.

using nlohmann::json;
using ordered_json = nlohmann::ordered_json;

#define _NLOHMANN_DEFINE_TYPE_INTRUSIVE(Type, ...)  \
    friend void to_json(ordered_json& nlohmann_json_j, const Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_TO, __VA_ARGS__)) } \
    friend void from_json(const ordered_json& nlohmann_json_j, Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_FROM, __VA_ARGS__)) }

class releaseInfo {
public:
    uint64_t id = 0;
    std::string key;
    _NLOHMANN_DEFINE_TYPE_INTRUSIVE(releaseInfo, id, key);
};

int main() {
    std::vector<releaseInfo> test;
    json parent;
    parent["test"] = test;
    return 0;
}

What is the expected behavior?

No error happened.

And what is the actual behavior instead?

/Users/kotori0/code/untitled/main.cpp:21:15: error: no viable overloaded '='
    a["test"] = test;
    ~~~~~~~~~ ^ ~~~~
/Users/kotori0/code/untitled/cmake-build-debug/_deps/json-src/single_include/nlohmann/json.hpp:18662:17: note: candidate function not viable: no known conversion from 'std::vector<releaseInfo>' to 'nlohmann::basic_json<std::map, std::vector, std::__1::basic_string<char>, bool, long long, unsigned long long, double, std::allocator, adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char> > >' for 1st argument
    basic_json& operator=(basic_json other) noexcept (
                ^
1 error generated.

Which compiler and operating system are you using?

  • Compiler: Apple clang version 12.0.0 (clang-1200.0.32.27)
  • Operating system: Darwin Hackintosh 20.1.0 Darwin Kernel Version 20.1.0: Sat Oct 31 00:07:11 PDT 2020; root:xnu-7195.50.7~2/RELEASE_X86_64 x86_64

Which version of the library did you use?

  • latest release version 3.9.1
  • other release - please state the version: ___
  • 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
@YarikTH
Copy link
Contributor

YarikTH commented Dec 14, 2020

It's a bug in your own code.
https://godbolt.org/z/qvjdGW
You defined to_json and from_json exactly for ordered_json& type, while try to use with json& type.
j̶s̶o̶n̶ ̶a̶n̶d̶ ̶o̶r̶d̶e̶r̶e̶d̶_̶j̶s̶o̶n̶ ̶a̶r̶e̶ ̶n̶o̶t̶ ̶c̶o̶n̶v̶e̶r̶t̶i̶b̶l̶e̶ ̶t̶o̶ ̶e̶a̶c̶h̶ ̶o̶t̶h̶e̶r̶, so you don't write proper to_json overload to be used with json.
Also, I don't get a part about "Actually it works if ..., or I removed the vector."
https://godbolt.org/z/v8xrhM
As you can see, vector or not, compilation failed, because you don't have to_json(nlohmann::json&, const releaseInfo&) overload.

Actually, json and ordered_json are convertible to each other so you can construct ordered_json from your type and then use it as a field in json, But it's not scalable.
https://godbolt.org/z/a8fhss

int main() {
    std::vector<releaseInfo> test;
    ordered_json children = test;
    json parent;
    parent["test"] = children;
    return 0;
}

See a possible solution:
https://godbolt.org/z/xMvaja

#include <nlohmann/json.hpp>

using nlohmann::json;
using ordered_json = nlohmann::ordered_json;

#define _NLOHMANN_DEFINE_TYPE_INTRUSIVE(JsonType, Type, ...)  \
    friend void to_json(JsonType& nlohmann_json_j, const Type& nlohmann_json_t){ \
        NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_TO, __VA_ARGS__)) \
    }


class releaseInfo {
public:
    uint64_t id = 0;
    std::string key;
    _NLOHMANN_DEFINE_TYPE_INTRUSIVE(nlohmann::json, releaseInfo, id, key);
    _NLOHMANN_DEFINE_TYPE_INTRUSIVE(nlohmann::ordered_json, releaseInfo, id, key);
};

int main() {
    std::vector<releaseInfo> test;
    json parent;
    parent["test"] = test;
    return 0;
}

As I can see NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE both don't work with ordered_json. And moreover, they don't work with any other basic_json overload.

From the nlohmann/json point of view, it's kind of lack of feature.
Ideally, NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE should work with any basic_json overload.
Like here:
https://godbolt.org/z/d16erK

#define NLOHMANN_DEFINE_TYPE_INTRUSIVE_2(Type, ...)  \
    template<class JsonType> friend void to_json(JsonType& nlohmann_json_j, const Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_TO, __VA_ARGS__)) } \
    template<class JsonType> friend void from_json(const JsonType& nlohmann_json_j, Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_FROM, __VA_ARGS__)) }

#define NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_2(Type, ...)  \
    template<class JsonType> void to_json(JsonType& nlohmann_json_j, const Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_TO, __VA_ARGS__)) } \
    template<class JsonType> void from_json(const JsonType& nlohmann_json_j, Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_FROM, __VA_ARGS__)) }

Need to test new macroses more though. I wrote them in a few minutes. They may break ODR or something.

@ghost
Copy link

ghost commented Dec 15, 2020

@kotori2 you can be able to fix the bug in order to debug it

@kotori2
Copy link
Author

kotori2 commented Dec 15, 2020

@YarikTH Thanks for your kind and thorough explanation.

Also, I don't get a part about "Actually it works if ..., or I removed the vector."

I think my memory just corrupted...

json and ordered_json are not convertible to each other
Actually, json and ordered_json are convertible to each other so you can construct ordered_json from your type and then use it as a field in json, But it's not scalable.

Actually it's kinda weird to me. Simply value assigning will work between json and ordered_json.

json a;
ordered_json b(a);
json c(b);

But when it comes to function calls, it doesn't work.
Does it means there is explicit constructor for json -> ordered_json, and ordered_json -> json, but it doesn't apply for function calls?
I tried to read the code but the tons of templates stopped me

@YarikTH
Copy link
Contributor

YarikTH commented Dec 15, 2020

But when it comes to function calls, it doesn't work.
Does it means there is explicit constructor for json -> ordered_json, and ordered_json -> json, but it doesn't apply for function calls?

I'm not the author, but it seems it is.
IMHO, in general, it's a good idea to declare to_json and from_json with templated BasicJsonType, as NLOHMANN_JSON_SERIALIZE_ENUM does. And not to mix json of different type, because it inconvenient at least and very likely does something unexprectable, because json values can't contain json values of another type. See example below:
https://godbolt.org/z/e3PzYc

#include <nlohmann/json.hpp>
#include <iostream>

using nlohmann::json;
using ordered_json = nlohmann::ordered_json;

#define _NLOHMANN_DEFINE_TYPE_INTRUSIVE(Type, ...)  \
    friend void to_json(ordered_json& nlohmann_json_j, const Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_TO, __VA_ARGS__)) } \
    friend void from_json(const ordered_json& nlohmann_json_j, Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_FROM, __VA_ARGS__)) }

class releaseInfo {
public:
    uint64_t id = 0;
    std::string key;
    std::string author;// field especialy intended to be alphabetically lower than others
    _NLOHMANN_DEFINE_TYPE_INTRUSIVE(releaseInfo, id, key, author);
};

int main() {
    std::vector<releaseInfo> test(1, releaseInfo{});
    ordered_json children = test;
    std::cout << "ordered children: \n" << children.dump(4);
    
    std::cout << "\n\n";

    json parent;
    parent["test"] = children;
    std::cout << "unordered parent: \n" << parent.dump(4);
    return 0;
}

Output:

ordered children: 
[
    {
        "id": 0,
        "key": "",
        "author": ""
    }
]

unordered parent: 
{
    "test": [
        {
            "author": "",
            "id": 0,
            "key": ""
        }
    ]
}

As you can see, values added in ordered_json being used in json become reordered.

@kotori2 kotori2 closed this as completed Dec 16, 2020
@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Jan 18, 2025
@nlohmann nlohmann linked a pull request Jan 18, 2025 that will close this issue
4 tasks
@nlohmann nlohmann added this to the Release 3.11.4 milestone Jan 18, 2025
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
Development

Successfully merging a pull request may close this issue.

3 participants