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

Unordered map json #165

Closed
wants to merge 3 commits into from
Closed

Unordered map json #165

wants to merge 3 commits into from

Conversation

erichkeane
Copy link

Work for unordered map in the json object. See #164 for the bug report and description of the solution.

Erich Keane added 2 commits December 30, 2015 11:12
NOTE: This does NOT compile due to unordered_map needing to have an
instance in the CompatibleObjectType CTOR for the basic_json object, and
basic_json being an incomplete type at that time.  For assistance
request only.
the unordered_map template cannot be instanced with an incomplete type
so this patch replaces all places where this is made necessary with
alternative though equivilent types.
@erichkeane
Copy link
Author

Looks like this doesn't build on MSVC... Perhaps I have to see if I can mess with a few other things in order to get it working. It might take a while for me to spin a VM, but I'll push a fix ASAP.

Key_compare is implemented in the std::unordered_map in MSVC, so the
result is the initial attempt to use SFINAE to select 1 or the other
wasn't effective.  This patch makes the 'default' case implement the
std::map case so that key_compare isn't used for SFINAE testing.
@erichkeane
Copy link
Author

I seem to have managed to cause the same issue you see in #144

@erichkeane
Copy link
Author

As mentioned in #144, I checked master out directly, and reproduce this same ambigious conversion issue on origin/master!

It seems that there is a problem running one of the tests (CHECK(*p2 == value.get<test_type>());, due to os << test_type(json::object_t); being ambigious.

I'm going to open a new bug for this one.

@nlohmann
Copy link
Owner

The current PR is #209. I close this one.

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

Successfully merging this pull request may close these issues.

2 participants