[cmake] Use generator expressions to change conditionalize target_compile_definition that are for c/c++ only be used for c/c++ #37510
Conversation
This file contains hidden or 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
gottesmm
commented
May 19, 2021
cmake/modules/AddSwift.cmake
Outdated
| else() | ||
| target_compile_definitions(${target} PRIVATE -DNDEBUG) | ||
| target_compile_options(${target} PRIVATE $<$<COMPILE_LANGUAGE:C,CXX,OBJC,OBJCXX>:-DNDEBUG>) |
There was a problem hiding this comment.
I by mistake changed this to definitions. I am going to change this back before it goes in. (I think I did it in a few more places as well).
There was a problem hiding this comment.
I looked through the commit and these were the only two that I found.
There was a problem hiding this comment.
This technically changes the behaviour - it changes ordering to be precise. I think that using target_compile_definitions is better, but Im not opposed to this change.
gottesmm
commented
May 19, 2021
compnerd
approved these changes
May 19, 2021
cmake/modules/AddSwift.cmake
Outdated
| else() | ||
| target_compile_definitions(${target} PRIVATE -DNDEBUG) | ||
| target_compile_options(${target} PRIVATE $<$<COMPILE_LANGUAGE:C,CXX,OBJC,OBJCXX>:-DNDEBUG>) |
There was a problem hiding this comment.
This technically changes the behaviour - it changes ordering to be precise. I think that using target_compile_definitions is better, but Im not opposed to this change.
…pile_definition that are for c/c++ only be used for c/c++ I am in the process now of preparing the tree for the addition of Swift code in the optimizer as a normal source of source file. The goal is to make it so that one can just include a random swift library anywhere in the swift project host build and the cmake will use the swift compiler from the host toolchain/compile-link the code just as if it was a normal host side thing (1). In order to do this though, we need to deal with the legacy of our cmake creating compile flags without constraining the flags to only being used if cmake is compiling c/c++ code. To fix this, I just inserted generator expressions into the host side swift build's cmake that uses generator expressions to perform such conditionalization. The result is that the parts of a target that are c/c++ will get these flags, but these flags will not propagate to any Swift files that we add. (1) With time this implies that we will need to be able to bootstrap the swift compiler. We are not crossing that bridge now since the only places that we are going to use this today is in the SILOptimizer optimier passes. These can always be disabled while cross compiling, meaning that we can make progress here without having the bootstrapping completely ironed out.
gottesmm
force-pushed
the
pr-af6b60d3b83155f33bb1d91e497422748ddcc917
branch
from
dd30a17 to
558c9d4
Compare
gottesmm
commented
May 19, 2021
|
@swift-ci smoke test |
|
should be NFC |
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.
I am in the process now of preparing the tree for the addition of Swift code in
the optimizer as a normal source of source file. The goal is to make it so that
one can just include a random swift library anywhere in the swift project host
build and the cmake will use the swift compiler from the host
toolchain/compile-link the code just as if it was a normal host side thing (1).
In order to do this though, we need to deal with the legacy of our cmake
creating compile flags without constraining the flags to only being used if
cmake is compiling c/c++ code. To fix this, I just inserted generator
expressions into the host side swift build's cmake that uses generator
expressions to perform such conditionalization. The result is that the parts of
a target that are c/c++ will get these flags, but these flags will not propagate
to any Swift files that we add.
(1) With time this implies that we will need to be able to bootstrap the swift
compiler. We are not crossing that bridge now since the only places that we are
going to use this today is in the SILOptimizer optimier passes. These can always
be disabled while cross compiling, meaning that we can make progress here
without having the bootstrapping completely ironed out.