-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Update SWIFT_COMPILER_VERSION language features #58480
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
Update SWIFT_COMPILER_VERSION language features #58480
Conversation
The `SWIFT_COMPILER_VERSION` define is used to stamp a vendor’s version number into a Swift compiler binary. It can be queried from Swift code using `#if _compiler_version` and from Clang by using a preprocessor definition called `__SWIFT_COMPILER_VERSION`. These are unsupported compiler-internal features used primarily by Apple Swift. In Swift 1.0 through 5.5, Apple Swift used a scheme for `SWIFT_COMPILER_VERSION` where the major version matched the embedded clang (e.g. 1300 for Apple Clang 13.0.0) and the minor version was ignored. Starting in Swift 5.6, Apple Swift started using major and minor version numbers that matched the Swift.org version number. This makes them easier to understand, but it means that version 1300.0.x was followed by version 5.6.x. Not only did version numbers go backwards, but also the old logic to ignore minor versions was now a liability, because it meant you would not be able to target a change to 5.7.x compilers but not 5.6.x compilers. This commit addresses the problem by: * Modifying the existing `#if _compiler_version(string-literal)` feature so it transforms the major version into a major and minor that will compare correctly to new version numbers. For instance, “1300.*” is transformed into “1.300”, which will compare correctly to a “5.6” or “5.7” version even if it doesn’t really capture the fact that “1300” was a Swift 5.5 compiler. As a bonus, this allows you to use the feature to backwards-compatibly test new compilers using the existing feature: “5007.*” will be seen by compilers before 5.7 as an unknown future version, but will be seen by 5.7 compilers as targeting them. * Modifying the `__SWIFT_COMPILER_VERSION` clang define similarly so that, to preprocessor conditions written for the old scheme, a 5.7 compiler will appear to have major version 5007. * Adding a new variant of `#if _compiler_version` with the same syntax as `#if swift` and `#if compiler`—that is, taking a comparison operator and a bare set of dotted version numbers, rather than a string literal. Going forward, this will be how version checks are written once compatibility with compilers before this change is no longer a concern. These changes are only lightly tested because tests have to work without any compiler version defined (the default in most configurations), but I’ve tested what I can. Fixes rdar://89841295.
@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'm not super familiar with most of the compiler code, so my approval on the implementation is a bit un-confident—the design of the normalization and diagnostics looks good to me. 👍🏻
Noted a test change you probably want to make.
@@ -7,7 +7,7 @@ | |||
asdf asdf asdf asdf | |||
#endif | |||
|
|||
#if _compiler_version("10.*.10.10") | |||
#if _compiler_version("600.*.10.10") |
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.
Grepped for low version numbers myself—it looks like there's also 3 instances of _compiler_version("4.*.0")
in test/Parse/ConditionalCompilation/identifierName.swift
that you might want to update.
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.
_compiler_version("4.*.0")
will still actually work correctly; it's just turned into a really silly example, since it's basically testing for compiler version 0.004
.I'm going to leave it that way in the interest of expedience.
auto currentVersion = version::Version::getCurrentCompilerVersion(); | ||
EXPECT_GE(CV("700"), currentVersion); | ||
EXPECT_GE(CV("1300"), currentVersion); | ||
EXPECT_GE(CV("5007"), currentVersion); |
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.
Is it useful to have a test for V("5.7")
here or is that trivially true enough that we don't care.
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 think we have adequate coverage elsewhere:
- The test case at test/Parse/ConditionalCompilation/compiler_version.swift:54 demonstrates that if there is a minor version present, we will warn that this version check is ill-formed and should be rewritten as
5007.*
. This is honestly more important than any specific mapping behavior for ill-formed version checks. - The
CV("700"), V("0.700")
unit test demonstrates that major versions > 1000 map to 0.x. - The
CV("5007"), V("5.7")
unit test demonstrates that the lowest digit of the major version is mapped to the lowest digit of the minor version.
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 looks good!
@swift-ci smoke test and merge |
The
SWIFT_COMPILER_VERSION
define is used to stamp a vendor’s version number into a Swift compiler binary. It can be queried from Swift code using#if _compiler_version
and from Clang by using a preprocessor definition called__SWIFT_COMPILER_VERSION
. These are unsupported compiler-internal features used (AFAIK) primarily by Apple Swift.In Swift 1.0 through 5.5, Apple Swift used a scheme for
SWIFT_COMPILER_VERSION
where the major version matched the embedded clang (e.g. 1300 for Apple Clang 13.0.0) and the minor version was ignored. Starting in Swift 5.6, Apple Swift started using major and minor version numbers that matched the Swift.org version number. This makes them easier to understand, but it means that version 1300.0.x was followed by version 5.6.x. Not only did version numbers go backwards, but also the old logic to ignore minor versions was now a liability, because it meant you would not be able to target a change to 5.7.x compilers but not 5.6.x compilers.This commit addresses the problem by:
Modifying the existing
#if _compiler_version(string-literal)
feature so it transforms the major version into a major and minor that will compare correctly to new version numbers. For instance, “1300.*” is transformed into “1.300”, which will compare correctly to a “5.6” or “5.7” version even if it doesn’t really capture the fact that “1300” was a Swift 5.5 compiler. As a bonus, this allows you to use the feature to backwards-compatibly test new compilers using the existing feature: “5007.*” will be seen by compilers before 5.7 as an unknown future version, but will be seen by 5.7 compilers as targeting them.Modifying the
__SWIFT_COMPILER_VERSION
clang define similarly so that, to preprocessor conditions written for the old scheme, a 5.7 compiler will appear to have major version 5007.Adding a new variant of
#if _compiler_version
with the same syntax as#if swift
and#if compiler
—that is, taking a comparison operator and a bare set of dotted version numbers, rather than a string literal. Going forward, this will be how version checks are written once compatibility with compilers before this change is no longer a concern.These changes are only lightly tested because tests have to work without any compiler version defined (the default in most configurations), but I’ve tested what I can.
Fixes rdar://89841295.