Skip to content

Resolve a Layering Violation in libBasic #61014

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.

Sign up for GitHub

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 2 commits into from
Sep 9, 2022
Merged

Resolve a Layering Violation in libBasic #61014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
18b79ff
Resolve a Layering Violation in libBasic
CodaFi Sep 9, 2022
a71b970
Revert "Add link between swiftBasic and swiftAST"
CodaFi Sep 9, 2022
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
33 changes: 3 additions & 30 deletions include/swift/Basic/Version.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

namespace swift {

class DiagnosticEngine;
class VersionParser;
class SourceLoc;

namespace version {
Expand All @@ -53,6 +53,7 @@ namespace version {
/// a: [0 - 999]
/// b: [0 - 999]
class Version {
friend class swift::VersionParser;
SmallVector<unsigned, 5> Components;
public:
/// Create the empty compiler version - this always compares greater
Expand All @@ -63,11 +64,6 @@ class Version {
/// Create a literal version from a list of components.
Version(std::initializer_list<unsigned> Values) : Components(Values) {}

/// Create a version from a string in source code.
///
/// Must include only groups of digits separated by a dot.
Version(StringRef VersionString, SourceLoc Loc, DiagnosticEngine *Diags);

/// Return a string to be used as an internal preprocessor define.
///
/// The components of the version are multiplied element-wise by
Expand Down Expand Up @@ -129,29 +125,6 @@ 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(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);

/// Parse a generic version string of the format [0-9]+(.[0-9]+)*
///
/// Version components can be any unsigned 64-bit number.
static Optional<Version> parseVersionString(StringRef VersionString,
SourceLoc Loc,
DiagnosticEngine *Diags);

/// Returns a version from the currently defined SWIFT_COMPILER_VERSION.
///
/// If SWIFT_COMPILER_VERSION is undefined, this will return the empty
/// compiler version.
static Version getCurrentCompilerVersion();

/// Returns a version from the currently defined SWIFT_VERSION_MAJOR and
/// SWIFT_VERSION_MINOR.
static Version getCurrentLanguageVersion();
Expand Down Expand Up @@ -190,7 +163,7 @@ std::string getSwiftFullVersion(Version effectiveLanguageVersion =
StringRef getSwiftRevision();

/// Is the running compiler built with a version tag for distribution?
/// When true, \c Version::getCurrentCompilerVersion returns a valid version
/// When true, \c version::getCurrentCompilerVersion returns a valid version
/// and \c getCurrentCompilerTag returns the version tuple in string format.
bool isCurrentCompilerTagged();

Expand Down
50 changes: 50 additions & 0 deletions include/swift/Parse/ParseVersion.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//===--- ParseVersion.h - Parser Swift Version Numbers ----------*- C++ -*-===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

#ifndef SWIFT_PARSE_PARSEVERSION_H
#define SWIFT_PARSE_PARSEVERSION_H

#include "swift/Basic/Version.h"

namespace swift {
class DiagnosticEngine;

namespace version {
/// Returns a version from the currently defined SWIFT_COMPILER_VERSION.
///
/// If SWIFT_COMPILER_VERSION is undefined, this will return the empty
/// compiler version.
Version getCurrentCompilerVersion();
} // namespace version

class VersionParser final {
Copy link
Member

@compnerd compnerd Sep 9, 2022

Choose a reason for hiding this comment

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

Is there a reason to make this a class and add static methods rather than a namespace and free functions for parseCompilerVersionString and parseVersionString?

Copy link
Contributor Author

@CodaFi CodaFi Sep 9, 2022

Choose a reason for hiding this comment

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

I need to friend Version so I can poke at the component vector.

Copy link
Member

@compnerd compnerd Sep 9, 2022

Choose a reason for hiding this comment

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

Ah, I see. That probably deserves a comment.

public:
/// 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::Version>
parseCompilerVersionString(StringRef VersionString, SourceLoc Loc,
DiagnosticEngine *Diags);

/// Parse a generic version string of the format [0-9]+(.[0-9]+)*
///
/// Version components can be any unsigned 64-bit number.
static Optional<version::Version> parseVersionString(StringRef VersionString,
SourceLoc Loc,
DiagnosticEngine *Diags);
};
} // namespace swift

#endif // SWIFT_PARSE_PARSEVERSION_H
4 changes: 0 additions & 4 deletions lib/Basic/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,6 @@ target_link_libraries(swiftBasic PUBLIC
target_link_libraries(swiftBasic PRIVATE
${UUID_LIBRARIES})

# This is because swiftBasic uses InFlightDiagnostic::flush, which is defined
# in swiftAST. This is a cyclic link dependency and it should be removed.
target_link_libraries(swiftBasic PUBLIC swiftAST)

message(STATUS "Swift version: ${SWIFT_VERSION}")
message(STATUS "Swift vendor: ${SWIFT_VENDOR}")

Expand Down
218 changes: 0 additions & 218 deletions lib/Basic/Version.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,8 @@

#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"
#include "swift/Basic/LLVM.h"
#include "swift/Basic/Version.h"

Expand Down Expand Up @@ -68,222 +66,6 @@ static void printFullRevisionString(raw_ostream &out) {
#endif
}

static void splitVersionComponents(
SmallVectorImpl<std::pair<StringRef, SourceRange>> &SplitComponents,
StringRef &VersionString, SourceLoc Loc,
bool skipQuote = false) {
SourceLoc Start = (Loc.isValid() && skipQuote) ? Loc.getAdvancedLoc(1) : Loc;
SourceLoc End = Start;

// Split the version string into tokens separated by the '.' character.
while (!VersionString.empty()) {
StringRef SplitComponent, Rest;
std::tie(SplitComponent, Rest) = VersionString.split('.');

if (Loc.isValid()) {
End = End.getAdvancedLoc(SplitComponent.size());
}
auto Range = Loc.isValid() ? SourceRange(Start, End) : SourceRange();
if (Loc.isValid())
End = End.getAdvancedLoc(1);
Start = End;
SplitComponents.push_back({SplitComponent, Range});
VersionString = Rest;
}
}

Optional<Version> Version::parseCompilerVersionString(
StringRef VersionString, SourceLoc Loc, DiagnosticEngine *Diags) {

Version CV;
SmallString<16> digits;
llvm::raw_svector_ostream OS(digits);
SmallVector<std::pair<StringRef, SourceRange>, 5> SplitComponents;

splitVersionComponents(SplitComponents, VersionString, Loc,
/*skipQuote=*/true);

uint64_t ComponentNumber;
bool isValidVersion = true;

auto checkVersionComponent = [&](unsigned Component, SourceRange Range) {
unsigned limit = CV.Components.empty() ? 9223371 : 999;

if (Component > limit) {
if (Diags)
Diags->diagnose(Range.Start,
diag::compiler_version_component_out_of_range, limit);
isValidVersion = false;
}
};

for (size_t i = 0; i < SplitComponents.size(); ++i) {
StringRef SplitComponent;
SourceRange Range;
std::tie(SplitComponent, Range) = SplitComponents[i];

// Version components can't be empty.
if (SplitComponent.empty()) {
if (Diags)
Diags->diagnose(Range.Start, diag::empty_version_component);
isValidVersion = false;
continue;
}

// The second version component isn't used for comparison.
if (i == 1) {
if (!SplitComponent.equals("*")) {
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);
continue;
}

// All other version components must be numbers.
if (!SplitComponent.getAsInteger(10, ComponentNumber)) {
checkVersionComponent(ComponentNumber, Range);
CV.Components.push_back(ComponentNumber);
continue;
} else {
if (Diags)
Diags->diagnose(Range.Start, diag::version_component_not_number);
isValidVersion = false;
}
}

if (CV.Components.size() > 5) {
if (Diags)
Diags->diagnose(Loc, diag::compiler_version_too_many_components);
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;
}

Optional<Version> Version::parseVersionString(StringRef VersionString,
SourceLoc Loc,
DiagnosticEngine *Diags) {
Version TheVersion;
SmallString<16> digits;
llvm::raw_svector_ostream OS(digits);
SmallVector<std::pair<StringRef, SourceRange>, 5> SplitComponents;
// Skip over quote character in string literal.

if (VersionString.empty()) {
if (Diags)
Diags->diagnose(Loc, diag::empty_version_string);
return None;
}

splitVersionComponents(SplitComponents, VersionString, Loc, Diags);

uint64_t ComponentNumber;
bool isValidVersion = true;

for (size_t i = 0; i < SplitComponents.size(); ++i) {
StringRef SplitComponent;
SourceRange Range;
std::tie(SplitComponent, Range) = SplitComponents[i];

// Version components can't be empty.
if (SplitComponent.empty()) {
if (Diags)
Diags->diagnose(Range.Start, diag::empty_version_component);

isValidVersion = false;
continue;
}

// All other version components must be numbers.
if (!SplitComponent.getAsInteger(10, ComponentNumber)) {
TheVersion.Components.push_back(ComponentNumber);
continue;
} else {
if (Diags)
Diags->diagnose(Range.Start,
diag::version_component_not_number);
isValidVersion = false;
}
}

return isValidVersion ? Optional<Version>(TheVersion) : None;
}

Version::Version(StringRef VersionString,
SourceLoc Loc,
DiagnosticEngine *Diags)
: Version(*parseVersionString(VersionString, Loc, Diags))
{}

Version Version::getCurrentCompilerVersion() {
#ifdef SWIFT_COMPILER_VERSION
auto currentVersion = Version::parseVersionString(
SWIFT_COMPILER_VERSION, SourceLoc(), nullptr);
assert(currentVersion.hasValue() &&
"Embedded Swift language version couldn't be parsed: '"
SWIFT_COMPILER_VERSION
"'");
return currentVersion.getValue();
#else
return Version();
#endif
}

Version Version::getCurrentLanguageVersion() {
#if SWIFT_VERSION_PATCHLEVEL
return {SWIFT_VERSION_MAJOR, SWIFT_VERSION_MINOR, SWIFT_VERSION_PATCHLEVEL};
Expand Down
5 changes: 3 additions & 2 deletions lib/ClangImporter/ClangImporter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include "swift/ClangImporter/ClangImporterRequests.h"
#include "swift/ClangImporter/ClangModule.h"
#include "swift/Parse/Lexer.h"
#include "swift/Parse/ParseVersion.h"
#include "swift/Parse/Parser.h"
#include "swift/Strings.h"
#include "swift/Subsystems.h"
Expand Down Expand Up @@ -73,8 +74,8 @@
#include "llvm/Support/VirtualFileSystem.h"
#include "llvm/Support/YAMLParser.h"
#include <algorithm>
#include <string>
#include <memory>
#include <string>

using namespace swift;
using namespace importer;
Expand Down Expand Up @@ -608,7 +609,7 @@ importer::getNormalInvocationArguments(

// Get the version of this compiler and pass it to C/Objective-C
// declarations.
auto V = version::Version::getCurrentCompilerVersion();
auto V = 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.
Expand Down
Loading