[PATCH] D41035: [driver][darwin] Refactor the target selection code, NFC
This revision was automatically updated to reflect the committed changes. Closed by commit rC320235: [driver][darwin] Refactor the target selection code, NFC (authored by arphaman). Repository: rC Clang https://reviews.llvm.org/D41035 Files: lib/Driver/ToolChains/Darwin.cpp lib/Driver/ToolChains/Darwin.h test/Driver/darwin-version.c Index: test/Driver/darwin-version.c === --- test/Driver/darwin-version.c +++ test/Driver/darwin-version.c @@ -139,3 +139,8 @@ // RUN: %clang -target x86_64-apple-watchos4.0 -c %s -### 2>&1 | \ // RUN: FileCheck --check-prefix=CHECK-VERSION-WATCHOS-TARGET %s // CHECK-VERSION-WATCHOS-TARGET: "x86_64-apple-watchos4.0.0-simulator" + +// RUN: env MACOSX_DEPLOYMENT_TARGET=1000.1000 \ +// RUN: %clang -c %s -### 2>&1 | \ +// RUN: FileCheck --check-prefix=CHECK-VERSION-INVALID-ENV %s +// CHECK-VERSION-INVALID-ENV: invalid version number in 'MACOSX_DEPLOYMENT_TARGET=1000.1000' Index: lib/Driver/ToolChains/Darwin.cpp === --- lib/Driver/ToolChains/Darwin.cpp +++ lib/Driver/ToolChains/Darwin.cpp @@ -941,13 +941,10 @@ case DarwinPlatformKind::MacOS: return "MacOSX"; case DarwinPlatformKind::IPhoneOS: -case DarwinPlatformKind::IPhoneOSSimulator: return "iPhone"; case DarwinPlatformKind::TvOS: -case DarwinPlatformKind::TvOSSimulator: return "AppleTV"; case DarwinPlatformKind::WatchOS: -case DarwinPlatformKind::WatchOSSimulator: return "Watch"; } llvm_unreachable("Unsupported platform"); @@ -971,17 +968,11 @@ case DarwinPlatformKind::MacOS: return "osx"; case DarwinPlatformKind::IPhoneOS: -return "ios"; - case DarwinPlatformKind::IPhoneOSSimulator: -return "iossim"; +return TargetEnvironment == NativeEnvironment ? "ios" : "iossim"; case DarwinPlatformKind::TvOS: -return "tvos"; - case DarwinPlatformKind::TvOSSimulator: -return "tvossim"; +return TargetEnvironment == NativeEnvironment ? "tvos" : "tvossim"; case DarwinPlatformKind::WatchOS: -return "watchos"; - case DarwinPlatformKind::WatchOSSimulator: -return "watchossim"; +return TargetEnvironment == NativeEnvironment ? "watchos" : "watchossim"; } llvm_unreachable("Unsupported platform"); } @@ -1172,28 +1163,131 @@ return MacOSSDKVersion; } -void Darwin::AddDeploymentTarget(DerivedArgList ) const { - const OptTable = getDriver().getOpts(); +namespace { - // Support allowing the SDKROOT environment variable used by xcrun and other - // Xcode tools to define the default sysroot, by making it the default for - // isysroot. - if (const Arg *A = Args.getLastArg(options::OPT_isysroot)) { -// Warn if the path does not exist. -if (!getVFS().exists(A->getValue())) - getDriver().Diag(clang::diag::warn_missing_sysroot) << A->getValue(); - } else { -if (char *env = ::getenv("SDKROOT")) { - // We only use this value as the default if it is an absolute path, - // exists, and it is not the root path. - if (llvm::sys::path::is_absolute(env) && getVFS().exists(env) && - StringRef(env) != "/") { -Args.append(Args.MakeSeparateArg( -nullptr, Opts.getOption(options::OPT_isysroot), env)); - } +/// The Darwin OS that was selected or inferred from arguments / environment. +struct DarwinPlatform { + enum SourceKind { +/// The OS was specified using the -target argument. +TargetArg, +/// The OS was specified using the -m-version-min argument. +OSVersionArg, +/// The OS was specified using the OS_DEPLOYMENT_TARGET environment. +DeploymentTargetEnv, +/// The OS was inferred from the SDK. +InferredFromSDK, +/// The OS was inferred from the -arch. +InferredFromArch + }; + + using DarwinPlatformKind = Darwin::DarwinPlatformKind; + + DarwinPlatformKind getPlatform() const { return Platform; } + + StringRef getOSVersion() const { +if (Kind == OSVersionArg) + return Argument->getValue(); +return OSVersion; + } + + /// Returns true if the target OS was explicitly specified. + bool isExplicitlySpecified() const { return Kind <= DeploymentTargetEnv; } + + /// Adds the -m-version-min argument to the compiler invocation. + void addOSVersionMinArgument(DerivedArgList , const OptTable ) { +if (Argument) + return; +assert(Kind != TargetArg && Kind != OSVersionArg && "Invalid kind"); +options::ID Opt; +switch (Platform) { +case DarwinPlatformKind::MacOS: + Opt = options::OPT_mmacosx_version_min_EQ; + break; +case DarwinPlatformKind::IPhoneOS: + Opt = options::OPT_miphoneos_version_min_EQ; + break; +case DarwinPlatformKind::TvOS: + Opt = options::OPT_mtvos_version_min_EQ; + break; +case DarwinPlatformKind::WatchOS: + Opt = options::OPT_mwatchos_version_min_EQ; + break; +} +Argument =
[PATCH] D41035: [driver][darwin] Refactor the target selection code, NFC
steven_wu added a comment. LGTM. Repository: rC Clang https://reviews.llvm.org/D41035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41035: [driver][darwin] Refactor the target selection code, NFC
arphaman updated this revision to Diff 126222. arphaman marked an inline comment as done. arphaman added a comment. Add static_assert Repository: rC Clang https://reviews.llvm.org/D41035 Files: lib/Driver/ToolChains/Darwin.cpp lib/Driver/ToolChains/Darwin.h test/Driver/darwin-version.c Index: test/Driver/darwin-version.c === --- test/Driver/darwin-version.c +++ test/Driver/darwin-version.c @@ -139,3 +139,8 @@ // RUN: %clang -target x86_64-apple-watchos4.0 -c %s -### 2>&1 | \ // RUN: FileCheck --check-prefix=CHECK-VERSION-WATCHOS-TARGET %s // CHECK-VERSION-WATCHOS-TARGET: "x86_64-apple-watchos4.0.0-simulator" + +// RUN: env MACOSX_DEPLOYMENT_TARGET=1000.1000 \ +// RUN: %clang -c %s -### 2>&1 | \ +// RUN: FileCheck --check-prefix=CHECK-VERSION-INVALID-ENV %s +// CHECK-VERSION-INVALID-ENV: invalid version number in 'MACOSX_DEPLOYMENT_TARGET=1000.1000' Index: lib/Driver/ToolChains/Darwin.h === --- lib/Driver/ToolChains/Darwin.h +++ lib/Driver/ToolChains/Darwin.h @@ -268,14 +268,17 @@ enum DarwinPlatformKind { MacOS, IPhoneOS, -IPhoneOSSimulator, TvOS, -TvOSSimulator, WatchOS, -WatchOSSimulator +LastDarwinPlatform = WatchOS + }; + enum DarwinEnvironmentKind { +NativeEnvironment, +Simulator, }; mutable DarwinPlatformKind TargetPlatform; + mutable DarwinEnvironmentKind TargetEnvironment; /// The OS version we are targeting. mutable VersionTuple TargetVersion; @@ -317,32 +320,34 @@ // FIXME: Eliminate these ...Target functions and derive separate tool chains // for these targets and put version in constructor. - void setTarget(DarwinPlatformKind Platform, unsigned Major, unsigned Minor, - unsigned Micro) const { + void setTarget(DarwinPlatformKind Platform, DarwinEnvironmentKind Environment, + unsigned Major, unsigned Minor, unsigned Micro) const { // FIXME: For now, allow reinitialization as long as values don't // change. This will go away when we move away from argument translation. if (TargetInitialized && TargetPlatform == Platform && +TargetEnvironment == Environment && TargetVersion == VersionTuple(Major, Minor, Micro)) return; assert(!TargetInitialized && "Target already initialized!"); TargetInitialized = true; TargetPlatform = Platform; +TargetEnvironment = Environment; TargetVersion = VersionTuple(Major, Minor, Micro); -if (Platform == IPhoneOSSimulator || Platform == TvOSSimulator || -Platform == WatchOSSimulator) +if (Environment == Simulator) const_cast(this)->setTripleEnvironment(llvm::Triple::Simulator); } bool isTargetIPhoneOS() const { assert(TargetInitialized && "Target not initialized!"); -return TargetPlatform == IPhoneOS || TargetPlatform == TvOS; +return (TargetPlatform == IPhoneOS || TargetPlatform == TvOS) && + TargetEnvironment == NativeEnvironment; } bool isTargetIOSSimulator() const { assert(TargetInitialized && "Target not initialized!"); -return TargetPlatform == IPhoneOSSimulator || - TargetPlatform == TvOSSimulator; +return (TargetPlatform == IPhoneOS || TargetPlatform == TvOS) && + TargetEnvironment == Simulator; } bool isTargetIOSBased() const { @@ -352,32 +357,32 @@ bool isTargetTvOS() const { assert(TargetInitialized && "Target not initialized!"); -return TargetPlatform == TvOS; +return TargetPlatform == TvOS && TargetEnvironment == NativeEnvironment; } bool isTargetTvOSSimulator() const { assert(TargetInitialized && "Target not initialized!"); -return TargetPlatform == TvOSSimulator; +return TargetPlatform == TvOS && TargetEnvironment == Simulator; } bool isTargetTvOSBased() const { assert(TargetInitialized && "Target not initialized!"); -return TargetPlatform == TvOS || TargetPlatform == TvOSSimulator; +return TargetPlatform == TvOS; } bool isTargetWatchOS() const { assert(TargetInitialized && "Target not initialized!"); -return TargetPlatform == WatchOS; +return TargetPlatform == WatchOS && TargetEnvironment == NativeEnvironment; } bool isTargetWatchOSSimulator() const { assert(TargetInitialized && "Target not initialized!"); -return TargetPlatform == WatchOSSimulator; +return TargetPlatform == WatchOS && TargetEnvironment == Simulator; } bool isTargetWatchOSBased() const { assert(TargetInitialized && "Target not initialized!"); -return TargetPlatform == WatchOS || TargetPlatform == WatchOSSimulator; +return TargetPlatform == WatchOS; } bool isTargetMacOS() const { @@ -413,10 +418,11 @@ Action::OffloadKind DeviceOffloadKind) const override; StringRef getPlatformFamily() const; - static
[PATCH] D41035: [driver][darwin] Refactor the target selection code, NFC
dexonsmith added a comment. This seems correct to me. I wouldn't mind having someone else back me up though. Also, I have a suggestion for a `static_assert`. Comment at: lib/Driver/ToolChains/Darwin.cpp:1334-1339 + const char *EnvVars[Darwin::LastDarwinPlatform + 1] = { + "MACOSX_DEPLOYMENT_TARGET", + "IPHONEOS_DEPLOYMENT_TARGET", + "TVOS_DEPLOYMENT_TARGET", + "WATCHOS_DEPLOYMENT_TARGET", + }; It would be nice to have a `static_assert` that `EnvVars` has the right number of elements. Perhaps something like this would work: ``` const char *EnvVars[] = { /* etc. */ }; static_assert(std::end(EnvVars) - std::begin(EndVars) == Darwin::LastDarwinPlatform + 1, "missing platform"); ``` Repository: rC Clang https://reviews.llvm.org/D41035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41035: [driver][darwin] Refactor the target selection code, NFC
arphaman created this revision. This patch refactors the target selection in Darwin's driver. Firstly, the simulator variant of Darwin's platforms is removed in favor of a new environment field. Secondly, the code that selects the platform and the version is split into 4 different functions instead of being all in one function. This is an NFC patch, although it slightly improves the "invalid version number" diagnostic by displaying the environment variable instead of -m-version-min if the OS version was derived from the environment. This patch is a preparation for making `-target` the canonical way of specifying the platform. This will be done in a follow-up patch https://reviews.llvm.org/D40998. Repository: rC Clang https://reviews.llvm.org/D41035 Files: lib/Driver/ToolChains/Darwin.cpp lib/Driver/ToolChains/Darwin.h test/Driver/darwin-version.c Index: test/Driver/darwin-version.c === --- test/Driver/darwin-version.c +++ test/Driver/darwin-version.c @@ -139,3 +139,8 @@ // RUN: %clang -target x86_64-apple-watchos4.0 -c %s -### 2>&1 | \ // RUN: FileCheck --check-prefix=CHECK-VERSION-WATCHOS-TARGET %s // CHECK-VERSION-WATCHOS-TARGET: "x86_64-apple-watchos4.0.0-simulator" + +// RUN: env MACOSX_DEPLOYMENT_TARGET=1000.1000 \ +// RUN: %clang -c %s -### 2>&1 | \ +// RUN: FileCheck --check-prefix=CHECK-VERSION-INVALID-ENV %s +// CHECK-VERSION-INVALID-ENV: invalid version number in 'MACOSX_DEPLOYMENT_TARGET=1000.1000' Index: lib/Driver/ToolChains/Darwin.h === --- lib/Driver/ToolChains/Darwin.h +++ lib/Driver/ToolChains/Darwin.h @@ -268,14 +268,17 @@ enum DarwinPlatformKind { MacOS, IPhoneOS, -IPhoneOSSimulator, TvOS, -TvOSSimulator, WatchOS, -WatchOSSimulator +LastDarwinPlatform = WatchOS + }; + enum DarwinEnvironmentKind { +NativeEnvironment, +Simulator, }; mutable DarwinPlatformKind TargetPlatform; + mutable DarwinEnvironmentKind TargetEnvironment; /// The OS version we are targeting. mutable VersionTuple TargetVersion; @@ -317,32 +320,34 @@ // FIXME: Eliminate these ...Target functions and derive separate tool chains // for these targets and put version in constructor. - void setTarget(DarwinPlatformKind Platform, unsigned Major, unsigned Minor, - unsigned Micro) const { + void setTarget(DarwinPlatformKind Platform, DarwinEnvironmentKind Environment, + unsigned Major, unsigned Minor, unsigned Micro) const { // FIXME: For now, allow reinitialization as long as values don't // change. This will go away when we move away from argument translation. if (TargetInitialized && TargetPlatform == Platform && +TargetEnvironment == Environment && TargetVersion == VersionTuple(Major, Minor, Micro)) return; assert(!TargetInitialized && "Target already initialized!"); TargetInitialized = true; TargetPlatform = Platform; +TargetEnvironment = Environment; TargetVersion = VersionTuple(Major, Minor, Micro); -if (Platform == IPhoneOSSimulator || Platform == TvOSSimulator || -Platform == WatchOSSimulator) +if (Environment == Simulator) const_cast(this)->setTripleEnvironment(llvm::Triple::Simulator); } bool isTargetIPhoneOS() const { assert(TargetInitialized && "Target not initialized!"); -return TargetPlatform == IPhoneOS || TargetPlatform == TvOS; +return (TargetPlatform == IPhoneOS || TargetPlatform == TvOS) && + TargetEnvironment == NativeEnvironment; } bool isTargetIOSSimulator() const { assert(TargetInitialized && "Target not initialized!"); -return TargetPlatform == IPhoneOSSimulator || - TargetPlatform == TvOSSimulator; +return (TargetPlatform == IPhoneOS || TargetPlatform == TvOS) && + TargetEnvironment == Simulator; } bool isTargetIOSBased() const { @@ -352,32 +357,32 @@ bool isTargetTvOS() const { assert(TargetInitialized && "Target not initialized!"); -return TargetPlatform == TvOS; +return TargetPlatform == TvOS && TargetEnvironment == NativeEnvironment; } bool isTargetTvOSSimulator() const { assert(TargetInitialized && "Target not initialized!"); -return TargetPlatform == TvOSSimulator; +return TargetPlatform == TvOS && TargetEnvironment == Simulator; } bool isTargetTvOSBased() const { assert(TargetInitialized && "Target not initialized!"); -return TargetPlatform == TvOS || TargetPlatform == TvOSSimulator; +return TargetPlatform == TvOS; } bool isTargetWatchOS() const { assert(TargetInitialized && "Target not initialized!"); -return TargetPlatform == WatchOS; +return TargetPlatform == WatchOS && TargetEnvironment == NativeEnvironment; } bool isTargetWatchOSSimulator() const { assert(TargetInitialized &&