llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-driver Author: Cyndy Ishida (cyndyishida) <details> <summary>Changes</summary> 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. --- 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: - (modified) clang/include/clang/Basic/DiagnosticDriverKinds.td (+5) - (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+154-123) - (modified) clang/test/Driver/darwin-debug-flags.c (+1-1) - (modified) clang/test/Driver/darwin-version.c (+6-1) ``````````diff 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] `````````` </details> https://github.com/llvm/llvm-project/pull/142013 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
