build: Rework setting of compiler flags #596
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
✅ Deploy Preview for dpp-dev ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
This commit uses the CMake variables CMAKE_CXX_FLAGS_DEBUG and CMAKE_CXX_FLAGS_RELEASE to handle setting varying compiler flags depending on the build target. This gets rid of a bunch of if-else blocks, allowing easier to read code. Furthermore, common flags are put in CMAKE_CXX_FLAGS only once to reduce code duplication. Issue: brainboxdotcc#595
-std=c++17 is automatically set on demand by target_compile_features. -fPIC is set by default for SHARED and MODULE targets and can be enabled by target properties for STATIC targets. Issue: brainboxdotcc#595
CMake provides various build types which already set the correct -O levels and -g depending on the user's choice. This commit gets rid of the previous explicit setting of these flags where possible. Issue: brainboxdotcc#595
C++17 is required when building against DPP. As such, it should be a PUBLIC compile feature.
NexAdn
force-pushed
the
cxxflags-rework
branch
from
9ab683d to
373d694
Compare
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.
This PR reworks how compiler flags are set, removing code duplication and unnecessary if-else blocks. This should reduce the complexity of the CMakeLists.txt and allows for users and packagers to append to the compiler flags as needed or desired (e.g. reducing the optimization level on a system where DPP might behave differently with -O3) and use the default build types to obtain the desired results (i.e. use Release and MinSizeRel build types instead of specifying
-DBUILD_O3=true/false).I've did a short successful test for the build types Debug, Release and MinSizeRel on Linux to ensure the flags are still set to the desired values and that the library compiles.
Debug:
Release:
MinSizeRel:
Note that parts of the Windows toolchain have been adjusted as well. However, I am unable to test this, since I don't have a Windows machine. We could still drop this if desired, but maybe we can find someone to test the Windows part as well.I've dropped the Windows stuff since the CI pipeline broke for some Windows builds.Closes #595