[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski updated this revision to Diff 506434.
calebzulawski added a comment.

Corrected docs and updated upstream


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-sdk-detect.c
  clang/test/Driver/lit.local.cfg

Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -1,4 +1,5 @@
 from lit.llvm import llvm_config
+import subprocess
 
 config.suffixes = ['.c', '.cpp', '.cppm', '.h', '.m', '.mm', '.S', '.s', '.f90', '.F90', '.f95',
'.cu', '.rs', '.cl', '.clcpp', '.hip', '.hipi', '.hlsl']
@@ -24,3 +25,9 @@
 
 if config.ppc_linux_default_ieeelongdouble:
   config.available_features.add('ppc_linux_default_ieeelongdouble')
+
+if os.path.exists('/usr/bin/xcrun') and sys.platform == 'darwin':
+if subprocess.run(['/usr/bin/xcrun', '--sdk', 'macosx', '--show-sdk-path']).returncode == 0:
+config.available_features.add('macos-sdk')
+if subprocess.run(['/usr/bin/xcrun', '--sdk', 'iphoneos', '--show-sdk-path']).returncode == 0:
+config.available_features.add('ios-sdk')
Index: clang/test/Driver/darwin-sdk-detect.c
===
--- /dev/null
+++ clang/test/Driver/darwin-sdk-detect.c
@@ -0,0 +1,20 @@
+// REQUIRES: system-darwin, ios-sdk, macos-sdk
+
+// Check that we default to running `xcrun --show-sdk-path` if there is no
+// SDKROOT defined in the environment.
+//
+// RUN: env -u SDKROOT %clang --infer-sdkroot-from-xcrun -target x86_64-apple-macos -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XC < %t.log %s
+//
+// CHECK-XC: clang
+// CHECK-XC: "-cc1"
+// CHECK-XC: "-isysroot" "{{.*MacOSX[0-9\.]*\.sdk}}"
+
+// Check once again that we default to running `xcrun`, this time with another target.
+//
+// RUN: env -u SDKROOT %clang --infer-sdkroot-from-xcrun -target arm64-apple-ios -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XC-IOS < %t.log %s
+//
+// CHECK-XC-IOS: clang
+// CHECK-XC-IOS: "-cc1"
+// CHECK-XC-IOS: "-isysroot" "{{.*iPhoneOS[0-9\.]*\.sdk}}"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -18,15 +18,22 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/TargetParser/TargetParser.h"
 #include  // ::getenv
+#include   // std::unique_ptr
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -2098,21 +2105,89 @@
 void Darwin::AddDeploymentTarget(DerivedArgList ) const {
   const OptTable  = getDriver().getOpts();
 
-  // 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.
+  // On Apple platforms, standard headers and libraries are not provided with
+  // the base system (e.g. in /usr/{include,lib}). Instead, they are provided
+  // in various SDKs for the different Apple platforms. Clang needs to know
+  // where that SDK lives, and there are a couple ways this can be achieved:
+  //
+  // (1) If `-isysroot ` is passed explicitly, use that.
   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));
+  }
+
+  // (2) If the SDKROOT environment variable is defined and points to a valid
+  // path, use that. $SDKROOT is set by `xcrun` and other Xcode tools, so
+  // running `xcrun clang` will work by going through this path.
+  else if (char *env = ::getenv("SDKROOT")) {
+// We only use this value as the default if it is an 

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added a comment.

In D136315#4070121 , @steven_wu wrote:

> I definitely feel like if to fix your problem, a reverse option like 
> `-infer-sdkroot-from-xcrun` might be better/safer but it doesn't help the 
> general case where people doesn't know `/usr/bin/clang` is different from a 
> regular clang that a regular clang needs to pass -isysroot.

I reversed the option for now since it seems the least objectionable.  I figure 
a future change could make this behavior automatic, or maybe even a 
compile-time configurable option?  Regarding the shim, if this option were to 
override `SDKROOT` it would still work with the shim, though I'm not sure which 
should be higher priority.  It would run `xcrun` twice, which isn't so good, 
but perhaps Apple could make use of this behavior in the shim, so it works with 
`--target`...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski updated this revision to Diff 490999.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-sdk-detect.c
  clang/test/Driver/lit.local.cfg

Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -1,4 +1,5 @@
 from lit.llvm import llvm_config
+import subprocess
 
 config.suffixes = ['.c', '.cpp', '.cppm', '.h', '.m', '.mm', '.S', '.s', '.f90', '.F90', '.f95',
'.cu', '.rs', '.cl', '.clcpp', '.hip', '.hipi', '.hlsl']
@@ -24,3 +25,9 @@
 
 if config.ppc_linux_default_ieeelongdouble:
   config.available_features.add('ppc_linux_default_ieeelongdouble')
+
+if os.path.exists('/usr/bin/xcrun') and sys.platform == 'darwin':
+if subprocess.run(['/usr/bin/xcrun', '--sdk', 'macosx', '--show-sdk-path']).returncode == 0:
+config.available_features.add('macos-sdk')
+if subprocess.run(['/usr/bin/xcrun', '--sdk', 'iphoneos', '--show-sdk-path']).returncode == 0:
+config.available_features.add('ios-sdk')
Index: clang/test/Driver/darwin-sdk-detect.c
===
--- /dev/null
+++ clang/test/Driver/darwin-sdk-detect.c
@@ -0,0 +1,20 @@
+// REQUIRES: system-darwin, ios-sdk, macos-sdk
+
+// Check that we default to running `xcrun --show-sdk-path` if there is no
+// SDKROOT defined in the environment.
+//
+// RUN: env -u SDKROOT %clang --infer-sdkroot-from-xcrun -target x86_64-apple-macos -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XC < %t.log %s
+//
+// CHECK-XC: clang
+// CHECK-XC: "-cc1"
+// CHECK-XC: "-isysroot" "{{.*MacOSX[0-9\.]*\.sdk}}"
+
+// Check once again that we default to running `xcrun`, this time with another target.
+//
+// RUN: env -u SDKROOT %clang --infer-sdkroot-from-xcrun -target arm64-apple-ios -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XC-IOS < %t.log %s
+//
+// CHECK-XC-IOS: clang
+// CHECK-XC-IOS: "-cc1"
+// CHECK-XC-IOS: "-isysroot" "{{.*iPhoneOS[0-9\.]*\.sdk}}"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -18,15 +18,22 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include  // ::getenv
+#include   // std::unique_ptr
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -2078,21 +2085,89 @@
 void Darwin::AddDeploymentTarget(DerivedArgList ) const {
   const OptTable  = getDriver().getOpts();
 
-  // 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.
+  // On Apple platforms, standard headers and libraries are not provided with
+  // the base system (e.g. in /usr/{include,lib}). Instead, they are provided
+  // in various SDKs for the different Apple platforms. Clang needs to know
+  // where that SDK lives, and there are a couple ways this can be achieved:
+  //
+  // (1) If `-isysroot ` is passed explicitly, use that.
   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));
+  }
+
+  // (2) If the SDKROOT environment variable is defined and points to a valid
+  // path, use that. $SDKROOT is set by `xcrun` and other Xcode tools, so
+  // running `xcrun clang` will work by going through this path.
+  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) 

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added inline comments.



Comment at: clang/test/Driver/darwin-sdk-detect.c:1
+// REQUIRES: system-darwin
+

steven_wu wrote:
> This test won't work in all conditions. Like I said, you requires to have a 
> Xcode/CommandLineTools installation to make `xcrun` work.
> 
> Maybe you can make the argument that you are compiling clang, so you must 
> have one of those installed, but CommandLineTools definitely doesn't have iOS 
> SDK for the second test.
True, maybe we can just remove the iOS test if you think that's sufficient.  
Otherwise, is there any way to make the have the test check that either the 
`-isysroot` flag is present with that pattern, or not present at all?  I'm not 
too familiar with llvm-lit, I just browsed a few other examples before writing 
this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added a comment.

In my situation, at least, I am the vendor of the toolchain and my 
configuration file contains `--sysroot=/../path/to/sysroot` with a 
known relative path to where the sysroot is distributed.  Apple is unique in 
this situation, since I am not distributing the sysroot, so there is no 
equivalent to using the `` variable.  There is no way to invoke `xcrun` 
or set `SDKROOT` from config files, only pass flags.

It is true that the build system could do it, but I think it's reasonable for 
the driver to make a guess when not provided.  I don't think it's all that 
different than detection of libstdc++ from GCC installs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added a comment.

If `SDKROOT` is set or `-isysroot` is passed, `xcrun` won't be called so there 
is no regression for existing use cases like CMake.  Additionally, if `xcrun` 
isn't found, no error is emitted.  I also added the `--no-detect-xcode` flag 
based on similar feedback that it may not always be desirable.

Better diagnostics would certainly be helpful, but for my particular use case, 
a diagnostic wouldn't be sufficient.  I am creating a set of default 
configuration files for cross compilation, e.g. 
`aarch64-android-linux-clang++.cfg` contains `-isysroot` with a known fixed 
path.  This allows me to pass `clang++ --target=aarch64-android-linux` with no 
additional flags to cross compile.  I would like to do the same for Apple 
targets (not with the system compiler), but since the path is not fixed I 
cannot embed `-isysroot`.  If there is enough opposition to this being default 
behavior, I'd be open to it being opt-in with something like `--detect-xcode`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added a comment.

That makes more sense, I thought perhaps it was using `DEFAULT_SYSROOT`.  The 
shim isn't smart enough to choose the sysroot from the target unfortunately.

It looks like the only error is unrelated to this change, something with 
concepts in libc++.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski updated this revision to Diff 490698.
calebzulawski added a comment.

This revision is basically the same as before, but with two changes:

- `darwin` targets (e.g. `x86_64-apple-darwin`) do not automatically detect the 
SDK. There is another bug where these targets don't seem to properly pass the 
target version (e.g. `x86_64-apple-darwin9`) whenever an SDK is provided.
- A new flag `--no-detect-xcode` disables this detection.

Fingers crossed, this passes all tests for me locally.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-ld-platform-version-macos.c
  clang/test/Driver/darwin-sdk-detect.c

Index: clang/test/Driver/darwin-sdk-detect.c
===
--- /dev/null
+++ clang/test/Driver/darwin-sdk-detect.c
@@ -0,0 +1,20 @@
+// REQUIRES: system-darwin
+
+// Check that we default to running `xcrun --show-sdk-path` if there is no
+// SDKROOT defined in the environment.
+//
+// RUN: env -u SDKROOT %clang -target x86_64-apple-macos -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XC < %t.log %s
+//
+// CHECK-XC: clang
+// CHECK-XC: "-cc1"
+// CHECK-XC: "-isysroot" "{{.*MacOSX[0-9\.]*\.sdk}}"
+
+// Check once again that we default to running `xcrun`, this time with another target.
+//
+// RUN: env -u SDKROOT %clang -target arm64-apple-ios -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XC-IOS < %t.log %s
+//
+// CHECK-XC-IOS: clang
+// CHECK-XC-IOS: "-cc1"
+// CHECK-XC-IOS: "-isysroot" "{{.*iPhoneOS[0-9\.]*\.sdk}}"
Index: clang/test/Driver/darwin-ld-platform-version-macos.c
===
--- clang/test/Driver/darwin-ld-platform-version-macos.c
+++ clang/test/Driver/darwin-ld-platform-version-macos.c
@@ -41,7 +41,7 @@
 // ARM64_NEW_1: "-platform_version" "macos" "11.1.0" "10.14"
 // ARM64_OLD: "-macosx_version_min" "11.0.0"
 
-// RUN: %clang -target x86_64-apple-macos10.13 -mlinker-version=520 \
+// RUN: %clang --no-detect-xcode -target x86_64-apple-macos10.13 -mlinker-version=520 \
 // RUN:   -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOSDK %s
 // RUN: %clang -target x86_64-apple-darwin17 -mlinker-version=520 \
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -18,15 +18,22 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include  // ::getenv
+#include   // std::unique_ptr
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -2078,21 +2085,90 @@
 void Darwin::AddDeploymentTarget(DerivedArgList ) const {
   const OptTable  = getDriver().getOpts();
 
-  // 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.
+  // On Apple platforms, standard headers and libraries are not provided with
+  // the base system (e.g. in /usr/{include,lib}). Instead, they are provided
+  // in various SDKs for the different Apple platforms. Clang needs to know
+  // where that SDK lives, and there are a couple ways this can be achieved:
+  //
+  // (1) If `-isysroot ` is passed explicitly, use that.
   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));
+  }
+
+  // (2) If the SDKROOT environment variable is defined and points to a valid
+  // path, use that. $SDKROOT is set by `xcrun` and other Xcode tools, so
+  // running `xcrun clang` will work by going through this path.
+  else if (char *env = 

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski reopened this revision.
calebzulawski added a comment.
This revision is now accepted and ready to land.

@thakis thanks. I have an updated revision that can be reviewed properly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added a comment.

@jryans thanks.

I've confirmed that this is specifically a bug with `darwin` targets (not 
`macos`) not respecting platform versions when an SDK is present, I can even 
reproduce this with clang provided with Xcode.

The following patch, for example, completely resolves all test errors for me:

  diff
  diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp 
b/clang/lib/Driver/ToolChains/Darwin.cpp
  index 03c28c14a0ec..a177a1d289b4 100644
  --- a/clang/lib/Driver/ToolChains/Darwin.cpp
  +++ b/clang/lib/Driver/ToolChains/Darwin.cpp
  @@ -2141,13 +2141,14 @@ void Darwin::AddDeploymentTarget(DerivedArgList 
) const {
 else
   SDKName = "iphoneos";
 break;
  -case llvm::Triple::OSType::Darwin:
   case llvm::Triple::OSType::MacOSX:
 SDKName = "macosx";
 break;
   case llvm::Triple::OSType::DriverKit:
 SDKName = "driverkit";
 break;
  +case llvm::Triple::OSType::Darwin:
  +  break;
   default:
 llvm_unreachable("unknown kind of Darwin platform");
   }
  diff --git a/clang/test/Driver/darwin-ld-platform-version-macos.c 
b/clang/test/Driver/darwin-ld-platform-version-macos.c
  index 355df8dfc1bc..5e50d84df6da 100644
  --- a/clang/test/Driver/darwin-ld-platform-version-macos.c
  +++ b/clang/test/Driver/darwin-ld-platform-version-macos.c
  @@ -41,9 +41,6 @@
   // ARM64_NEW_1: "-platform_version" "macos" "11.1.0" "10.14"
   // ARM64_OLD: "-macosx_version_min" "11.0.0"
  
  -// RUN: %clang -target x86_64-apple-macos10.13 -mlinker-version=520 \
  -// RUN:   -### %t.o 2>&1 \
  -// RUN:   | FileCheck --check-prefix=NOSDK %s
   // RUN: %clang -target x86_64-apple-darwin17 -mlinker-version=520 \
   // RUN:   -### %t.o 2>&1 \
   // RUN:   | FileCheck --check-prefix=NOSDK %s
  diff --git a/clang/test/Driver/darwin-sdk-detect.c 
b/clang/test/Driver/darwin-sdk-detect.c
  index dff4def0568a..c5b270c526bd 100644
  --- a/clang/test/Driver/darwin-sdk-detect.c
  +++ b/clang/test/Driver/darwin-sdk-detect.c
  @@ -3,7 +3,7 @@
   // Check that we default to running `xcrun --show-sdk-path` if there is no
   // SDKROOT defined in the environment.
   //
  -// RUN: env -u SDKROOT %clang -target x86_64-apple-darwin -c %s -### 2> 
%t.log
  +// RUN: env -u SDKROOT %clang -target x86_64-apple-macos -c %s -### 2> %t.log
   // RUN: FileCheck --check-prefix=CHECK-XC < %t.log %s
   //
   // CHECK-XC: clang


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added a comment.

One thing to throw into the mix: Apple's clang has a default sysroot 
configured, so with the default system compiler, there is no way to replicate 
this "build without a sysroot" scenario as far as I can tell.  For the system 
compiler, I believe this behavior is a strict improvement.

Looking at all of the errors, it looks like the common thread is that when a 
sysroot is present, the version in the target triple (e.g. 
`x86_64-apple-macos10`) isn't respected.  If this is true, I think this 
possibly uncovered an existing bug...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added a comment.

In D136315#4065481 , @thakis wrote:

> In D136315#4065426 , @calebzulawski 
> wrote:
>
>> I don't think we necessarily need to revert due to the xcrun quirk since I 
>> think it's harmless, though I can provide a follow up change if necessary.  
>> I just checked Rust's source as a comparison and it appears that for macOS 
>> targets it will always attempt to invoke xcrun regardless of host as well.
>
> Needlessly spawning additional processes doesn't seem "harmless" to me.
>
> If you can fix quickly, sure, do that.
>
>> I could see a macOS cross-compiler environment providing an xcrun 
>> replacement, for example.
>
> That's theoretically possible I suppose, but it doesn't exist. No mac cross 
> compiling uses `xcrun`.

Sorry, I agree that we shouldn't do things needlessly, but xcrun is only 
invoked if the compiler otherwise can't figure out the sysroot.  I just tried 
using `--target=x86_64-apple-macos` on its own on Linux and sure enough the 
sysroot included `/usr/include` etc, which is surely also wrong.  I suspect any 
existing cross-compiler environments either set SDKROOT or pass a sysroot flag, 
otherwise they wouldn't work (and this change doesn't affect them).

It seems osxcross does provide an xcrun replacement: 
https://github.com/tpoechtrager/osxcross/blob/master/wrapper/programs/xcrun.cpp

>> The test failures are another issue but I still don't quite understand the 
>> cause.
>
> I can investigate a bit more. Which version of macOS are you running? Are you 
> using system python3 or brew python3? I know that system python3 sets SDKROOT 
> by default on macOS 13+ for some reason, so maybe that's related. (The bot 
> runs macOS 13.)

I'm using 13.1 and I do have brew python installed.  I see what you're 
describing, system python has SDKROOT set.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added a comment.

I don't think we necessarily need to revert due to the xcrun quirk since I 
think it's harmless, though I can provide a follow up change if necessary.  I 
just checked Rust's source as a comparison and it appears that for macOS 
targets it will always attempt to invoke xcrun regardless of host as well.  I 
could see a macOS cross-compiler environment providing an xcrun replacement, 
for example.  The test failures are another issue but I still don't quite 
understand the cause.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added a comment.

Are those failures on Linux? I only developed this change on macOS, but the 
automated tests passed...

Admittedly I'm not sure what should happen when building for macOS from Linux 
(is there any use case where that works and does anything useful? I'm not 
sure).  I was under the impression that the Darwin driver applied to Darwin 
hosts, not targets, but if that's not true, it would attempt to run xcrun. If 
xcrun isn't found, no error occurs but the sysroot remains unset.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added a comment.

@jryans I do not, I would appreciate that. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added a comment.

Happy new year all--
Is there anything left for me to do before this can be merged? Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski updated this revision to Diff 483697.
calebzulawski added a comment.

Looks like the failure was somehow in clangd?  Trying to trigger another build.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

Files:
  clang/docs/UsersManual.rst
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-sdk-detect.c

Index: clang/test/Driver/darwin-sdk-detect.c
===
--- /dev/null
+++ clang/test/Driver/darwin-sdk-detect.c
@@ -0,0 +1,20 @@
+// REQUIRES: system-darwin
+
+// Check that we default to running `xcrun --show-sdk-path` if there is no
+// SDKROOT defined in the environment.
+//
+// RUN: env -u SDKROOT %clang -target x86_64-apple-darwin -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XC < %t.log %s
+//
+// CHECK-XC: clang
+// CHECK-XC: "-cc1"
+// CHECK-XC: "-isysroot" "{{.*MacOSX[0-9\.]*\.sdk}}"
+
+// Check once again that we default to running `xcrun`, this time with another target.
+//
+// RUN: env -u SDKROOT %clang -target arm64-apple-ios -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XC-IOS < %t.log %s
+//
+// CHECK-XC-IOS: clang
+// CHECK-XC-IOS: "-cc1"
+// CHECK-XC-IOS: "-isysroot" "{{.*iPhoneOS[0-9\.]*\.sdk}}"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -18,15 +18,22 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include  // ::getenv
+#include   // std::unique_ptr
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -2071,21 +2078,89 @@
 void Darwin::AddDeploymentTarget(DerivedArgList ) const {
   const OptTable  = getDriver().getOpts();
 
-  // 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.
+  // On Apple platforms, standard headers and libraries are not provided with
+  // the base system (e.g. in /usr/{include,lib}). Instead, they are provided
+  // in various SDKs for the different Apple platforms. Clang needs to know
+  // where that SDK lives, and there are a couple ways this can be achieved:
+  //
+  // (1) If `-isysroot ` is passed explicitly, use that.
   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));
+  }
+
+  // (2) If the SDKROOT environment variable is defined and points to a valid
+  // path, use that. $SDKROOT is set by `xcrun` and other Xcode tools, so
+  // running `xcrun clang` will work by going through this path.
+  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));
+}
+  }
+
+  // (3) Otherwise, we try to guess the path of the default SDK by running
+  // `xcrun --show-sdk-path`. This won't always be correct, but if the
+  //  user wants to use the non-default SDK, they should specify it
+  //  explicitly with methods (1) or (2) above.
+  else {
+llvm::SmallString<64> OutputFile;
+llvm::sys::fs::createTemporaryFile("print-sdk-path", "" /* No Suffix */,
+   OutputFile);
+llvm::FileRemover OutputRemover(OutputFile.c_str());
+std::optional Redirects[] = {{""}, OutputFile.str(), {""}};
+
+Optional SDKName = std::nullopt;
+switch (getTriple().getOS()) {
+case llvm::Triple::OSType::WatchOS:
+  if (getTriple().isSimulatorEnvironment())
+   

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski updated this revision to Diff 483416.
calebzulawski added a comment.

Update user manual


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

Files:
  clang/docs/UsersManual.rst
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-sdk-detect.c

Index: clang/test/Driver/darwin-sdk-detect.c
===
--- /dev/null
+++ clang/test/Driver/darwin-sdk-detect.c
@@ -0,0 +1,20 @@
+// REQUIRES: system-darwin
+
+// Check that we default to running `xcrun --show-sdk-path` if there is no
+// SDKROOT defined in the environment.
+//
+// RUN: env -u SDKROOT %clang -target x86_64-apple-darwin -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XC < %t.log %s
+//
+// CHECK-XC: clang
+// CHECK-XC: "-cc1"
+// CHECK-XC: "-isysroot" "{{.*MacOSX[0-9\.]*\.sdk}}"
+
+// Check once again that we default to running `xcrun`, this time with another target.
+//
+// RUN: env -u SDKROOT %clang -target arm64-apple-ios -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XC-IOS < %t.log %s
+//
+// CHECK-XC-IOS: clang
+// CHECK-XC-IOS: "-cc1"
+// CHECK-XC-IOS: "-isysroot" "{{.*iPhoneOS[0-9\.]*\.sdk}}"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -18,15 +18,22 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include  // ::getenv
+#include   // std::unique_ptr
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -2071,21 +2078,89 @@
 void Darwin::AddDeploymentTarget(DerivedArgList ) const {
   const OptTable  = getDriver().getOpts();
 
-  // 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.
+  // On Apple platforms, standard headers and libraries are not provided with
+  // the base system (e.g. in /usr/{include,lib}). Instead, they are provided
+  // in various SDKs for the different Apple platforms. Clang needs to know
+  // where that SDK lives, and there are a couple ways this can be achieved:
+  //
+  // (1) If `-isysroot ` is passed explicitly, use that.
   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));
+  }
+
+  // (2) If the SDKROOT environment variable is defined and points to a valid
+  // path, use that. $SDKROOT is set by `xcrun` and other Xcode tools, so
+  // running `xcrun clang` will work by going through this path.
+  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));
+}
+  }
+
+  // (3) Otherwise, we try to guess the path of the default SDK by running
+  // `xcrun --show-sdk-path`. This won't always be correct, but if the
+  //  user wants to use the non-default SDK, they should specify it
+  //  explicitly with methods (1) or (2) above.
+  else {
+llvm::SmallString<64> OutputFile;
+llvm::sys::fs::createTemporaryFile("print-sdk-path", "" /* No Suffix */,
+   OutputFile);
+llvm::FileRemover OutputRemover(OutputFile.c_str());
+std::optional Redirects[] = {{""}, OutputFile.str(), {""}};
+
+Optional SDKName = std::nullopt;
+switch (getTriple().getOS()) {
+case llvm::Triple::OSType::WatchOS:
+  if (getTriple().isSimulatorEnvironment())
+SDKName = "watchsimulator";
+  else
+

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski updated this revision to Diff 482697.
calebzulawski added a comment.

Update diff for upstream changes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-sdk-detect.c

Index: clang/test/Driver/darwin-sdk-detect.c
===
--- /dev/null
+++ clang/test/Driver/darwin-sdk-detect.c
@@ -0,0 +1,20 @@
+// REQUIRES: system-darwin
+
+// Check that we default to running `xcrun --show-sdk-path` if there is no
+// SDKROOT defined in the environment.
+//
+// RUN: env -u SDKROOT %clang -target x86_64-apple-darwin -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XC < %t.log %s
+//
+// CHECK-XC: clang
+// CHECK-XC: "-cc1"
+// CHECK-XC: "-isysroot" "{{.*MacOSX[0-9\.]*\.sdk}}"
+
+// Check once again that we default to running `xcrun`, this time with another target.
+//
+// RUN: env -u SDKROOT %clang -target arm64-apple-ios -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XC-IOS < %t.log %s
+//
+// CHECK-XC-IOS: clang
+// CHECK-XC-IOS: "-cc1"
+// CHECK-XC-IOS: "-isysroot" "{{.*iPhoneOS[0-9\.]*\.sdk}}"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -18,15 +18,22 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include  // ::getenv
+#include   // std::unique_ptr
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -2071,21 +2078,89 @@
 void Darwin::AddDeploymentTarget(DerivedArgList ) const {
   const OptTable  = getDriver().getOpts();
 
-  // 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.
+  // On Apple platforms, standard headers and libraries are not provided with
+  // the base system (e.g. in /usr/{include,lib}). Instead, they are provided
+  // in various SDKs for the different Apple platforms. Clang needs to know
+  // where that SDK lives, and there are a couple ways this can be achieved:
+  //
+  // (1) If `-isysroot ` is passed explicitly, use that.
   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));
+  }
+
+  // (2) If the SDKROOT environment variable is defined and points to a valid
+  // path, use that. $SDKROOT is set by `xcrun` and other Xcode tools, so
+  // running `xcrun clang` will work by going through this path.
+  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));
+}
+  }
+
+  // (3) Otherwise, we try to guess the path of the default SDK by running
+  // `xcrun --show-sdk-path`. This won't always be correct, but if the
+  //  user wants to use the non-default SDK, they should specify it
+  //  explicitly with methods (1) or (2) above.
+  else {
+llvm::SmallString<64> OutputFile;
+llvm::sys::fs::createTemporaryFile("print-sdk-path", "" /* No Suffix */,
+   OutputFile);
+llvm::FileRemover OutputRemover(OutputFile.c_str());
+std::optional Redirects[] = {{""}, OutputFile.str(), {""}};
+
+Optional SDKName = std::nullopt;
+switch (getTriple().getOS()) {
+case llvm::Triple::OSType::WatchOS:
+  if (getTriple().isSimulatorEnvironment())
+SDKName = "watchsimulator";
+  else
+SDKName = "watchos";

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski updated this revision to Diff 482650.
calebzulawski added a comment.

Update comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-sdk-detect.c

Index: clang/test/Driver/darwin-sdk-detect.c
===
--- /dev/null
+++ clang/test/Driver/darwin-sdk-detect.c
@@ -0,0 +1,20 @@
+// REQUIRES: system-darwin
+
+// Check that we default to running `xcrun --show-sdk-path` if there is no
+// SDKROOT defined in the environment.
+//
+// RUN: env -u SDKROOT %clang -target x86_64-apple-darwin -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XC < %t.log %s
+//
+// CHECK-XC: clang
+// CHECK-XC: "-cc1"
+// CHECK-XC: "-isysroot" "{{.*MacOSX[0-9\.]*\.sdk}}"
+
+// Check once again that we default to running `xcrun`, this time with another target.
+//
+// RUN: env -u SDKROOT %clang -target arm64-apple-ios -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XC-IOS < %t.log %s
+//
+// CHECK-XC-IOS: clang
+// CHECK-XC-IOS: "-cc1"
+// CHECK-XC-IOS: "-isysroot" "{{.*iPhoneOS[0-9\.]*\.sdk}}"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -18,15 +18,22 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include  // ::getenv
+#include   // std::unique_ptr
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -2071,21 +2078,90 @@
 void Darwin::AddDeploymentTarget(DerivedArgList ) const {
   const OptTable  = getDriver().getOpts();
 
-  // 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.
+  // On Apple platforms, standard headers and libraries are not provided with
+  // the base system (e.g. in /usr/{include,lib}). Instead, they are provided
+  // in various SDKs for the different Apple platforms. Clang needs to know
+  // where that SDK lives, and there are a couple ways this can be achieved:
+  //
+  // (1) If `-isysroot ` is passed explicitly, use that.
   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));
+  }
+
+  // (2) If the SDKROOT environment variable is defined and points to a valid
+  // path, use that. $SDKROOT is set by `xcrun` and other Xcode tools, so
+  // running `xcrun clang` will work by going through this path.
+  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));
+}
+  }
+
+  // (3) Otherwise, we try to guess the path of the default SDK by running
+  // `xcrun --show-sdk-path`. This won't always be correct, but if the
+  //  user wants to use the non-default SDK, they should specify it
+  //  explicitly with methods (1) or (2) above.
+  else {
+llvm::SmallString<64> OutputFile;
+llvm::sys::fs::createTemporaryFile("print-sdk-path", "" /* No Suffix */,
+   OutputFile);
+llvm::FileRemover OutputRemover(OutputFile.c_str());
+llvm::Optional Redirects[] = {
+{""}, OutputFile.str(), {""}};
+
+Optional SDKName = None;
+switch (getTriple().getOS()) {
+case llvm::Triple::OSType::WatchOS:
+  if (getTriple().isSimulatorEnvironment())
+SDKName = "watchsimulator";
+  else
+SDKName = "watchos";
+  break;
+ 

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added a comment.

Is there any update to this? Or maybe someone I should add as a reviewer? 
Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski updated this revision to Diff 469987.
calebzulawski added a comment.

Move SDK detection to its own test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-sdk-detect.c

Index: clang/test/Driver/darwin-sdk-detect.c
===
--- /dev/null
+++ clang/test/Driver/darwin-sdk-detect.c
@@ -0,0 +1,20 @@
+// REQUIRES: system-darwin
+
+// Check that we default to running `xcrun --show-sdk-path` if there is no
+// SDKROOT defined in the environment.
+//
+// RUN: env -u SDKROOT %clang -target x86_64-apple-darwin -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XC < %t.log %s
+//
+// CHECK-XC: clang
+// CHECK-XC: "-cc1"
+// CHECK-XC: "-isysroot" "{{.*MacOSX[0-9\.]*\.sdk}}"
+
+// Check once again that we default to running `xcrun`, this time with another target.
+//
+// RUN: env -u SDKROOT %clang -target arm64-apple-ios -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XC-IOS < %t.log %s
+//
+// CHECK-XC-IOS: clang
+// CHECK-XC-IOS: "-cc1"
+// CHECK-XC-IOS: "-isysroot" "{{.*iPhoneOS[0-9\.]*\.sdk}}"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -18,15 +18,22 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include  // ::getenv
+#include   // std::unique_ptr
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -2074,21 +2081,90 @@
 void Darwin::AddDeploymentTarget(DerivedArgList ) const {
   const OptTable  = getDriver().getOpts();
 
-  // 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.
+  // On Apple platforms, C and C++ Standard Library headers are not provided
+  // with the base system. Instead, they are provided in various SDKs for the
+  // different Apple platforms. Clang needs to know where that SDK lives, and
+  // there are a couple ways this can be achieved:
+  //
+  // (1) If `-isysroot ` is passed explicitly, use that.
   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));
+  }
+
+  // (2) If the SDKROOT environment variable is defined and points to a valid
+  // path, use that. $SDKROOT is set by `xcrun` and other Xcode tools, so
+  // running `xcrun clang` will work by going through this path.
+  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));
+}
+  }
+
+  // (3) Otherwise, we try to guess the path of the default SDK by running
+  // `xcrun --show-sdk-path`. This won't always be correct, but if the
+  //  user wants to use the non-default SDK, they should specify it
+  //  explicitly with methods (1) or (2) above.
+  else {
+llvm::SmallString<64> OutputFile;
+llvm::sys::fs::createTemporaryFile("print-sdk-path", "" /* No Suffix */,
+   OutputFile);
+llvm::FileRemover OutputRemover(OutputFile.c_str());
+llvm::Optional Redirects[] = {
+{""}, OutputFile.str(), {""}};
+
+Optional SDKName = None;
+switch (getTriple().getOS()) {
+case llvm::Triple::OSType::WatchOS:
+  if (getTriple().isSimulatorEnvironment())
+SDKName = "watchsimulator";
+  else
+SDKName = "watchos";
+  break;
+

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski updated this revision to Diff 469953.
calebzulawski added a comment.

Try to fix windows test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-sdkroot.c

Index: clang/test/Driver/darwin-sdkroot.c
===
--- clang/test/Driver/darwin-sdkroot.c
+++ clang/test/Driver/darwin-sdkroot.c
@@ -1,4 +1,5 @@
-// Check that SDKROOT is used to define the default for -isysroot on Darwin.
+// Check that SDKROOT is used to define the default for -isysroot on Darwin,
+// or that we call `xcrun --show-sdk-path` if SDKROOT is not in the environment.
 //
 // RUN: rm -rf %t.tmpdir
 // RUN: mkdir -p %t.tmpdir
@@ -39,6 +40,25 @@
 //
 // This test do pass using GnuWin32 env.exe.
 
+// Check that we default to running `xcrun --show-sdk-path` if there is no
+// SDKROOT defined in the environment.
+//
+// RUN: env -u SDKROOT %clang -target x86_64-apple-darwin -c %s -### 2> %t.log
+// RUN: if [[ -f "/usr/bin/xcrun" ]]; then FileCheck --check-prefix=CHECK-XC < %t.log %s; fi
+//
+// CHECK-XC: clang
+// CHECK-XC: "-cc1"
+// CHECK-XC: "-isysroot" "{{.*MacOSX[0-9\.]*\.sdk}}"
+
+// Check once again that we default to running `xcrun`, this time with another target.
+//
+// RUN: env -u SDKROOT %clang -target arm64-apple-ios -c %s -### 2> %t.log
+// RUN: if [[ -f "/usr/bin/xcrun" ]]; then FileCheck --check-prefix=CHECK-XC-IOS < %t.log %s; fi
+//
+// CHECK-XC-IOS: clang
+// CHECK-XC-IOS: "-cc1"
+// CHECK-XC-IOS: "-isysroot" "{{.*iPhoneOS[0-9\.]*\.sdk}}"
+
 // Check if clang set the correct deployment target from -sysroot
 //
 // RUN: rm -rf %t/SDKs/iPhoneOS8.0.0.sdk
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -18,15 +18,22 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include  // ::getenv
+#include   // std::unique_ptr
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -2074,21 +2081,90 @@
 void Darwin::AddDeploymentTarget(DerivedArgList ) const {
   const OptTable  = getDriver().getOpts();
 
-  // 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.
+  // On Apple platforms, C and C++ Standard Library headers are not provided
+  // with the base system. Instead, they are provided in various SDKs for the
+  // different Apple platforms. Clang needs to know where that SDK lives, and
+  // there are a couple ways this can be achieved:
+  //
+  // (1) If `-isysroot ` is passed explicitly, use that.
   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));
+  }
+
+  // (2) If the SDKROOT environment variable is defined and points to a valid
+  // path, use that. $SDKROOT is set by `xcrun` and other Xcode tools, so
+  // running `xcrun clang` will work by going through this path.
+  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));
+}
+  }
+
+  // (3) Otherwise, we try to guess the path of the default SDK by running
+  // `xcrun --show-sdk-path`. This won't always be correct, but if the
+  //  user wants to use the non-default SDK, they should specify it
+  //  explicitly with methods (1) or (2) above.
+  else {
+llvm::SmallString<64> 

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski updated this revision to Diff 469948.
calebzulawski added a comment.

Fix test using `RUN` in `CHECK` prefix


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-sdkroot.c

Index: clang/test/Driver/darwin-sdkroot.c
===
--- clang/test/Driver/darwin-sdkroot.c
+++ clang/test/Driver/darwin-sdkroot.c
@@ -1,4 +1,5 @@
-// Check that SDKROOT is used to define the default for -isysroot on Darwin.
+// Check that SDKROOT is used to define the default for -isysroot on Darwin,
+// or that we call `xcrun --show-sdk-path` if SDKROOT is not in the environment.
 //
 // RUN: rm -rf %t.tmpdir
 // RUN: mkdir -p %t.tmpdir
@@ -39,6 +40,25 @@
 //
 // This test do pass using GnuWin32 env.exe.
 
+// Check that we default to running `xcrun --show-sdk-path` if there is no
+// SDKROOT defined in the environment.
+//
+// RUN: env -u SDKROOT %clang -target x86_64-apple-darwin -c %s -### 2> %t.log
+// RUN: if test -f /usr/bin/xcrun; then FileCheck --check-prefix=CHECK-XC < %t.log %s; fi
+//
+// CHECK-XC: clang
+// CHECK-XC: "-cc1"
+// CHECK-XC: "-isysroot" "{{.*MacOSX[0-9\.]*\.sdk}}"
+
+// Check once again that we default to running `xcrun`, this time with another target.
+//
+// RUN: env -u SDKROOT %clang -target arm64-apple-ios -c %s -### 2> %t.log
+// RUN: if test -f /usr/bin/xcrun; then FileCheck --check-prefix=CHECK-XC-IOS < %t.log %s; fi
+//
+// CHECK-XC-IOS: clang
+// CHECK-XC-IOS: "-cc1"
+// CHECK-XC-IOS: "-isysroot" "{{.*iPhoneOS[0-9\.]*\.sdk}}"
+
 // Check if clang set the correct deployment target from -sysroot
 //
 // RUN: rm -rf %t/SDKs/iPhoneOS8.0.0.sdk
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -18,15 +18,22 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include  // ::getenv
+#include   // std::unique_ptr
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -2074,21 +2081,90 @@
 void Darwin::AddDeploymentTarget(DerivedArgList ) const {
   const OptTable  = getDriver().getOpts();
 
-  // 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.
+  // On Apple platforms, C and C++ Standard Library headers are not provided
+  // with the base system. Instead, they are provided in various SDKs for the
+  // different Apple platforms. Clang needs to know where that SDK lives, and
+  // there are a couple ways this can be achieved:
+  //
+  // (1) If `-isysroot ` is passed explicitly, use that.
   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));
+  }
+
+  // (2) If the SDKROOT environment variable is defined and points to a valid
+  // path, use that. $SDKROOT is set by `xcrun` and other Xcode tools, so
+  // running `xcrun clang` will work by going through this path.
+  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));
+}
+  }
+
+  // (3) Otherwise, we try to guess the path of the default SDK by running
+  // `xcrun --show-sdk-path`. This won't always be correct, but if the
+  //  user wants to use the non-default SDK, they should specify it
+  //  explicitly with methods (1) or (2) above.
+  else {
+

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski marked an inline comment as done.
calebzulawski added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2147
+default:
+  // We shouldn't get here, unless the target OS doesn't have an SDK.
+  break;

calebzulawski wrote:
> carlocab wrote:
> > `llvm_unreachable`? Or `assert`, at least.
> I'm not sure that's correct--what if you target something like 
> `x86_64-unknown-none`?  I figured it would be safest to fall back to the 
> previous behavior of simply not providing a sysroot.
Realizing this is never hit for non-darwin targets anyway, added the 
`llvm_unreachable`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski updated this revision to Diff 469940.
calebzulawski added a comment.

Fix formatting, error on unrecognized OS, skip tests if xcrun isn't present.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-sdkroot.c

Index: clang/test/Driver/darwin-sdkroot.c
===
--- clang/test/Driver/darwin-sdkroot.c
+++ clang/test/Driver/darwin-sdkroot.c
@@ -1,4 +1,5 @@
-// Check that SDKROOT is used to define the default for -isysroot on Darwin.
+// Check that SDKROOT is used to define the default for -isysroot on Darwin,
+// or that we call `xcrun --show-sdk-path` if SDKROOT is not in the environment.
 //
 // RUN: rm -rf %t.tmpdir
 // RUN: mkdir -p %t.tmpdir
@@ -39,6 +40,25 @@
 //
 // This test do pass using GnuWin32 env.exe.
 
+// Check that we default to running `xcrun --show-sdk-path` if there is no
+// SDKROOT defined in the environment.
+//
+// RUN: env -u SDKROOT %clang -target x86_64-apple-darwin -c %s -### 2> %t.log
+// RUN: if test -f /usr/bin/xcrun; then FileCheck --check-prefix=CHECK-XCRUN < %t.log %s; fi
+//
+// CHECK-XCRUN: clang
+// CHECK-XCRUN: "-cc1"
+// CHECK-XCRUN: "-isysroot" "{{.*MacOSX[0-9\.]*\.sdk}}"
+
+// Check once again that we default to running `xcrun`, this time with another target.
+//
+// RUN: env -u SDKROOT %clang -target arm64-apple-ios -c %s -### 2> %t.log
+// RUN: if test -f /usr/bin/xcrun; then FileCheck --check-prefix=CHECK-XCRUN-IOS < %t.log %s; fi
+//
+// CHECK-XCRUN-IOS: clang
+// CHECK-XCRUN-IOS: "-cc1"
+// CHECK-XCRUN-IOS: "-isysroot" "{{.*iPhoneOS[0-9\.]*\.sdk}}"
+
 // Check if clang set the correct deployment target from -sysroot
 //
 // RUN: rm -rf %t/SDKs/iPhoneOS8.0.0.sdk
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -18,15 +18,22 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include  // ::getenv
+#include   // std::unique_ptr
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -2074,21 +2081,90 @@
 void Darwin::AddDeploymentTarget(DerivedArgList ) const {
   const OptTable  = getDriver().getOpts();
 
-  // 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.
+  // On Apple platforms, C and C++ Standard Library headers are not provided
+  // with the base system. Instead, they are provided in various SDKs for the
+  // different Apple platforms. Clang needs to know where that SDK lives, and
+  // there are a couple ways this can be achieved:
+  //
+  // (1) If `-isysroot ` is passed explicitly, use that.
   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));
+  }
+
+  // (2) If the SDKROOT environment variable is defined and points to a valid
+  // path, use that. $SDKROOT is set by `xcrun` and other Xcode tools, so
+  // running `xcrun clang` will work by going through this path.
+  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));
+}
+  }
+
+  // (3) Otherwise, we try to guess the path of the default SDK by running
+  // `xcrun --show-sdk-path`. This won't always be correct, but if the
+  //  user wants to use the non-default SDK, they should specify it
+  //  

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2147
+default:
+  // We shouldn't get here, unless the target OS doesn't have an SDK.
+  break;

carlocab wrote:
> `llvm_unreachable`? Or `assert`, at least.
I'm not sure that's correct--what if you target something like 
`x86_64-unknown-none`?  I figured it would be safest to fall back to the 
previous behavior of simply not providing a sysroot.



Comment at: clang/test/Driver/darwin-sdkroot.c:1
-// Check that SDKROOT is used to define the default for -isysroot on Darwin.
+// REQUIRES: system-darwin
+

carlocab wrote:
> I'm not sure this is the right move here, since there are a number of other 
> test cases in this file that don't require `system-darwin`. We probably do 
> want those tests to run on more buildbots to make sure problems with them are 
> caught sooner rather than later.
> 
> Maybe we can check that `/usr/bin/xcrun` is executable instead?
That would work, but I'm not sure how to exclude a particular test if it's not 
found.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2120
+Optional SDKName = None;
+if (getTriple().isWatchOS()) {
+  if (getTriple().isSimulatorEnvironment())

tschuett wrote:
> Will there be an enum over the Apple variants to make this less error prone 
> and future proof? I want a switch statement.
It doesn't look like there is any enum available that _only_ contains Apple OS 
variants, but I did change this to a switch-case over the OS in general.  Also, 
I noticed I missed DriverKit, so I added it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski updated this revision to Diff 469909.
calebzulawski added a comment.

Use switch when detecting OS.  Limit sdkroot test to darwin platforms.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-sdkroot.c

Index: clang/test/Driver/darwin-sdkroot.c
===
--- clang/test/Driver/darwin-sdkroot.c
+++ clang/test/Driver/darwin-sdkroot.c
@@ -1,4 +1,7 @@
-// Check that SDKROOT is used to define the default for -isysroot on Darwin.
+// REQUIRES: system-darwin
+
+// Check that SDKROOT is used to define the default for -isysroot on Darwin,
+// or that we call `xcrun --show-sdk-path` if SDKROOT is not in the environment.
 //
 // RUN: rm -rf %t.tmpdir
 // RUN: mkdir -p %t.tmpdir
@@ -39,6 +42,25 @@
 //
 // This test do pass using GnuWin32 env.exe.
 
+// Check that we default to running `xcrun --show-sdk-path` if there is no
+// SDKROOT defined in the environment.
+//
+// RUN: env -u SDKROOT %clang -target x86_64-apple-darwin -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XCRUN < %t.log %s
+//
+// CHECK-XCRUN: clang
+// CHECK-XCRUN: "-cc1"
+// CHECK-XCRUN: "-isysroot" "{{.*MacOSX[0-9\.]*\.sdk}}"
+
+// Check once again that we default to running `xcrun`, this time with another target.
+//
+// RUN: env -u SDKROOT %clang -target arm64-apple-ios -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XCRUN-IOS < %t.log %s
+//
+// CHECK-XCRUN-IOS: clang
+// CHECK-XCRUN-IOS: "-cc1"
+// CHECK-XCRUN-IOS: "-isysroot" "{{.*iPhoneOS[0-9\.]*\.sdk}}"
+
 // Check if clang set the correct deployment target from -sysroot
 //
 // RUN: rm -rf %t/SDKs/iPhoneOS8.0.0.sdk
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -18,15 +18,22 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include  // ::getenv
+#include   // std::unique_ptr
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -2074,21 +2081,89 @@
 void Darwin::AddDeploymentTarget(DerivedArgList ) const {
   const OptTable  = getDriver().getOpts();
 
-  // 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.
+  // On Apple platforms, C and C++ Standard Library headers are not provided
+  // with the base system. Instead, they are provided in various SDKs for the
+  // different Apple platforms. Clang needs to know where that SDK lives, and
+  // there are a couple ways this can be achieved:
+  //
+  // (1) If `-isysroot ` is passed explicitly, use that.
   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));
+  }
+
+  // (2) If the SDKROOT environment variable is defined and points to a valid
+  // path, use that. $SDKROOT is set by `xcrun` and other Xcode tools, so
+  // running `xcrun clang` will work by going through this path.
+  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));
+}
+  }
+
+  // (3) Otherwise, we try to guess the path of the default SDK by running
+  // `xcrun --show-sdk-path`. This won't always be correct, but if the
+  //  user wants to use the non-default SDK, they should specify it
+  //  explicitly with methods (1) or (2) above.
+  else {
+

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski updated this revision to Diff 469901.
calebzulawski added a comment.

Change tests to specifically remove `SDKROOT` environment variable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-sdkroot.c

Index: clang/test/Driver/darwin-sdkroot.c
===
--- clang/test/Driver/darwin-sdkroot.c
+++ clang/test/Driver/darwin-sdkroot.c
@@ -1,4 +1,5 @@
-// Check that SDKROOT is used to define the default for -isysroot on Darwin.
+// Check that SDKROOT is used to define the default for -isysroot on Darwin,
+// or that we call `xcrun --show-sdk-path` if SDKROOT is not in the environment.
 //
 // RUN: rm -rf %t.tmpdir
 // RUN: mkdir -p %t.tmpdir
@@ -39,6 +40,25 @@
 //
 // This test do pass using GnuWin32 env.exe.
 
+// Check that we default to running `xcrun --show-sdk-path` if there is no
+// SDKROOT defined in the environment.
+//
+// RUN: env -u SDKROOT %clang -target x86_64-apple-darwin -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XCRUN < %t.log %s
+//
+// CHECK-XCRUN: clang
+// CHECK-XCRUN: "-cc1"
+// CHECK-XCRUN: "-isysroot" "{{.*MacOSX[0-9\.]*\.sdk}}"
+
+// Check once again that we default to running `xcrun`, this time with another target.
+//
+// RUN: env -u SDKROOT %clang -target arm64-apple-ios -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XCRUN-IOS < %t.log %s
+//
+// CHECK-XCRUN-IOS: clang
+// CHECK-XCRUN-IOS: "-cc1"
+// CHECK-XCRUN-IOS: "-isysroot" "{{.*iPhoneOS[0-9\.]*\.sdk}}"
+
 // Check if clang set the correct deployment target from -sysroot
 //
 // RUN: rm -rf %t/SDKs/iPhoneOS8.0.0.sdk
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -18,15 +18,22 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include  // ::getenv
+#include   // std::unique_ptr
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -2074,21 +2081,77 @@
 void Darwin::AddDeploymentTarget(DerivedArgList ) const {
   const OptTable  = getDriver().getOpts();
 
-  // 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.
+  // On Apple platforms, C and C++ Standard Library headers are not provided
+  // with the base system. Instead, they are provided in various SDKs for the
+  // different Apple platforms. Clang needs to know where that SDK lives, and
+  // there are a couple ways this can be achieved:
+  //
+  // (1) If `-isysroot ` is passed explicitly, use that.
   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));
+  }
+
+  // (2) If the SDKROOT environment variable is defined and points to a valid
+  // path, use that. $SDKROOT is set by `xcrun` and other Xcode tools, so
+  // running `xcrun clang` will work by going through this path.
+  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));
+}
+  }
+
+  // (3) Otherwise, we try to guess the path of the default SDK by running
+  // `xcrun --show-sdk-path`. This won't always be correct, but if the
+  //  user wants to use the non-default SDK, they should specify it
+  //  explicitly with methods (1) or (2) above.
+  else {
+llvm::SmallString<64> 

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added a comment.

In D136315#3870829 , @ldionne wrote:

> Thanks for picking this up! I looked at my local changes and I had started 
> modifying `inferDeploymentTargetFromSDK`. I had left the comment:
>
>   /// TODO: We should only infer it if the SDK was provided with SDKROOT or 
> -isysroot.
>   ///   If we inferred the SDK with xcrun, it doesn't make sense to infer 
> the
>   ///   deployment target because 
>
> I don't remember what I was thinking at the time, but I encourage you to 
> pause for a minute and think about it. I can't think of the reason anymore. 
> If you cant' either, maybe just disregard my comment entirely.

I took another look at this, and I think it makes sense to determine the 
deployment target regardless.  The target platform shouldn't change since the 
platform is passed to `xcrun`, but there is still value in determining the 
default deployment version.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski updated this revision to Diff 469859.
calebzulawski added a comment.

Changed to not invoke `xcrun` if the SDK name can't be determined.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-sdkroot.c

Index: clang/test/Driver/darwin-sdkroot.c
===
--- clang/test/Driver/darwin-sdkroot.c
+++ clang/test/Driver/darwin-sdkroot.c
@@ -1,4 +1,5 @@
-// Check that SDKROOT is used to define the default for -isysroot on Darwin.
+// Check that SDKROOT is used to define the default for -isysroot on Darwin,
+// or that we call `xcrun --show-sdk-path` if SDKROOT is not in the environment.
 //
 // RUN: rm -rf %t.tmpdir
 // RUN: mkdir -p %t.tmpdir
@@ -39,6 +40,25 @@
 //
 // This test do pass using GnuWin32 env.exe.
 
+// Check that we default to running `xcrun --show-sdk-path` if there is no
+// SDKROOT defined in the environment.
+//
+// RUN: env -i %clang -target x86_64-apple-darwin -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XCRUN < %t.log %s
+//
+// CHECK-XCRUN: clang
+// CHECK-XCRUN: "-cc1"
+// CHECK-XCRUN: "-isysroot" "{{.*MacOSX[0-9\.]*\.sdk}}"
+
+// Check once again that we default to running `xcrun`, this time with another target.
+//
+// RUN: env -i %clang -target arm64-apple-ios -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XCRUN-IOS < %t.log %s
+//
+// CHECK-XCRUN-IOS: clang
+// CHECK-XCRUN-IOS: "-cc1"
+// CHECK-XCRUN-IOS: "-isysroot" "{{.*iPhoneOS[0-9\.]*\.sdk}}"
+
 // Check if clang set the correct deployment target from -sysroot
 //
 // RUN: rm -rf %t/SDKs/iPhoneOS8.0.0.sdk
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -18,15 +18,22 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include  // ::getenv
+#include   // std::unique_ptr
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -2074,21 +2081,77 @@
 void Darwin::AddDeploymentTarget(DerivedArgList ) const {
   const OptTable  = getDriver().getOpts();
 
-  // 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.
+  // On Apple platforms, C and C++ Standard Library headers are not provided
+  // with the base system. Instead, they are provided in various SDKs for the
+  // different Apple platforms. Clang needs to know where that SDK lives, and
+  // there are a couple ways this can be achieved:
+  //
+  // (1) If `-isysroot ` is passed explicitly, use that.
   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));
+  }
+
+  // (2) If the SDKROOT environment variable is defined and points to a valid
+  // path, use that. $SDKROOT is set by `xcrun` and other Xcode tools, so
+  // running `xcrun clang` will work by going through this path.
+  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));
+}
+  }
+
+  // (3) Otherwise, we try to guess the path of the default SDK by running
+  // `xcrun --show-sdk-path`. This won't always be correct, but if the
+  //  user wants to use the non-default SDK, they should specify it
+  //  explicitly with methods (1) or (2) above.
+  else {
+llvm::SmallString<64> OutputFile;
+

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added a comment.

If `clang -target arm64-apple-ios -isysroot path/to/MacOSX.sdk` changes the 
target to `x86_64-apple-macos` to match the SDK, that would be very confusing 
behavior :) I'll have to investigate that function some more.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added a comment.

That's a good point, I was a little suspicious about that function.

I'm not sure it makes sense to infer the target at all from the SDK (e.g. macOS 
could be x86_64 or arm64) rather than infer the SDK from the target.  Rust, for 
example, disregards `SYSROOT` and runs `xcrun` if it doesn't match the target 
triple.  I wonder if that behavior is cemented enough that it shouldn't change, 
though.  At a minimum, you're right that we should skip inferring the target in 
the case where the sysroot isn't specified at all.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: Diff 2

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski created this revision.
Herald added a project: All.
calebzulawski requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136315

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-sdkroot.c

Index: clang/test/Driver/darwin-sdkroot.c
===
--- clang/test/Driver/darwin-sdkroot.c
+++ clang/test/Driver/darwin-sdkroot.c
@@ -1,4 +1,5 @@
-// Check that SDKROOT is used to define the default for -isysroot on Darwin.
+// Check that SDKROOT is used to define the default for -isysroot on Darwin,
+// or that we call `xcrun --show-sdk-path` if SDKROOT is not in the environment.
 //
 // RUN: rm -rf %t.tmpdir
 // RUN: mkdir -p %t.tmpdir
@@ -39,6 +40,16 @@
 //
 // This test do pass using GnuWin32 env.exe.
 
+// Check that we default to running `xcrun --show-sdk-path` if there is no
+// SDKROOT defined in the environment.
+//
+// RUN: env -i %clang -target arm64-apple-ios -c %s -### 2> %t.log
+// RUN: FileCheck --check-prefix=CHECK-XCRUN < %t.log %s
+//
+// CHECK-XCRUN: clang
+// CHECK-XCRUN: "-cc1"
+// CHECK-XCRUN: "-isysroot" "{{.*iPhoneOS[0-9\.]*\.sdk}}"
+
 // Check if clang set the correct deployment target from -sysroot
 //
 // RUN: rm -rf %t/SDKs/iPhoneOS8.0.0.sdk
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -18,15 +18,22 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include  // ::getenv
+#include  // std::unique_ptr
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -2074,21 +2081,75 @@
 void Darwin::AddDeploymentTarget(DerivedArgList ) const {
   const OptTable  = getDriver().getOpts();
 
-  // 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.
+  // On Apple platforms, C and C++ Standard Library headers are not provided
+  // with the base system. Instead, they are provided in various SDKs for the
+  // different Apple platforms. Clang needs to know where that SDK lives, and
+  // there are a couple ways this can be achieved:
+  //
+  // (1) If `-isysroot ` is passed explicitly, use that.
   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));
+  }
+
+  // (2) If the SDKROOT environment variable is defined and points to a valid
+  // path, use that. $SDKROOT is set by `xcrun` and other Xcode tools, so
+  // running `xcrun clang` will work by going through this path.
+  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));
+}
+  }
+
+  // (3) Otherwise, we try to guess the path of the default SDK by running
+  // `xcrun --show-sdk-path`. This won't always be correct, but if the
+  //  user wants to use the non-default SDK, they should specify it
+  //  explicitly with methods (1) or (2) above.
+  else {
+llvm::SmallString<64> OutputFile;
+llvm::sys::fs::createTemporaryFile("print-sdk-path", "" /* No Suffix */, OutputFile);
+llvm::FileRemover OutputRemover(OutputFile.c_str());
+llvm::Optional Redirects[] = {{""}, OutputFile.str(), {""}};
+
+StringRef sdk;
+if (getTriple().isWatchOS()) {
+  if (getTriple().isSimulatorEnvironment())
+sdk = "watchsimulator";
+  else
+sdk = 

[PATCH] D109460: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added a comment.
Herald added a subscriber: MaskRay.
Herald added a project: All.

I'm interested in this feature--is there any reason this wasn't completed? If 
not, I might be able to take over implementing this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109460/new/

https://reviews.llvm.org/D109460

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits