Ambiguity in parsing nested maps

[quicknir]

This is considered ambiguous and a compile time error:

#include "json.hpp"

#include <iostream>
#include <unordered_map>

using json = nlohmann::json;

int main()
{
    std::cerr << "foo";
    try {
        auto j = json::parse(R"(
                         { "m": {"one": int}}
                         )");

        using T = std::unordered_map<std::string, std::unordered_map<std::string, int>>;

         auto m = j.at("m").get<T>();
    }
    catch (...) {}
    return 0;
}

The error is a bit long but I can paste it here if desired. I don't see why this is ambiguous, seems like given the input and the types I specified there is only one possible output. Is this considered a bug? Either way, is there a workaround?

[gregmarr]

Have you tried using std::map instead of std::unordered_map to see if that makes any difference? The conversion from std::map to std::unordered_map has seen problems at times.

The compile errors would not take into account the input string at all. I would, however, expect that program to fail at runtime, as your json is invalid (you need to replace int with an actual integer), and even when that's fixed, you're trying to extract {"one": 1} into a std::unordered_map<std::string, std::unordered_map<std::string, int>> when it's actually std::map<std::string, int>. You have too many layers of map there.

[quicknir]

I did try map, it didn't matter. Sorry about the int thing, yes, should fix that up to be 1. But I don't follow your "too many layers" observation. My string is (with integer replaced) { "m": {"one": 1}}. That seems to correspond quite well to an unordered_map<string, unordered_map<string, int>> ?

[gregmarr]

But you're calling get<> on m, which is {"one": 1}.

[quicknir]

hah, you're right, I should have just done auto m = j. But I suppose that would not affect the compilation error.

[gregmarr]

I'm not sure that the library can do nested retrieval like that without some help in the form of a from_json function. Can you post the errors?

[quicknir]

Sure, here is the error:

/foo/gcc/5.4.0/include/c++/5.4.0/bits/unordered_map.h:217:7: note: candidate: std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::unordered_map(std::initializer_list<typename std::_Hashtable<_Key, std::pair<const _Key, _Tp>, _Alloc, std::__detail::_Select1st, _Pred, _Hash, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<std::__not_<std::__and_<std::__is_fast_hash<_Hash>, std::__detail::__is_noexcept_hash<_Key, _Hash> > >::value, false, true> >::value_type>, std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::size_type, const hasher&, const key_equal&, const allocator_type&) [with _Key = std::basic_string<char>; _Tp = int; _Hash = std::hash<std::basic_string<char> >; _Pred = std::equal_to<std::basic_string<char> >; _Alloc = std::allocator<std::pair<const std::basic_string<char>, int> >; typename std::_Hashtable<_Key, std::pair<const _Key, _Tp>, _Alloc, std::__detail::_Select1st, _Pred, _Hash, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<std::__not_<std::__and_<std::__is_fast_hash<_Hash>, std::__detail::__is_noexcept_hash<_Key, _Hash> > >::value, false, true> >::value_type = std::pair<const std::basic_string<char>, int>; std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::size_type = long unsigned int; std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::hasher = std::hash<std::basic_string<char> >; std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::key_equal = std::equal_to<std::basic_string<char> >; std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::allocator_type = std::allocator<std::pair<const std::basic_string<char>, int> >]
       unordered_map(initializer_list<value_type> __l,
       ^
/foo/gcc/5.4.0/include/c++/5.4.0/bits/unordered_map.h:182:7: note: candidate: std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::unordered_map(const allocator_type&) [with _Key = std::basic_string<char>; _Tp = int; _Hash = std::hash<std::basic_string<char> >; _Pred = std::equal_to<std::basic_string<char> >; _Alloc = std::allocator<std::pair<const std::basic_string<char>, int> >; std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::allocator_type = std::allocator<std::pair<const std::basic_string<char>, int> >]
       unordered_map(const allocator_type& __a)
       ^
/foo/gcc/5.4.0/include/c++/5.4.0/bits/unordered_map.h:175:7: note: candidate: std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::unordered_map(std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>&&) [with _Key = std::basic_string<char>; _Tp = int; _Hash = std::hash<std::basic_string<char> >; _Pred = std::equal_to<std::basic_string<char> >; _Alloc = std::allocator<std::pair<const std::basic_string<char>, int> >]
       unordered_map(unordered_map&&) = default;
       ^
/foo/gcc/5.4.0/include/c++/5.4.0/bits/unordered_map.h:172:7: note: candidate: std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::unordered_map(const std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>&) [with _Key = std::basic_string<char>; _Tp = int; _Hash = std::hash<std::basic_string<char> >; _Pred = std::equal_to<std::basic_string<char> >; _Alloc = std::allocator<std::pair<const std::basic_string<char>, int> >]
       unordered_map(const unordered_map&) = default;
       ^
/foo/gcc/5.4.0/include/c++/5.4.0/bits/unordered_map.h:142:7: note: candidate: std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::unordered_map(std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::size_type, const hasher&, const key_equal&, const allocator_type&) [with _Key = std::basic_string<char>; _Tp = int; _Hash = std::hash<std::basic_string<char> >; _Pred = std::equal_to<std::basic_string<char> >; _Alloc = std::allocator<std::pair<const std::basic_string<char>, int> >; std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::size_type = long unsigned int; std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::hasher = std::hash<std::basic_string<char> >; std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::key_equal = std::equal_to<std::basic_string<char> >; std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::allocator_type = std::allocator<std::pair<const std::basic_string<char>, int> >]
       unordered_map(size_type __n,

I mean, the maps should have their own from_json and I don't see why this case shouldn't be handled.

[quicknir]

Here is also the part of the error backtrace from nlohman:

/foo/json/2.1.1.1/include/nlohmann/json.hpp:784:9:   required from 'void nlohmann::detail::from_json(const BasicJsonType&, CompatibleObjectType&) [with BasicJsonType = nlohmann::basic_json<>; CompatibleObjectType = std::unordered_map<std::basic_string<char>, std::unordered_map<std::basic_string<char>, int> >; typename std::enable_if<nlohmann::detail::is_compatible_object_type<BasicJsonType, CompatibleObjectType>::value, int>::type <anonymous> = 0]'
/foo/json/2.1.1.1/include/nlohmann/json.hpp:864:25:   required from 'decltype ((nlohmann::detail::from_json(j, val), void())) nlohmann::detail::from_json_fn::call(const BasicJsonType&, T&, nlohmann::detail::priority_tag<1u>) const [with BasicJsonType = nlohmann::basic_json<>; T = std::unordered_map<std::basic_string<char>, std::unordered_map<std::basic_string<char>, int> >; decltype ((nlohmann::detail::from_json(j, val), void())) = void]'
/foo/json/2.1.1.1/include/nlohmann/json.hpp:879:47:   required from 'void nlohmann::detail::from_json_fn::operator()(const BasicJsonType&, T&) const [with BasicJsonType = nlohmann::basic_json<>; T = std::unordered_map<std::basic_string<char>, std::unordered_map<std::basic_string<char>, int> >]'
/foo/json/2.1.1.1/include/nlohmann/json.hpp:926:30:   required from 'static void nlohmann::adl_serializer< <template-parameter-1-1>, <template-parameter-1-2> >::from_json(BasicJsonType&&, ValueType&) [with BasicJsonType = const nlohmann::basic_json<>&; ValueType = std::unordered_map<std::basic_string<char>, std::unordered_map<std::basic_string<char>, int> >; <template-parameter-1-1> = std::unordered_map<std::basic_string<char>, std::unordered_map<std::basic_string<char>, int> >; <template-parameter-1-2> = void]'
/foo/json/2.1.1.1/include/nlohmann/json.hpp:3237:45:   required from 'ValueType nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::get() const [with ValueTypeCV = std::unordered_map<std::basic_string<char>, std::unordered_map<std::basic_string<char>, int> >; ValueType = std::unordered_map<std::basic_string<char>, std::unordered_map<std::basic_string<char>, int> >; typename std::enable_if<(((! std::is_same<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>, ValueType>::value) && nlohmann::detail::has_from_json<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>, ValueType>::value) && (! nlohmann::detail::has_non_default_from_json<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>, ValueType>::value)), int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; JSONSerializer = nlohmann::adl_serializer]'

[gregmarr]

This doesn't line up with the head of devel. Which branch and commit are you using?

I'm guessing this is the line:

    j.m_value.object = j.template create<typename BasicJsonType::object_t>(begin(obj), end(obj));

This means that it's trying to create the unordered map using its range constructors, none of which match up. This will likely require that you implement nlohmann::from_json(const json &j, <your map type> &v). I think a couple other people have posted some from_json implementations for their own desired types in other issues. If there's nothing there, then maybe @theodelrieu can do some of his magic here.

[quicknir]

@gregmarr It's not really a solution to implement from_json, because it's a recursive problem. I could implemented for map<string, map<string, int>> but then of course it could go one level deeper. Nested vectors for example do not have this problem.

It seems to me like this is a defect in the library, in the sense that there's no reason why the library couldn't do this correctly. I'll try to dig a bit more when I have a chance. If we know which constructor we want there should be a way to resolve the ambiguity.

[gregmarr]

It could be possible to make the library do that, but I'm not sure it can be considered a defect that it doesn't have the ability to extract a particular type without writing a from_json() function for that type. That's something that @theodelrieu and @nlohmann will need to address.

Note that this is not an ambiguity in parsing, as the title says, but rather in extracting the data to your desired data type.

Maybe it's time to take a step back. Why are you trying to extract the data into that data type? What are you going to do with that data after you copy all of it into those maps? Could that be better done just by working with the json object directly, and avoiding all the copying?

[quicknir]

I think we disagree somewhat on the library's responsibilities. The library already does this to a degree: if you create a type Foo and write a from_json for it, then you can immediately extract unordered_map<string, Foo>, and vector<Foo>. Why don't I need to write a from_json for vector<Foo>? Well, it's pretty clear that this shouldn't be necessary. If the library knows how to extract Foo, it should automatically know how to extract a vector<Foo>. The library could in principle wash its hands of this, but then the user would have to write out a lot of boilerplate, and it would significantly diminish the value of the library for no good reason.

Applying this logic to unordered_map<string, int>, which the library knows how to extract, immediately leads us to the conclusion that the library should know how to extract unordered_map<string, unordered_map<string, int>. So, I do quite strongly feel that this is a defect.

I don't want to work directly with the json because a) it's much more weakly typed than the nested map, and I want to verify the type precisely at the config parsing stage, and use the data at a later stage knowing that the types have at least been verified, and b) I don't want to introduce dependencies on the json library deep into my code. I want to restrict the usage to simply parsing out json into useful structures, and then process those structures. I don't really want to focus on this though, because you may disagree but I think these are legitimate reasons, and in any case obviously these decisions are being made in the context of a broad code base that I'm not going to get into.

[nlohmann]

I'm late to this discussion. Do we have a small example that demonstrates the problem with the develop version? Just to make sure: does the issue still exist when all unordered_map instances are replaced with map?

[quicknir]

When I tried it I had the same issue with map. I can't do it easily now but I can try to pull to HEAD when I get home and duplicate it with map. I expect though that if you try the original example and simply replace unordered_map with map, you will see the issue.

[nlohmann]

Thanks, that would be great.

[gregmarr]

I don't want to work directly with the json because a) it's much more weakly typed than the nested map, and I want to verify the type precisely at the config parsing stage, and use the data at a later stage knowing that the types have at least been verified, and b) I don't want to introduce dependencies on the json library deep into my code. I want to restrict the usage to simply parsing out json into useful structures, and then process those structures.

That makes sense, thanks.

I imagine that if @theodelrieu or @nlohmann can find a way to make this work easily, that it will get added.

[gregmarr]

FYI, this compiles just fine with the latest code on the develop branch.

https://godbolt.org/g/CzKxNj

[quicknir]

@gregmarr Ah great stuff! At first maybe I thought it was the compiler but it doesn't seem like it. I will verify at home, in which case I will be sure to close this. Maybe at that point I can bisect a little and see where exactly this was fixed.

[nlohmann]

@quicknir Any news on this?

[quicknir]

Sorry I dropped this, will verify tonight.

[quicknir]

Confirmed this works on HEAD, and pretty sure it didn't in 2.1.1. Just curious if there is a release coming soon? If not I'll probably just grab HEAD. Thanks for your help!