[PATCH] D41035: [driver][darwin] Refactor the target selection code, NFC

2017-12-08 Thread Alex Lorenz via Phabricator via cfe-commits
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

2017-12-08 Thread Steven Wu via Phabricator via cfe-commits
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

2017-12-08 Thread Alex Lorenz via Phabricator via cfe-commits
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

2017-12-08 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
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

2017-12-08 Thread Alex Lorenz via Phabricator via cfe-commits
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 &&