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