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

Caleb Zulawski via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Oct 22 09:17:05 PDT 2022


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



More information about the cfe-commits mailing list