add new is_constructible_* traits used in from_json #1301
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
is_compatible_* traits were used in from_json, but it made no sense whatsoever. It used to work because of non-SFINAE correctness + json_ref unconstrained variadic template constructor. SFINAE checks are becoming quite complex, we need a specification of some sort describing: * which concepts the library uses * how the conversion to/from json works in detail Having such a specification would really help simplifying the current code (as well as having meaningful checks). Fixes !1299
|
I can only trust the code - I can't provide a thorough review right now. |
jaredgrubb
reviewed
Oct 16, 2018
|
|
||
| // This trait checks if JSONSerializer<T>::from_json(json const&) exists | ||
| // this overload is used for non-default-constructible user-defined-types | ||
| template <typename BasicJsonType, typename T, typename = void> |
There was a problem hiding this comment.
Maybe add a comment on whether or not 'T' is expected to be a concrete type (string) or is allowed to be a CV-qualified type (string const&). It's not clear if these need std::decay's in there or whether the layer above it should add them?
There was a problem hiding this comment.
IIRC, the only places where CV-qualified types are specified in the current code is in is_detected related code.
I can add a comment at the top of the file.
|
I'll open an issue in the next few days to explain in detail what I meant in the commit message. I hope it will be a lot clearer then :) |
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
is_compatible_* traits were used in from_json, but it made no sense
whatsoever, since they check if a type can be converted TO json.
It used to work because of non-SFINAE correctness + json_ref
unconstrained variadic template constructor.
SFINAE checks are becoming quite complex, we need a specification of
some sort describing:
Having such a specification would really help simplifying the current
code (as well as having meaningful checks).
Fixes #1299
Pull request checklist
Read the Contribution Guidelines for detailed information.
include/nlohmanndirectory, runmake amalgamateto create the single-header filesingle_include/nlohmann/json.hpp. The whole process is described here.