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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion include/swift/AST/DiagnosticsParse.def
Original file line number Diff line number Diff line change
Expand Up @@ -1819,7 +1819,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,
Expand Down
7 changes: 6 additions & 1 deletion include/swift/Basic/Version.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
59 changes: 56 additions & 3 deletions lib/Basic/Version.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}

Expand Down
8 changes: 7 additions & 1 deletion lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -649,9 +649,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 {
Expand Down
50 changes: 24 additions & 26 deletions lib/Parse/ParseIfConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
18 changes: 16 additions & 2 deletions test/Parse/ConditionalCompilation/compiler_version.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.


#if os(iOS)
let z = 1
Expand Down Expand Up @@ -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}}
Expand All @@ -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
29 changes: 29 additions & 0 deletions unittests/Parse/BuildConfigTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
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.

}