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

NLOHMANN_DEFINE_TYPE_INTRUSIVE with nlohmann::json::json_pointer #4104

Open
2 tasks
risa2000 opened this issue Aug 11, 2023 · 8 comments
Open
2 tasks

NLOHMANN_DEFINE_TYPE_INTRUSIVE with nlohmann::json::json_pointer #4104

risa2000 opened this issue Aug 11, 2023 · 8 comments

Comments

@risa2000
Copy link

Description

Using NLOHMANN_DEFINE_TYPE_INTRUSIVE (or non-intrusive) macro to define conversions for struct with nlohmann::json::json_pointer member fails to compile (MSVC v19.3 - VS 2022), with or without explicit conversion.

Reproduction steps

Compile the sample code.
Here is a Godbolt link https://godbolt.org/z/fdrPrcT7T

Expected vs. actual results

Expecting it will compile.

Minimal code example

#include <nlohmann/json.hpp>
#include <fmt/format.h>
#include <string>

inline void to_json(nlohmann::json& j, const nlohmann::json::json_pointer& crj)
{
    j = nlohmann::json(crj.to_string());
}

inline void from_json(const nlohmann::json& cj, nlohmann::json::json_pointer& rj)
{
    rj = nlohmann::json::json_pointer(cj.get<std::string>());
}

struct Test {
    nlohmann::json::json_pointer ptr;
//    std::string ptr;
    nlohmann::json value;
    NLOHMANN_DEFINE_TYPE_INTRUSIVE(Test, ptr, value);
};

// Type your code here, or load an example.
int main() {
    Test test{nlohmann::json::json_pointer("/test"), 10};
    nlohmann::json jtest = test;
    fmt::print("jtest={}\n", jtest.dump());
}

Error messages

example.cpp
<source>(19): error C2672: 'nlohmann::json_abi_v3_11_2::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::json_abi_v3_11_2::adl_serializer,std::vector<uint8_t,std::allocator<uint8_t>>>::get_to': no matching overloaded function found
C:/data/libraries/installed/x64-windows/include\nlohmann/json.hpp(1809): note: could be 'Array nlohmann::json_abi_v3_11_2::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::json_abi_v3_11_2::adl_serializer,std::vector<uint8_t,std::allocator<uint8_t>>>::get_to(T (&)[N]) noexcept(<expr>) const'
<source>(19): note: 'Array nlohmann::json_abi_v3_11_2::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::json_abi_v3_11_2::adl_serializer,std::vector<uint8_t,std::allocator<uint8_t>>>::get_to(T (&)[N]) noexcept(<expr>) const': could not deduce template argument for 'T (&)[N]' from 'nlohmann::json_abi_v3_11_2::json_pointer<StringType>'
        with
        [
            StringType=std::string
        ]
C:/data/libraries/installed/x64-windows/include\nlohmann/json.hpp(1809): note: see declaration of 'nlohmann::json_abi_v3_11_2::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::json_abi_v3_11_2::adl_serializer,std::vector<uint8_t,std::allocator<uint8_t>>>::get_to'
C:/data/libraries/installed/x64-windows/include\nlohmann/json.hpp(1798): note: or       'ValueType &nlohmann::json_abi_v3_11_2::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::json_abi_v3_11_2::adl_serializer,std::vector<uint8_t,std::allocator<uint8_t>>>::get_to(ValueType &) const'
<source>(19): note: 'ValueType &nlohmann::json_abi_v3_11_2::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::json_abi_v3_11_2::adl_serializer,std::vector<uint8_t,std::allocator<uint8_t>>>::get_to(ValueType &) const': could not deduce template argument for '__formal'
C:/data/libraries/installed/x64-windows/include\nlohmann/json.hpp(1798): note: see declaration of 'nlohmann::json_abi_v3_11_2::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::json_abi_v3_11_2::adl_serializer,std::vector<uint8_t,std::allocator<uint8_t>>>::get_to'
C:/data/libraries/installed/x64-windows/include\nlohmann/json.hpp(1785): note: or       'ValueType &nlohmann::json_abi_v3_11_2::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::json_abi_v3_11_2::adl_serializer,std::vector<uint8_t,std::allocator<uint8_t>>>::get_to(ValueType &) noexcept(<expr>) const'
<source>(19): note: 'ValueType &nlohmann::json_abi_v3_11_2::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::json_abi_v3_11_2::adl_serializer,std::vector<uint8_t,std::allocator<uint8_t>>>::get_to(ValueType &) noexcept(<expr>) const': could not deduce template argument for '__formal'
C:/data/libraries/installed/x64-windows/include\nlohmann/json.hpp(1785): note: see declaration of 'nlohmann::json_abi_v3_11_2::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::json_abi_v3_11_2::adl_serializer,std::vector<uint8_t,std::allocator<uint8_t>>>::get_to'
Compiler returned: 2

Compiler and operating system

MSVC v19.3 - VS 2022

Library version

3.11.2

Validation

Copy link

github-actions bot commented Feb 2, 2025

This issue has been marked as stale because it has been open for 90 days without activity. If this issue is still relevant, please add a comment or remove the "stale" label. Otherwise, it will be closed in 10 days. Thank you for helping us prioritize our work!

@github-actions github-actions bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Feb 2, 2025
@gregmarr
Copy link
Contributor

gregmarr commented Feb 2, 2025

The to_json and from_json functions need to be in the namespace of the type, not global functions unless the type is global. You are probably better off using the third party conversion option adl_serializer https://github.com/nlohmann/json?tab=readme-ov-file#arbitrary-types-conversions:~:text=I%20convert%20third%2D-,party,-types%3F

@risa2000
Copy link
Author

risa2000 commented Feb 2, 2025

@gregmarr
Right! A trivial overlook on my side :(.
If I declare the conversions in nlohmann namespace it works as expected (https://godbolt.org/z/K94hcz7ja):

#include <nlohmann/json.hpp>
#include <string>
#include <cstdio>

namespace nlohmann {
inline void to_json(json& j, const json::json_pointer& crj)
{
    j = json(crj.to_string());
}

inline void from_json(const json& cj, json::json_pointer& rj)
{
    rj = nlohmann::json::json_pointer(cj.get<std::string>());
}
}

struct Test {
    nlohmann::json::json_pointer ptr;
    nlohmann::json value;
    NLOHMANN_DEFINE_TYPE_INTRUSIVE(Test, ptr, value);
};

// Type your code here, or load an example.
int main() {
    Test test{.ptr = nlohmann::json::json_pointer("/test"), .value = 10};
    nlohmann::json jtest = test;
    printf("jtest=%s\n", jtest.dump().c_str());
}

The remaining question is if it would make sense to have these conversions in the library instead. json::json_pointer is a "native" library type. Writing a conversion function for this, while trivial, looks like something library could do as well.
What do you think?

@gregmarr
Copy link
Contributor

gregmarr commented Feb 2, 2025

I'm not sure it's obvious that everyone would want this represented in their JSON as a plain string.

@risa2000
Copy link
Author

risa2000 commented Feb 2, 2025

It is basically an analogy to std::filesystem::path. I am not sure there is any other (sane) way how to represent it in JSON as much as there is any other (sane) way how to represent std::filesystem::path. I give you the benefit of doubt though ;-).

@github-actions github-actions bot removed the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Feb 3, 2025
@gregmarr
Copy link
Contributor

gregmarr commented Feb 3, 2025

It's really @nlohmann's call. It might make sense for there to be a default here. The only reason I wasn't sure is that it could be reference to something in the JSON itself, and it would change based on where in the tree you did a dump(). You could pull a JSON file into a larger json object, and that could change the reference. That's probably a niche enough case that it would need special handling and the default of it being a string may be sufficient.

@risa2000
Copy link
Author

risa2000 commented Feb 3, 2025

@gregmarr I am not sure I am following you. For me json::json_pointer is an opaque structure, which may (in the same way as std::filesystem::path) have a semantical meaning (i.e. will be pointing to some valid sub JSON) or not (the "path" will not exist, will be invalid, etc.).
What do you mean by "it could be referencing to something in JSON itself"?

@gregmarr
Copy link
Contributor

gregmarr commented Feb 3, 2025

I mean you have a JSON file that has the object /a/b/1 which is pointed to by the json_pointer at /c/d/0, but then you load that file into an existing json object at /x/y, so now /a/b/1 is /x/y/a/b/1 so the pointer which is now at /x/y/c/d/0 is invalid.

    json j = { "x": { "q": "" } };
    j["x"]["y"] = json::parse(str);
    auto out = j.dump();

Again, I think this is something that can likely be ignored for the common case.

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

No branches or pull requests

2 participants