Skip to content

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

Merged

Conversation

beccadax
Copy link
Contributor

@beccadax beccadax commented Apr 28, 2022

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.

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.
@beccadax beccadax requested review from xymus and porglezomp April 28, 2022 01:30
@beccadax
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@porglezomp porglezomp left a 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")
Copy link
Contributor

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.

Copy link
Contributor Author

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);
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

@xymus xymus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good!

@beccadax
Copy link
Contributor Author

@swift-ci smoke test and merge

@swift-ci swift-ci merged commit 645d73f into swiftlang:main May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants