-
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
Generate template functions with NLOHMANN_DEFINE_TYPE macros #4597
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
Generate template functions with NLOHMANN_DEFINE_TYPE macros #4597
Conversation
9f84443 to
b22b43a
Compare
|
On second thought, I can use |
|
(I'm only commenting on the CI issues right now: Clang-Tidy should be fixable by just adding some |
|
The coverage job is now fixed in the |
749b983 to
43c41f0
Compare
Signed-off-by: kimci86 <kimci86@hotmail.fr>
Signed-off-by: kimci86 <kimci86@hotmail.fr>
Signed-off-by: kimci86 <kimci86@hotmail.fr>
Signed-off-by: kimci86 <kimci86@hotmail.fr>
Signed-off-by: kimci86 <kimci86@hotmail.fr>
Signed-off-by: kimci86 <kimci86@hotmail.fr>
Signed-off-by: kimci86 <kimci86@hotmail.fr>
Signed-off-by: kimci86 <kimci86@hotmail.fr>
Signed-off-by: kimci86 <kimci86@hotmail.fr>
43c41f0 to
2a90012
Compare
|
Rebased and fixed conflicts. |
Signed-off-by: kimci86 <kimci86@hotmail.fr>
Signed-off-by: kimci86 <kimci86@hotmail.fr>
Yes, I think using |
Signed-off-by: kimci86 <kimci86@hotmail.fr>
Signed-off-by: kimci86 <kimci86@hotmail.fr>
13d2d07 to
a6d937c
Compare
|
Added type constraint via SFINAE. Not sure it is relevant to show this kind of ugly type in documentation too, but at least it describes accurately what happens. |
|
No, don't show this in the documentation. |
a6d937c to
5d3d8fa
Compare
|
Ok I dropped a6d937c then. |
This comment was marked as resolved.
This comment was marked as resolved.
Signed-off-by: kimci86 <kimci86@hotmail.fr>
Sorry I am not sure to understand where I should add such documentation. |
You are right - the macros have no template parameter. My bad. |
nlohmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
|
Thanks! |
Changes:
NLOHMANN_DEFINE_TYPE_*generate template functions. This allows to use any specialization ofnlohmann::basic_jsonsuch asnlohmann::ordered_json, instead ofnlohmann::jsononly.Existing issues:
Related PRs:
Reviews on related PRs have raised the following concerns:
to_json/from_jsonbecoming a template change overload resolution is some twisted cases.BasicJsonTypeis not restricted tobasic_jsoninstances.To be noted:
NLOHMANN_JSON_SERIALIZE_ENUMalready uses that same design of unrestrictedBasicJsonTypetemplate parameter.If we want to restrict it, I see two options:
is_basic_json.Currently
include/nlohmann/detail/meta/type_traits.hppincludesinclude/nlohmann/detail/macro_scope.hppso we cannot useis_basic_jsonin macros.#undefNLOHMANN_BASIC_JSON_TPL_DECLARATIONandNLOHMANN_BASIC_JSON_TPLinmacro_unscope.hppso that we can use them in macros to effectively write the fullbasic_jsontemplate type.What do you think?
Thank you for this awesome library!
Pull request checklist
Read the Contribution Guidelines for detailed information.
include/nlohmanndirectory, runmake amalgamateto create the single-header filessingle_include/nlohmann/json.hppandsingle_include/nlohmann/json_fwd.hpp. The whole process is described here.