-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
Fixes #2730 #2731
Fixes #2730 #2731
Conversation
Added define JSON_VERSION_IS_PREDEFINED for skipping automatic JSON_HAS_CPP_* detection
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.
Please run make amalgamate
to make sure the single header is properly generated. The CI currently fails because this.
Yeah this sounds great!
But I would have to define the new define JSON_HAS_CPP_11
Is this ok?
Niels Lohmann ***@***.***> schrieb am Sa., 24. Apr. 2021,
12:34:
… ***@***.**** requested changes on this pull request.
------------------------------
In include/nlohmann/detail/macro_scope.hpp
<#2731 (comment)>:
> @@ -20,15 +20,18 @@
#endif
// C++ language standard detection
-#if (defined(__cplusplus) && __cplusplus >= 202002L) || (defined(_MSVC_LANG) && _MSVC_LANG >= 202002L)
- #define JSON_HAS_CPP_20
- #define JSON_HAS_CPP_17
- #define JSON_HAS_CPP_14
-#elif (defined(__cplusplus) && __cplusplus >= 201703L) || (defined(_HAS_CXX17) && _HAS_CXX17 == 1) // fix for issue #464
- #define JSON_HAS_CPP_17
- #define JSON_HAS_CPP_14
-#elif (defined(__cplusplus) && __cplusplus >= 201402L) || (defined(_HAS_CXX14) && _HAS_CXX14 == 1)
- #define JSON_HAS_CPP_14
+// if the user wants to manually specify the used c++ version this is skipped
+#ifndef JSON_VERSION_IS_PREDEFINED
How about replacing this with
#if !defined(JSON_HAS_CPP_20) && !defined(JSON_HAS_CPP_17) && !defined(JSON_HAS_CPP_14)
Wouldn't this have the same effect, but does not need an additional macro?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2731 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABK7ANYJILJLC7MGERGB6ZDTKKNE3ANCNFSM43J5X4BQ>
.
|
Oh right - I forgot this. Then I guess the best would be the following:
What do you think? |
Sounds great! Will do tomorrow!
Niels Lohmann ***@***.***> schrieb am Sa., 24. Apr. 2021,
14:09:
… Yeah this sounds great! But I would have to define the new define
JSON_HAS_CPP_11 Is this ok?
Oh right - I forgot this. Then I guess the best would be the following:
- Adjust the if to also check for JSON_HAS_CPP11.
- Define JSON_HAS_CPP11 if now other C++ standard has been detected.
This way, the (otherwise unused) variable is at least always set. This
allows to maybe make some compile-time decisions later on, but also to
"force" any C++ standard different to how it would have been detected.
What do you think?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2731 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABK7AN2R46W4BH2BT7Y6GDDTKKYHXANCNFSM43J5X4BQ>
.
|
JSON_VERSION_IS_PREDEFINED with JSON_HAS_CPP_11
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! |
Thank you very much for your support! |
Added define JSON_VERSION_IS_PREDEFINED
for skipping automatic JSON_HAS_CPP_* detection