Merge pull request #58480 from beccadax/so-i-put-versions-in-your-version-numbers

Update SWIFT_COMPILER_VERSION language features

Author: swift-ci

include/swift/AST/DiagnosticsParse.def

@@ -1816,7 +1816,10 @@ ERROR(version_component_not_number,none,
 ERROR(compiler_version_too_many_components,none,
       "compiler version must not have more than five components", ())
 WARNING(unused_compiler_version_component,NoUsage,
-      "the second version component is not used for comparison", ())
+      "the second version component is not used for comparison in legacy "
+      "compiler versions%select{|; are you trying to encode a new Swift "
+      "compiler version for compatibility with legacy compilers?}0",
+      (bool))
 ERROR(empty_version_component,none,
       "found empty version component", ())
 ERROR(compiler_version_component_out_of_range,none,

include/swift/Basic/Version.h

@@ -129,7 +129,12 @@ class Version {
   /// Return this Version struct as the appropriate version string for APINotes.
   std::string asAPINotesVersionString() const;
 
-  /// Parse a version in the form used by the _compiler_version \#if condition.
+  /// Parse a version in the form used by the _compiler_version(string-literal)
+  /// \#if condition.
+  ///
+  /// \note This is \em only used for the string literal version, so it includes
+  /// backwards-compatibility logic to convert it to something that can be
+  /// compared with a modern SWIFT_COMPILER_VERSION.
   static Optional<Version> parseCompilerVersionString(StringRef VersionString,
                                                       SourceLoc Loc,
                                                       DiagnosticEngine *Diags);

lib/Basic/Version.cpp

@@ -16,6 +16,7 @@
 
 #include "clang/Basic/CharInfo.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "swift/AST/DiagnosticsParse.h"
@@ -132,9 +133,29 @@ Optional<Version> Version::parseCompilerVersionString(
     // The second version component isn't used for comparison.
     if (i == 1) {
       if (!SplitComponent.equals("*")) {
-        if (Diags)
-          Diags->diagnose(Range.Start, diag::unused_compiler_version_component)
-          .fixItReplaceChars(Range.Start, Range.End, "*");
+        if (Diags) {
+          // Majors 600-1300 were used for Swift 1.0-5.5 (based on clang
+          // versions), but then we reset the numbering based on Swift versions,
+          // so 5.6 had major 5. We assume that majors below 600 use the new
+          // scheme and equal/above it use the old scheme.
+          bool firstComponentLooksNew = CV.Components[0] < 600;
+
+          auto diag = Diags->diagnose(Range.Start,
+                                      diag::unused_compiler_version_component,
+                                      firstComponentLooksNew);
+
+          if (firstComponentLooksNew &&
+              !SplitComponent.getAsInteger(10, ComponentNumber)) {
+            // Fix-it version like "5.7.1.2.3" to "5007.*.1.2.3".
+            auto newDigits = llvm::formatv("{0}{1,0+3}.*", CV.Components[0],
+                                           ComponentNumber).str();
+            diag.fixItReplaceChars(SplitComponents[0].second.Start,
+                                   Range.End, newDigits);
+          }
+          else {
+            diag.fixItReplaceChars(Range.Start, Range.End, "*");
+          }
+        }
       }
 
       CV.Components.push_back(0);
@@ -159,6 +180,38 @@ Optional<Version> Version::parseCompilerVersionString(
     isValidVersion = false;
   }
 
+  // In the beginning, '_compiler_version(string-literal)' was designed for a
+  // different version scheme where the major was fairly large and the minor
+  // was ignored; now we use one where the minor is significant and major and
+  // minor match the Swift language version. See the comment above on
+  // `firstComponentLooksNew` for details.
+  //
+  // However, we want the string literal variant of '_compiler_version' to
+  // maintain source compatibility with old checks; that means checks for new
+  // versions have to be written so that old compilers will think they represent
+  // newer versions, while new compilers have to interpret old version number
+  // strings in a way that will compare correctly to the new versions compiled
+  // into them.
+  //
+  // To achieve this, modern compilers divide the major by 1000 and overwrite
+  // the wildcard component with the remainder, effectively shifting the last
+  // three digits of the major into the minor, before comparing it to the
+  // compiler version:
+  //
+  //     _compiler_version("5007.*.1.2.3") -> 5.7.1.2.3
+  //     _compiler_version("1300.*.1.2.3") -> 1.300.1.2.3 (smaller than 5.6)
+  //     _compiler_version( "600.*.1.2.3") -> 0.600.1.2.3 (smaller than 5.6)
+  //
+  // So if you want to specify a 5.7.z.a.b version, we ask users to either write
+  // it as 5007.*.z.a.b, or to use the new '_compiler_version(>= version)'
+  // syntax instead, which does not perform this conversion.
+  if (!CV.Components.empty()) {
+    if (CV.Components.size() == 1)
+      CV.Components.push_back(0);
+    CV.Components[1] = CV.Components[0] % 1000;
+    CV.Components[0] = CV.Components[0] / 1000;
+  }
+
   return isValidVersion ? Optional<Version>(CV) : None;
 }
 

lib/ClangImporter/ClangImporter.cpp

@@ -651,9 +651,15 @@ importer::getNormalInvocationArguments(
     // declarations.
     auto V = version::Version::getCurrentCompilerVersion();
     if (!V.empty()) {
+      // Note: Prior to Swift 5.7, the "Y" version component was omitted and the
+      // "X" component resided in its digits.
       invocationArgStrs.insert(invocationArgStrs.end(), {
         V.preprocessorDefinition("__SWIFT_COMPILER_VERSION",
-                                 {1000000000, /*ignored*/ 0, 1000000, 1000, 1}),
+                                 {1000000000000,   // X
+                                     1000000000,   // Y
+                                        1000000,   // Z
+                                           1000,   // a
+                                              1}), // b
       });
     }
   } else {

lib/Parse/ParseIfConfig.cpp

@@ -283,30 +283,26 @@ class ValidateIfConfigCondition :
     }
     // '_compiler_version' '(' string-literal ')'
     if (*KindName == "_compiler_version") {
-      auto SLE = dyn_cast<StringLiteralExpr>(Arg);
-      if (!SLE) {
-        D.diagnose(Arg->getLoc(),
-                   diag::unsupported_platform_condition_argument,
-                   "string literal");
-        return nullptr;
-      }
-
-      auto ValStr = SLE->getValue();
-      if (ValStr.empty()) {
-        D.diagnose(SLE->getLoc(), diag::empty_version_string);
-        return nullptr;
+      if (auto SLE = dyn_cast<StringLiteralExpr>(Arg)) {
+        auto ValStr = SLE->getValue();
+        if (ValStr.empty()) {
+          D.diagnose(SLE->getLoc(), diag::empty_version_string);
+          return nullptr;
+        }
+
+        auto Val = version::Version::parseCompilerVersionString(
+            SLE->getValue(), SLE->getLoc(), &D);
+        if (!Val.hasValue())
+          return nullptr;
+        return E;
       }
-
-      auto Val = version::Version::parseCompilerVersionString(
-          SLE->getValue(), SLE->getLoc(), &D);
-      if (!Val.hasValue())
-        return nullptr;
-      return E;
     }
 
     // 'swift' '(' ('>=' | '<') float-literal ( '.' integer-literal )* ')'
     // 'compiler' '(' ('>=' | '<') float-literal ( '.' integer-literal )* ')'
-    if (*KindName == "swift" || *KindName == "compiler") {
+    // '_compiler_version' '(' ('>=' | '<') float-literal ( '.' integer-literal )* ')'
+    if (*KindName == "swift" || *KindName == "compiler" ||
+        *KindName == "_compiler_version") {
       auto PUE = dyn_cast<PrefixUnaryExpr>(Arg);
       Optional<StringRef> PrefixName =
           PUE ? getDeclRefStr(PUE->getFn(), DeclRefKind::PrefixOperator) : None;
@@ -500,28 +496,30 @@ class EvaluateIfConfigCondition :
   bool visitCallExpr(CallExpr *E) {
     auto KindName = getDeclRefStr(E->getFn());
     auto *Arg = getSingleSubExp(E->getArgs(), KindName, nullptr);
-    if (KindName == "_compiler_version") {
+    if (KindName == "_compiler_version" && isa<StringLiteralExpr>(Arg)) {
       auto Str = cast<StringLiteralExpr>(Arg)->getValue();
       auto Val = version::Version::parseCompilerVersionString(
           Str, SourceLoc(), nullptr).getValue();
       auto thisVersion = version::Version::getCurrentCompilerVersion();
       return thisVersion >= Val;
-    } else if ((KindName == "swift") || (KindName == "compiler")) {
+    } else if ((KindName == "swift") || (KindName == "compiler") ||
+               (KindName == "_compiler_version")) {
       auto PUE = cast<PrefixUnaryExpr>(Arg);
       auto PrefixName = getDeclRefStr(PUE->getFn());
       auto Str = extractExprSource(Ctx.SourceMgr, PUE->getOperand());
       auto Val = version::Version::parseVersionString(
           Str, SourceLoc(), nullptr).getValue();
+      version::Version thisVersion;
       if (KindName == "swift") {
-        return isValidVersion(Ctx.LangOpts.EffectiveLanguageVersion, Val,
-                              PrefixName);
+        thisVersion = Ctx.LangOpts.EffectiveLanguageVersion;
       } else if (KindName == "compiler") {
-        auto currentLanguageVersion =
-            version::Version::getCurrentLanguageVersion();
-        return isValidVersion(currentLanguageVersion, Val, PrefixName);
+        thisVersion = version::Version::getCurrentLanguageVersion();
+      } else if (KindName == "_compiler_version") {
+        thisVersion = version::Version::getCurrentCompilerVersion();
       } else {
         llvm_unreachable("unsupported version conditional");
       }
+      return isValidVersion(thisVersion, Val, PrefixName);
     } else if (KindName == "canImport") {
       auto Str = extractExprSource(Ctx.SourceMgr, Arg);
       bool underlyingModule = false;

test/Parse/ConditionalCompilation/compiler_version.swift

@@ -7,7 +7,7 @@
   asdf asdf asdf asdf
 #endif
 
-#if _compiler_version("10.*.10.10")
+#if _compiler_version("600.*.10.10")
 
 #if os(iOS)
   let z = 1
@@ -48,7 +48,10 @@
   let thisWillStillParseBecauseConfigIsError = 1
 #endif
 
-#if _compiler_version("700.0.100") // expected-warning {{the second version component is not used for comparison}}
+#if _compiler_version("700.0.100") // expected-warning {{the second version component is not used for comparison in legacy compiler versions}} {{28-29=*}}
+#endif
+
+#if _compiler_version("5.7.100") // expected-warning {{the second version component is not used for comparison in legacy compiler versions; are you trying to encode a new Swift compiler version for compatibility with legacy compilers?}} {{24-27=5007.*}}
 #endif
 
 #if _compiler_version("700.*.1.1.1.1") // expected-error {{version must not have more than five components}}
@@ -65,3 +68,14 @@
 
 #if _compiler_version("700.*.1.1.1000") // expected-error {{version component out of range: must be in [0, 999]}}
 #endif
+
+// New style _compiler_version()
+#if _compiler_version(<4.0)
+  // This shouldn't emit any diagnostics.
+  asdf asdf asdf asdf
+#endif
+
+#if !_compiler_version(>=4.3.2.1.0)
+  // This shouldn't emit any diagnostics.
+  asdf asdf asdf asdf
+#endif

unittests/Parse/BuildConfigTests.cpp

@@ -8,6 +8,7 @@ using namespace llvm;
 
 class CompilerVersionTest : public ::testing::Test {};
 class VersionTest : public ::testing::Test{};
+class CompilerVersionUnpackingTest : public ::testing::Test {};
 
 Optional<version::Version> CV(const char *VersionString) {
   return version::Version::parseCompilerVersionString(VersionString,
@@ -52,3 +53,31 @@ TEST_F(VersionTest, VersionComparison) {
   EXPECT_FALSE(V(".1").hasValue());
 
 }
+
+TEST_F(CompilerVersionUnpackingTest, VersionComparison) {
+  EXPECT_EQ(CV("700").getValue(), V("0.700").getValue());
+  EXPECT_EQ(CV("700.*").getValue(), V("0.700").getValue());
+  EXPECT_EQ(CV("700.*.1").getValue(), V("0.700.1").getValue());
+  EXPECT_EQ(CV("700.*.23").getValue(), V("0.700.23").getValue());
+  EXPECT_EQ(CV("700.*.1.1").getValue(), V("0.700.1.1").getValue());
+
+  EXPECT_EQ(CV("1300").getValue(), V("1.300").getValue());
+  EXPECT_EQ(CV("1300.*").getValue(), V("1.300").getValue());
+  EXPECT_EQ(CV("1300.*.1").getValue(), V("1.300.1").getValue());
+  EXPECT_EQ(CV("1300.*.23").getValue(), V("1.300.23").getValue());
+  EXPECT_EQ(CV("1300.*.1.1").getValue(), V("1.300.1.1").getValue());
+
+  EXPECT_EQ(CV("5007").getValue(), V("5.7").getValue());
+  EXPECT_EQ(CV("5007.*").getValue(), V("5.7").getValue());
+  EXPECT_EQ(CV("5007.*.1").getValue(), V("5.7.1").getValue());
+  EXPECT_EQ(CV("5007.*.23").getValue(), V("5.7.23").getValue());
+  EXPECT_EQ(CV("5007.*.1.1").getValue(), V("5.7.1.1").getValue());
+
+  // Since this test was added during 5.7, we expect all of these comparisons to
+  // be GE, either because we are comparing to the empty version or because we
+  // are comparing to a version >= 5.7.0.0.0.
+  auto currentVersion = version::Version::getCurrentCompilerVersion();
+  EXPECT_GE(CV("700"), currentVersion);
+  EXPECT_GE(CV("1300"), currentVersion);
+  EXPECT_GE(CV("5007"), currentVersion);
+}