-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
Ban building runtimes using compilers without SwiftCC/SwiftAsyncCC #64773
base: main
Are you sure you want to change the base?
Conversation
@swift-ci please test |
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 raised my eyebrows when I originally saw those #else
cases, but assumed someone must think it was OK. #error
does seem safer.
#define SWIFT_CC_swift | ||
#define SWIFT_CONTEXT | ||
#define SWIFT_ERROR_RESULT | ||
#define SWIFT_INDIRECT_RESULT |
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.
This is a bit scary...
c840634
to
61e8229
Compare
@swift-ci please test |
Linux failure:
Windows failure:
|
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. |
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.
b5da077
to
ac4a5d6
Compare
@swift-ci please test |
Linux failure:
|
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.