-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fix MSVC++ command line warnings and support RELWITHDEBINFO / MINSIZEREL builds #207
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
Conversation
This change looks for CMake's default setting for MSVC++, /W3 (and any other level should that change in the future), and removes it before adding /W4. This stops the build for MSVC++ emitting warnings about /W4 overriding /W3 earlier on the command line.
BENCHMARK_ENABLE_LTO=true was completely replacing CMAKE_CXX_FLAGS_RELEASE; meaning neither CMake's release defaults nor user customizations were being applied.
In addition to release, CMake supports RELWITHDEBINFO and MINSIZEREL build configurations. In particular, debug info is necessary for many profilers to do anything useful, making RELWITHDEBINFO important here. MINSIZEREL was added for completeness' sake.
| set(CMAKE_SHARED_LINKER_FLAGS_RELEASE "${CMAKE_SHARED_LINKER_FLAGS_RELEASE} /LTCG") | ||
| set(CMAKE_EXE_LINKER_FLAGS_RELEASE "${CMAKE_EXE_LINKER_FLAGS_RELEASE} /LTCG") | ||
|
|
||
| set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} /GL") |
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.
If there are better ways to do this rather than spamming this out I'm all ears :/
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.
i have no idea.. i haven't built with MSVC in a looong time.
maybe @anton-dirac knows
MSVC++ before 2015 Update 2 has a bug in sleep_for where it tries to implicitly += the input with a nanoseconds variable. Work around this by using nanoseconds directly (which can be implicitly +='d with chrono::nanoseconds).
|
Sorry, I messed this up somehow. Please ignore for now. |
No description provided.