-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[clang][Darwin] Simplify deployment version assignment in the Driver #142013
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
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-driver Author: Cyndy Ishida (cyndyishida) ChangesTo be able to handle all of the ways the platform & deployment version can be represented in command line flags, the Darwin toolchain holds a type
This is not a major refactor, but more simple & common tweaks across the file, in hopes to make it more readable. Patch is 29.21 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/142013.diff 4 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 4da8f80345ddc..7c501f7641667 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -206,6 +206,8 @@ def err_drv_cannot_open_randomize_layout_seed_file : Error<
"cannot read randomize layout seed file '%0'">;
def err_drv_invalid_version_number : Error<
"invalid version number in '%0'">;
+def err_drv_missing_version_number : Error<
+ "missing version number in '%0'">;
def err_drv_kcfi_arity_unsupported_target : Error<
"target '%0' is unsupported by -fsanitize-kcfi-arity">;
def err_drv_no_linker_llvm_support : Error<
@@ -478,6 +480,9 @@ def warn_ignoring_ftabstop_value : Warning<
def warn_drv_overriding_option : Warning<
"overriding '%0' option with '%1'">,
InGroup<DiagGroup<"overriding-option">>;
+def warn_drv_overriding_deployment_version
+ : Warning<"overriding deployment version from '%0' to '%1'">,
+ InGroup<DiagGroup<"overriding-deployment-version">>;
def warn_drv_treating_input_as_cxx : Warning<
"treating '%0' input as '%1' when in C++ mode, this behavior is deprecated">,
InGroup<Deprecated>;
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index 26e24ad0ab17c..1fea71fd2bb18 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1709,24 +1709,21 @@ struct DarwinPlatform {
Environment = Kind;
InferSimulatorFromArch = false;
}
+ const VersionTuple &getOSVersion() const { return ResolvedOSVersion; }
- StringRef getOSVersion() const {
- if (Kind == OSVersionArg)
- return Argument->getValue();
- return OSVersion;
+ VersionTuple getCanonicalOSVersion() const {
+ return llvm::Triple::getCanonicalVersionForOS(getOSFromPlatform(Platform),
+ ResolvedOSVersion);
}
- void setOSVersion(StringRef S) {
- assert(Kind == TargetArg && "Unexpected kind!");
- OSVersion = std::string(S);
- }
+ void setOSVersion(VersionTuple Version) { ResolvedOSVersion = Version; }
- bool hasOSVersion() const { return HasOSVersion; }
+ bool providedOSVersion() const { return ProvidedOSVersion; }
- VersionTuple getNativeTargetVersion() const {
+ VersionTuple getZipperedOSVersion() const {
assert(Environment == DarwinEnvironmentKind::MacCatalyst &&
- "native target version is specified only for Mac Catalyst");
- return NativeTargetVersion;
+ "zippered target version is specified only for Mac Catalyst");
+ return ZipperedOSVersion;
}
/// Returns true if the target OS was explicitly specified.
@@ -1766,7 +1763,8 @@ struct DarwinPlatform {
// DriverKit always explicitly provides a version in the triple.
return;
}
- Argument = Args.MakeJoinedArg(nullptr, Opts.getOption(Opt), OSVersion);
+ Argument = Args.MakeJoinedArg(nullptr, Opts.getOption(Opt),
+ ResolvedOSVersion.getAsString());
Args.append(Argument);
}
@@ -1782,7 +1780,8 @@ struct DarwinPlatform {
assert(Argument && "OS version argument not yet inferred");
return Argument->getAsString(Args);
case DeploymentTargetEnv:
- return (llvm::Twine(EnvVarName) + "=" + OSVersion).str();
+ return (llvm::Twine(EnvVarName) + "=" + ResolvedOSVersion.getAsString())
+ .str();
}
llvm_unreachable("Unsupported Darwin Source Kind");
}
@@ -1797,13 +1796,13 @@ struct DarwinPlatform {
case llvm::Triple::MacABI: {
Environment = DarwinEnvironmentKind::MacCatalyst;
// The minimum native macOS target for MacCatalyst is macOS 10.15.
- NativeTargetVersion = VersionTuple(10, 15);
- if (HasOSVersion && SDKInfo) {
+ ZipperedOSVersion = VersionTuple(10, 15);
+ if (providedOSVersion() && SDKInfo) {
if (const auto *MacCatalystToMacOSMapping = SDKInfo->getVersionMapping(
DarwinSDKInfo::OSEnvPair::macCatalystToMacOSPair())) {
if (auto MacOSVersion = MacCatalystToMacOSMapping->map(
- OSVersion, NativeTargetVersion, std::nullopt)) {
- NativeTargetVersion = *MacOSVersion;
+ OSVersion, ZipperedOSVersion, std::nullopt)) {
+ ZipperedOSVersion = *MacOSVersion;
}
}
}
@@ -1813,8 +1812,8 @@ struct DarwinPlatform {
if (TargetVariantTriple) {
auto TargetVariantVersion = TargetVariantTriple->getOSVersion();
if (TargetVariantVersion.getMajor()) {
- if (TargetVariantVersion < NativeTargetVersion)
- NativeTargetVersion = TargetVariantVersion;
+ if (TargetVariantVersion < ZipperedOSVersion)
+ ZipperedOSVersion = TargetVariantVersion;
}
}
break;
@@ -1825,14 +1824,12 @@ struct DarwinPlatform {
}
static DarwinPlatform
- createFromTarget(const llvm::Triple &TT, StringRef OSVersion, Arg *A,
+ createFromTarget(const llvm::Triple &TT, Arg *A,
std::optional<llvm::Triple> TargetVariantTriple,
const std::optional<DarwinSDKInfo> &SDKInfo) {
- DarwinPlatform Result(TargetArg, getPlatformFromOS(TT.getOS()), OSVersion,
- A);
+ DarwinPlatform Result(TargetArg, getPlatformFromOS(TT.getOS()),
+ TT.getOSVersion(), A);
VersionTuple OsVersion = TT.getOSVersion();
- if (OsVersion.getMajor() == 0)
- Result.HasOSVersion = false;
Result.TargetVariantTriple = TargetVariantTriple;
Result.setEnvironment(TT.getEnvironment(), OsVersion, SDKInfo);
return Result;
@@ -1841,38 +1838,37 @@ struct DarwinPlatform {
createFromMTargetOS(llvm::Triple::OSType OS, VersionTuple OSVersion,
llvm::Triple::EnvironmentType Environment, Arg *A,
const std::optional<DarwinSDKInfo> &SDKInfo) {
- DarwinPlatform Result(MTargetOSArg, getPlatformFromOS(OS),
- OSVersion.getAsString(), A);
+ DarwinPlatform Result(MTargetOSArg, getPlatformFromOS(OS), OSVersion, A);
Result.InferSimulatorFromArch = false;
Result.setEnvironment(Environment, OSVersion, SDKInfo);
return Result;
}
static DarwinPlatform createOSVersionArg(DarwinPlatformKind Platform, Arg *A,
bool IsSimulator) {
- DarwinPlatform Result{OSVersionArg, Platform, A};
+ DarwinPlatform Result{OSVersionArg, Platform, getVersionFromString(A->getValue()), A};
if (IsSimulator)
Result.Environment = DarwinEnvironmentKind::Simulator;
return Result;
}
static DarwinPlatform createDeploymentTargetEnv(DarwinPlatformKind Platform,
StringRef EnvVarName,
- StringRef Value) {
- DarwinPlatform Result(DeploymentTargetEnv, Platform, Value);
+ StringRef OSVersion) {
+ DarwinPlatform Result(DeploymentTargetEnv, Platform, getVersionFromString(OSVersion));
Result.EnvVarName = EnvVarName;
return Result;
}
static DarwinPlatform createFromSDK(DarwinPlatformKind Platform,
StringRef Value,
bool IsSimulator = false) {
- DarwinPlatform Result(InferredFromSDK, Platform, Value);
+ DarwinPlatform Result(InferredFromSDK, Platform, getVersionFromString(Value));
if (IsSimulator)
Result.Environment = DarwinEnvironmentKind::Simulator;
Result.InferSimulatorFromArch = false;
return Result;
}
static DarwinPlatform createFromArch(llvm::Triple::OSType OS,
- StringRef Value) {
- return DarwinPlatform(InferredFromArch, getPlatformFromOS(OS), Value);
+ VersionTuple Version) {
+ return DarwinPlatform(InferredFromArch, getPlatformFromOS(OS), Version);
}
/// Constructs an inferred SDKInfo value based on the version inferred from
@@ -1880,22 +1876,27 @@ struct DarwinPlatform {
/// the platform from the SDKPath.
DarwinSDKInfo inferSDKInfo() {
assert(Kind == InferredFromSDK && "can infer SDK info only");
- llvm::VersionTuple Version;
- bool IsValid = !Version.tryParse(OSVersion);
- (void)IsValid;
- assert(IsValid && "invalid SDK version");
- return DarwinSDKInfo(
- Version,
- /*MaximumDeploymentTarget=*/VersionTuple(Version.getMajor(), 0, 99),
- getOSFromPlatform(Platform));
+ return DarwinSDKInfo(ResolvedOSVersion,
+ /*MaximumDeploymentTarget=*/
+ VersionTuple(ResolvedOSVersion.getMajor(), 0, 99),
+ getOSFromPlatform(Platform));
}
private:
DarwinPlatform(SourceKind Kind, DarwinPlatformKind Platform, Arg *Argument)
: Kind(Kind), Platform(Platform), Argument(Argument) {}
- DarwinPlatform(SourceKind Kind, DarwinPlatformKind Platform, StringRef Value,
- Arg *Argument = nullptr)
- : Kind(Kind), Platform(Platform), OSVersion(Value), Argument(Argument) {}
+ DarwinPlatform(SourceKind Kind, DarwinPlatformKind Platform,
+ VersionTuple Value, Arg *Argument = nullptr)
+ : Kind(Kind), Platform(Platform), ResolvedOSVersion(Value),
+ ProvidedOSVersion(!Value.empty()), Argument(Argument) {}
+
+ static VersionTuple getVersionFromString(const StringRef Input) {
+ llvm::VersionTuple Version;
+ bool IsValid = !Version.tryParse(Input);
+ assert(IsValid && "unable to convert input version to version tuple");
+ (void)IsValid;
+ return Version;
+ }
static DarwinPlatformKind getPlatformFromOS(llvm::Triple::OSType OS) {
switch (OS) {
@@ -1938,11 +1939,22 @@ struct DarwinPlatform {
SourceKind Kind;
DarwinPlatformKind Platform;
DarwinEnvironmentKind Environment = DarwinEnvironmentKind::NativeEnvironment;
- VersionTuple NativeTargetVersion;
- std::string OSVersion;
- bool HasOSVersion = true, InferSimulatorFromArch = true;
+ // When compiling for a zippered target, this means both target &
+ // target variant is set on the command line, ZipperedOSVersion holds the
+ // OSVersion tied to the main target value.
+ VersionTuple ZipperedOSVersion;
+ // We allow multiple ways to set or default the OS
+ // version used for compilation. The ResolvedOSVersion always represents what
+ // will be used.
+ VersionTuple ResolvedOSVersion;
+ // Track whether the OS deployment version was explicitly set on creation.
+ // This can be used for overidding the resolved version or error reporting.
+ bool ProvidedOSVersion = false;
+ bool InferSimulatorFromArch = true;
Arg *Argument;
StringRef EnvVarName;
+ // When compiling for a zippered target, this value represents the target
+ // triple encoded in the target variant.
std::optional<llvm::Triple> TargetVariantTriple;
};
@@ -1960,6 +1972,19 @@ getDeploymentTargetFromOSVersionArg(DerivedArgList &Args,
Arg *WatchOSVersion =
Args.getLastArg(options::OPT_mwatchos_version_min_EQ,
options::OPT_mwatchos_simulator_version_min_EQ);
+
+ auto GetDarwinPlatform =
+ [&](DarwinPlatform::DarwinPlatformKind Platform, Arg *VersionArg,
+ bool IsSimulator) -> std::optional<DarwinPlatform> {
+ if (StringRef(VersionArg->getValue()).empty()) {
+ TheDriver.Diag(diag::err_drv_missing_version_number)
+ << VersionArg->getAsString(Args);
+ return std::nullopt;
+ }
+ return DarwinPlatform::createOSVersionArg(Platform, VersionArg,
+ /*IsSimulator=*/IsSimulator);
+ };
+
if (macOSVersion) {
if (iOSVersion || TvOSVersion || WatchOSVersion) {
TheDriver.Diag(diag::err_drv_argument_not_allowed_with)
@@ -1968,15 +1993,15 @@ getDeploymentTargetFromOSVersionArg(DerivedArgList &Args,
: TvOSVersion ? TvOSVersion : WatchOSVersion)
->getAsString(Args);
}
- return DarwinPlatform::createOSVersionArg(Darwin::MacOS, macOSVersion,
- /*IsSimulator=*/false);
+ return GetDarwinPlatform(Darwin::MacOS, macOSVersion, /*IsSimulator=*/false);
+
} else if (iOSVersion) {
if (TvOSVersion || WatchOSVersion) {
TheDriver.Diag(diag::err_drv_argument_not_allowed_with)
<< iOSVersion->getAsString(Args)
<< (TvOSVersion ? TvOSVersion : WatchOSVersion)->getAsString(Args);
}
- return DarwinPlatform::createOSVersionArg(
+ return GetDarwinPlatform(
Darwin::IPhoneOS, iOSVersion,
iOSVersion->getOption().getID() ==
options::OPT_mios_simulator_version_min_EQ);
@@ -1986,12 +2011,12 @@ getDeploymentTargetFromOSVersionArg(DerivedArgList &Args,
<< TvOSVersion->getAsString(Args)
<< WatchOSVersion->getAsString(Args);
}
- return DarwinPlatform::createOSVersionArg(
+ return GetDarwinPlatform(
Darwin::TvOS, TvOSVersion,
TvOSVersion->getOption().getID() ==
options::OPT_mtvos_simulator_version_min_EQ);
} else if (WatchOSVersion)
- return DarwinPlatform::createOSVersionArg(
+ return GetDarwinPlatform(
Darwin::WatchOS, WatchOSVersion,
WatchOSVersion->getOption().getID() ==
options::OPT_mwatchos_simulator_version_min_EQ);
@@ -2125,8 +2150,8 @@ inferDeploymentTargetFromSDK(DerivedArgList &Args,
return CreatePlatformFromSDKName(dropSDKNamePrefix(SDK));
}
-std::string getOSVersion(llvm::Triple::OSType OS, const llvm::Triple &Triple,
- const Driver &TheDriver) {
+VersionTuple getOSVersion(llvm::Triple::OSType OS, const llvm::Triple &Triple,
+ const Driver &TheDriver) {
VersionTuple OsVersion;
llvm::Triple SystemTriple(llvm::sys::getProcessTriple());
switch (OS) {
@@ -2165,12 +2190,7 @@ std::string getOSVersion(llvm::Triple::OSType OS, const llvm::Triple &Triple,
llvm_unreachable("Unexpected OS type");
break;
}
-
- std::string OSVersion;
- llvm::raw_string_ostream(OSVersion)
- << OsVersion.getMajor() << '.' << OsVersion.getMinor().value_or(0) << '.'
- << OsVersion.getSubminor().value_or(0);
- return OSVersion;
+ return OsVersion;
}
/// Tries to infer the target OS from the -arch.
@@ -2206,7 +2226,7 @@ std::optional<DarwinPlatform> getDeploymentTargetFromTargetArg(
if (Triple.getOS() == llvm::Triple::Darwin ||
Triple.getOS() == llvm::Triple::UnknownOS)
return std::nullopt;
- std::string OSVersion = getOSVersion(Triple.getOS(), Triple, TheDriver);
+ VersionTuple OSVersion = getOSVersion(Triple.getOS(), Triple, TheDriver);
std::optional<llvm::Triple> TargetVariantTriple;
for (const Arg *A : Args.filtered(options::OPT_darwin_target_variant)) {
llvm::Triple TVT(A->getValue());
@@ -2232,9 +2252,14 @@ std::optional<DarwinPlatform> getDeploymentTargetFromTargetArg(
<< A->getSpelling() << A->getValue();
}
}
- return DarwinPlatform::createFromTarget(Triple, OSVersion,
- Args.getLastArg(options::OPT_target),
- TargetVariantTriple, SDKInfo);
+ DarwinPlatform DP = DarwinPlatform::createFromTarget(
+ Triple, Args.getLastArg(options::OPT_target), TargetVariantTriple,
+ SDKInfo);
+
+ // Override the OSVersion if it doesn't match the one from the triple.
+ if (Triple.getOSVersion() != OSVersion)
+ DP.setOSVersion(OSVersion);
+ return DP;
}
/// Returns the deployment target that's specified using the -mtargetos option.
@@ -2313,12 +2338,12 @@ void Darwin::AddDeploymentTarget(DerivedArgList &Args) const {
SDKInfo = parseSDKSettings(getVFS(), Args, getDriver());
// The OS and the version can be specified using the -target argument.
- std::optional<DarwinPlatform> OSTarget =
+ std::optional<DarwinPlatform> DP =
getDeploymentTargetFromTargetArg(Args, getTriple(), getDriver(), SDKInfo);
- if (OSTarget) {
+ if (DP) {
// Disallow mixing -target and -mtargetos=.
if (const auto *MTargetOSArg = Args.getLastArg(options::OPT_mtargetos_EQ)) {
- std::string TargetArgStr = OSTarget->getAsString(Args, Opts);
+ std::string TargetArgStr = DP->getAsString(Args, Opts);
std::string MTargetOSArgStr = MTargetOSArg->getAsString(Args);
getDriver().Diag(diag::err_drv_cannot_mix_options)
<< TargetArgStr << MTargetOSArgStr;
@@ -2330,102 +2355,112 @@ void Darwin::AddDeploymentTarget(DerivedArgList &Args) const {
bool TargetExtra;
unsigned ArgMajor, ArgMinor, ArgMicro;
bool ArgExtra;
- if (OSTarget->getPlatform() != OSVersionArgTarget->getPlatform() ||
- (Driver::GetReleaseVersion(OSTarget->getOSVersion(), TargetMajor,
- TargetMinor, TargetMicro, TargetExtra) &&
- Driver::GetReleaseVersion(OSVersionArgTarget->getOSVersion(),
- ArgMajor, ArgMinor, ArgMicro, ArgExtra) &&
+ if (DP->getPlatform() != OSVersionArgTarget->getPlatform() ||
+ (Driver::GetReleaseVersion(DP->getOSVersion().getAsString(),
+ TargetMajor, TargetMinor, TargetMicro,
+ TargetExtra) &&
+ Driver::GetReleaseVersion(
+ OSVersionArgTarget->getOSVersion().getAsString(), ArgMajor,
+ ArgMinor, ArgMicro, ArgExtra) &&
(VersionTuple(TargetMajor, TargetMinor, TargetMicro) !=
VersionTuple(ArgMajor, ArgMinor, ArgMicro) ||
TargetExtra != ArgExtra))) {
// Select the OS version from the -m<os>-version-min argument when
// the -target does not include an OS version.
- if (OSTarget->getPlatform() == OSVersionArgTarget->getPlatform() &&
- !OSTarget->hasOSVersion()) {
- OSTarget->setOSVersion(OSVersionArgTarget->getOSVersion());
+ if (DP->getPlatform() == OSVersionArgTarget->getPlatform() &&
+ !DP->providedOSVersion()) {
+ DP->setOSVersion(OSVersionArgTarget->getOSVersion());
} else {
// Warn about -m<os>-version-min that doesn't match the OS version
// that's specified in the target.
std::string OSVersionArg =
OSVersionArgTarget->getAsString(Args, Opts);
- std::string TargetArg = OSTarget->getAsString(Args, Opts);
+ std::string TargetArg = DP->getAsString(Args, Opts);
getDriver().Diag(clang::diag::warn_drv_overriding_option)
<< OSVersionArg << TargetArg;
}
}
}
- } else if ((OSTarget = getDeploymentTargetFromMTargetOSArg(Args, getDriver(),
- SDKInfo))) {
+ } else if ((DP = getDeploymentTargetFromMTargetOSArg(Args, getDriver(),
+ SDKInfo))) {
// The OS target can be specified using the -mtargetos= argument.
// Disallow mixing -mtargetos= and -m<os>version-min=.
std::optional<DarwinPlatform> OSVersionArgTarget =
getDeploymentTargetFromOSVersionArg(Args, getDriver());
if (OSVersionArgTarget) {
- std::string MTargetOSArgStr = OSTarget->getAsString(Args, Opts);
+ std::string MTargetOSArgStr = DP->getAsString(Args, Opts);
std::string OSVersionArgStr = OSVersionArgTarget->getAsString(Args, Opts);
getDriver().Diag(diag::err_drv_cannot_mix_options)
<< MTargetOSArgStr << OSVersionArgStr;
}
} else {
// The OS target can be specified using the -m<os>version-min argument.
- OSTarget = getDeploymentTargetFromOSVersionArg(Args, getDriver());
+ DP = getDeploymentTargetFromOSVersionArg(Args, getDriver());
// If no deployment target was specified on the command line, check for
// environment defines.
- if (!OSTarget) {
- OSTarget =
+ if (!DP) {
+ DP =
getDeploymentTargetFromEnvironmentVariables(getDriver(), getTriple());
- if (OSTarget) {
+ if (DP) {
// Don't infer simulator from the arch when the SDK is also specified.
std::optional<DarwinPlatform> SDKTarget =
inferDeploymentTargetFromSDK(Args, ...
[truncated]
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
To be able to handle all of the ways the platform & deployment version can be represented in command line flags, the Darwin toolchain holds a type DarwinPlatform to help represent them. This patch simplifies the logic by: reducing the amount of work done between string & version tuples conversions renaming variables to reduce confusion about what target triple information is being manipulated. allowing implicit transformation of macOS10.16 -> 11, there are other places in the compiler where this happens, and it was a bit confusing that the driver didn't do that for the cc1 call. This is not a major refactor, but more simple & common tweaks across the file, in hopes to make it more readable.
a867aa7
to
a12e45e
Compare
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.
Generally looks fine.
@@ -2313,12 +2340,12 @@ void Darwin::AddDeploymentTarget(DerivedArgList &Args) const { | |||
SDKInfo = parseSDKSettings(getVFS(), Args, getDriver()); | |||
|
|||
// The OS and the version can be specified using the -target argument. | |||
std::optional<DarwinPlatform> OSTarget = | |||
std::optional<DarwinPlatform> DP = |
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 feel DP
is not a good variable name as it is used everywhere but it is hard to tell what it is from variable name.
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 can change it to . Actually I like PlatformForDarwin
PlatformAndVersion
better.
To be able to handle all of the ways the platform & deployment version can be represented in command line flags, the Darwin toolchain holds a type
DarwinPlatform
to help represent them. This patch simplifies the logic by:This is not a major refactor, but more simple & common tweaks across the file, in hopes to make it more readable.