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

Enable the latest C++ standard with Visual Studio #1287

Closed
naszta opened this issue Oct 9, 2018 · 11 comments
Closed

Enable the latest C++ standard with Visual Studio #1287

naszta opened this issue Oct 9, 2018 · 11 comments
Labels
platform: visual studio related to MSVC state: please discuss please discuss the issue or vote for your favorite option

Comments

@naszta
Copy link
Contributor

naszta commented Oct 9, 2018

If the following flag would be added to MSVC part, the MSVC C++14 and C++17 features could be used in the code (including std::string_view and variadic template std::less).

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /std:c++latest")

I'm happy to write the code (opt-out as the latest VS runs properly with all tests passed).

@nlohmann nlohmann added the platform: visual studio related to MSVC label Oct 9, 2018
@nlohmann
Copy link
Owner

nlohmann commented Oct 9, 2018

Is it good style to set these flags from the project? Wouldn't this be something the user sets?

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Oct 9, 2018
@naszta
Copy link
Contributor Author

naszta commented Oct 9, 2018

In visual studio project I normally specify explicitly /std:c++14 or /std:c++17 for an application. But for tests or libs (when both VS2015 and VS2017 should be supported) I definitely would use the latest flag.

Reference

@nlohmann
Copy link
Owner

nlohmann commented Oct 9, 2018

I'm not using MSVC myself, that's why I would like to discuss whether it is best practice to set such switches.

@gregmarr
Copy link
Contributor

gregmarr commented Oct 9, 2018

What features are you looking to use, and how would you make sure that the library is still usable with only C++11?

@naszta
Copy link
Contributor Author

naszta commented Oct 9, 2018

There are standard switch macros in the code (JSON_HAS_CPP_17 and JSON_HAS_CPP_14). They are set accordingly on used standard by the compiler (in VS case _HAS_CXX17 and _HAS_CXX14). With this flag always the latest standard could be used what supported by VS. (In current code variadic std::less and std::string_ref are protected by these macros.)

@nlohmann
Copy link
Owner

I still have not understood why this library should set the C++ version when this is usually done by the code that actually uses the library. Do I miss something between CMake and MSVC?

@naszta
Copy link
Contributor Author

naszta commented Oct 16, 2018

No: my idea here is using the latest supported C++ in MSVC even in tests: MSVC is not a major platform for you and in this way you will have less macros for VS (as it uses the latest standard what it could handle).

@nlohmann
Copy link
Owner

I am already testing with these flags on AppVeyor. And if I would remove macros, wouldn't this break support for older MSVC versions?

@naszta
Copy link
Contributor Author

naszta commented Oct 22, 2018

The flag is supported since Visual C++ 2015 Update 3. If older VS versions are supported in tests, it shouldn't be introduced. Otherwise it's safe to go.

@nlohmann
Copy link
Owner

I still have not understood the benefit of enforcing this flag to projects using the library. FWIW, we use this AppVeyor configuration, where we additionally check /permissive- /std:c++latest /utf-8 for MSVC 2017.

@naszta
Copy link
Contributor Author

naszta commented Oct 23, 2018

Ok, nevermind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: visual studio related to MSVC state: please discuss please discuss the issue or vote for your favorite option
Projects
None yet
Development

No branches or pull requests

3 participants