Ban building runtimes using compilers without SwiftCC/SwiftAsyncCC #64773
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
|
@swift-ci please test |
al45tair
approved these changes
Mar 30, 2023
tbkka
approved these changes
Mar 30, 2023
zoecarver
reviewed
Mar 30, 2023
Comment on lines
-211
to
-214
| #define SWIFT_CC_swift | ||
| #define SWIFT_CONTEXT | ||
| #define SWIFT_ERROR_RESULT | ||
| #define SWIFT_INDIRECT_RESULT |
zoecarver
approved these changes
Mar 30, 2023
compnerd
reviewed
Mar 30, 2023
etcwilde
force-pushed
the
ewilde/dont-go-alone-use-this
branch
2 times, most recently
from
c840634
to
61e8229
Compare
|
@swift-ci please test |
compnerd
approved these changes
Mar 30, 2023
mikeash
approved these changes
Mar 30, 2023
|
Linux failure: Windows failure: |
MaxDesiatov
approved these changes
Mar 30, 2023
etcwilde
force-pushed
the
ewilde/dont-go-alone-use-this
branch
from
61e8229
to
e0fa2b7
Compare
|
@swift-ci please test |
|
Windows failure: |
|
The error in |
|
I think the best we can do is just refrain from defining the macros, which will fail to compile if people try to use them in contexts where the C/C++ compiler doesn't have the necessary calling conventions. |
|
Does |
No, unfortunately it doesn't expand nicely. I'm pretty sure you're not supposed to have multiple layers of recursive macros. For this program Clang says: GCC says: MSVC says: And lastly ICC: Clang and GCC are almost tolerable if people read diagnostic notes, but MSVC is pretty bad, unfortunately. |
etcwilde
force-pushed
the
ewilde/dont-go-alone-use-this
branch
from
e0fa2b7
to
b5da077
Compare
|
@swift-ci please test |
Building the runtime with a compiler that doesn't understand swift
calling conventions is playing with fire, and not a fun fire because the
Swift compiler will emit calls expecting swift calling conventions,
resulting in an ABI mismatch and lots of pain. This patch stops us from
defining the offending macros in cases where the C/C++ compiler building
the runtime doesn't understand Swift calling conventions, so attempted
usage will result in a hard build failure.
Unfortunately, the runtime-inspection library includes this header, but
is part of the toolchain build, which should be buildable with the host
C/C++ compiler (msvc, gcc, etc...), so I can't outright `#error` the
failing clause or it will cause that library to fail to build.
Sadly, unknown attributes are either handled as soft warnings or ignored
altogether, which is not a great situation when building a broken ABI.
We could use `_Pragma("clang diagnostic error
\"-Wattribute-ignored\"")`, but that only works for clang, where my main
target is gcc. I've gone with not defining the macro in cases where the
compiler doesn't understand the calling conventions so that it is a hard
error at the point of use, and emitting a warning saying that we're not
defining the macro because the compiler doesn't understand the calling
convention to explain why.
I've done the same with the swift async calling convention, removing the
fallback on the Swift CC. Windows added swift async calling convention
support in apple/llvm-project commit
e5198240d982c635129cf1a2ae2f2251747a981d, and apple/swift commit
4f6a054.
etcwilde
force-pushed
the
ewilde/dont-go-alone-use-this
branch
from
b5da077
to
ac4a5d6
Compare
|
@swift-ci please test |
|
Linux failure: |
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.
Building the runtime with a compiler that doesn't understand swift calling conventions is playing with fire, and not a fun fire. Also, swapping out swift async calling conventions with normal swift CC is not a great idea either. Emit an error message saying stopping folks from burning themselves.
Folks keep seeing this and thinking that they can build without the right compiler and arguing with me about not being able to use a random compiler to build the runtime "because it's just normal C++". The Swift async calling conventions are different enough that just falling back on synchronous calling conventions (especially without telling the Swift compiler) is a risky endeavor, if it works at all.