[clang] [clang] Match -isysroot behaviour with system compiler on Darwin (PR #80524)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 2 17:46:37 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-driver
@llvm/pr-subscribers-clang
Author: Daniel RodrÃguez Troitiño (drodriguez)
<details>
<summary>Changes</summary>
The current Apple Clang behaviour is to prefer `-isysroot` vs libc++ headers side-by-side the compiler. This has been like that for several Xcode versions, at least since Xcode 14.
The code was originally written in D89001 chosing the order that was correct at the time for Apple Clang. In 2023 D157283 tried to flip the order to match the current Apple Clang, but was reverted in 3197357b7e39a58bc7eb0600eb337ac2a1c8c225 because it brokes two tests. The code was further changed in #<!-- -->70817 to add a third option.
The changes in this commit try to match Apple Clang, and incorporate the changes in #<!-- -->70817, as well as fixing the tests that broke when D157283 was applied.
---
Full diff: https://github.com/llvm/llvm-project/pull/80524.diff
4 Files Affected:
- (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+16-16)
- (modified) clang/test/Driver/darwin-header-search-libcxx.cpp (+21-20)
- (modified) clang/test/Tooling/clang-check-mac-libcxx-abspath.cpp (+2-1)
- (modified) clang/test/Tooling/clang-check-mac-libcxx-relpath.cpp (+2-1)
``````````diff
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index fae8ad1a958ad..004a19284ee4a 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2515,20 +2515,31 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs(
switch (GetCXXStdlibType(DriverArgs)) {
case ToolChain::CST_Libcxx: {
// On Darwin, libc++ can be installed in one of the following places:
- // 1. Alongside the compiler in <install>/include/c++/v1
- // 2. Alongside the compiler in <clang-executable-folder>/../include/c++/v1
- // 3. In a SDK (or a custom sysroot) in <sysroot>/usr/include/c++/v1
+ // 1. In a SDK (or a custom sysroot) in <sysroot>/usr/include/c++/v1
+ // 2. Alongside the compiler in <install>/include/c++/v1
+ // 3. Alongside the compiler in <clang-executable-folder>/../include/c++/v1
//
// The precedence of paths is as listed above, i.e. we take the first path
// that exists. Note that we never include libc++ twice -- we take the first
// path that exists and don't send the other paths to CC1 (otherwise
// include_next could break).
//
- // Also note that in most cases, (1) and (2) are exactly the same path.
+ // Also note that in most cases, (2) and (3) are exactly the same path.
// Those two paths will differ only when the `clang` program being run
// is actually a symlink to the real executable.
// Check for (1)
+ llvm::SmallString<128> SysrootUsr = Sysroot;
+ llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1");
+ if (getVFS().exists(SysrootUsr)) {
+ addSystemInclude(DriverArgs, CC1Args, SysrootUsr);
+ return;
+ } else if (DriverArgs.hasArg(options::OPT_v)) {
+ llvm::errs() << "ignoring nonexistent directory \"" << SysrootUsr
+ << "\"\n";
+ }
+
+ // Check for (2)
// Get from '<install>/bin' to '<install>/include/c++/v1'.
// Note that InstallBin can be relative, so we use '..' instead of
// parent_path.
@@ -2543,7 +2554,7 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs(
<< "\"\n";
}
- // (2) Check for the folder where the executable is located, if different.
+ // (3) Check for the folder where the executable is located, if different.
if (getDriver().getInstalledDir() != getDriver().Dir) {
InstallBin = llvm::StringRef(getDriver().Dir);
llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
@@ -2556,17 +2567,6 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs(
}
}
- // Otherwise, check for (3)
- llvm::SmallString<128> SysrootUsr = Sysroot;
- llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1");
- if (getVFS().exists(SysrootUsr)) {
- addSystemInclude(DriverArgs, CC1Args, SysrootUsr);
- return;
- } else if (DriverArgs.hasArg(options::OPT_v)) {
- llvm::errs() << "ignoring nonexistent directory \"" << SysrootUsr
- << "\"\n";
- }
-
// Otherwise, don't add any path.
break;
}
diff --git a/clang/test/Driver/darwin-header-search-libcxx.cpp b/clang/test/Driver/darwin-header-search-libcxx.cpp
index 70cc06090a993..82f2616a87dd1 100644
--- a/clang/test/Driver/darwin-header-search-libcxx.cpp
+++ b/clang/test/Driver/darwin-header-search-libcxx.cpp
@@ -23,8 +23,8 @@
// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
// RUN: --check-prefix=CHECK-LIBCXX-TOOLCHAIN-1 %s
// CHECK-LIBCXX-TOOLCHAIN-1: "-cc1"
-// CHECK-LIBCXX-TOOLCHAIN-1: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
// CHECK-LIBCXX-TOOLCHAIN-1-NOT: "-internal-isystem" "/usr/include/c++/v1"
+// CHECK-LIBCXX-TOOLCHAIN-1: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
//
// RUN: %clang -### %s -fsyntax-only 2>&1 \
// RUN: --target=x86_64-apple-darwin \
@@ -35,8 +35,8 @@
// RUN: -DSYSROOT=%S/Inputs/basic_darwin_sdk_no_libcxx \
// RUN: --check-prefix=CHECK-LIBCXX-TOOLCHAIN-2 %s
// CHECK-LIBCXX-TOOLCHAIN-2: "-cc1"
-// CHECK-LIBCXX-TOOLCHAIN-2: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
// CHECK-LIBCXX-TOOLCHAIN-2-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
+// CHECK-LIBCXX-TOOLCHAIN-2: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
// Check with only headers in the sysroot (those should be used).
//
@@ -49,11 +49,11 @@
// RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
// RUN: --check-prefix=CHECK-LIBCXX-SYSROOT-1 %s
// CHECK-LIBCXX-SYSROOT-1: "-cc1"
-// CHECK-LIBCXX-SYSROOT-1: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
// CHECK-LIBCXX-SYSROOT-1-NOT: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
+// CHECK-LIBCXX-SYSROOT-1: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
// Check with both headers in the sysroot and headers alongside the installation
-// (the headers in the toolchain should be preferred over the <sysroot> headers).
+// (the headers in the <sysroot> headers should be preferred over the toolchain).
// Ensure that both -isysroot and --sysroot work, and that isysroot has precedence
// over --sysroot.
//
@@ -89,8 +89,8 @@
// RUN: --check-prefix=CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1 %s
//
// CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1: "-cc1"
-// CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
-// CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
+// CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1-NOT: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
+// CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
// Make sure that using -nostdinc, -nostdinc++ or -nostdlib will drop both the toolchain
// C++ include path and the sysroot one.
@@ -157,8 +157,8 @@
// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_no_libcxx \
// RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
// RUN: --check-prefix=CHECK-LIBCXX-MISSING-BOTH %s
-// CHECK-LIBCXX-MISSING-BOTH: ignoring nonexistent directory "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
// CHECK-LIBCXX-MISSING-BOTH: ignoring nonexistent directory "[[SYSROOT]]/usr/include/c++/v1"
+// CHECK-LIBCXX-MISSING-BOTH: ignoring nonexistent directory "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
// Make sure that on Darwin, we use libc++ header search paths by default even when
// -stdlib= isn't passed.
@@ -175,9 +175,9 @@
// ----------------------------------------------------------------------------
// On Darwin, libc++ can be installed in one of the following places:
-// 1. Alongside the compiler in <install>/include/c++/v1
-// 2. Alongside the compiler in <clang-executable-folder>/../include/c++/v1
-// 3. In a SDK (or a custom sysroot) in <sysroot>/usr/include/c++/v1
+// 1. In a SDK (or a custom sysroot) in <sysroot>/usr/include/c++/v1
+// 2. Alongside the compiler in <install>/include/c++/v1
+// 3. Alongside the compiler in <clang-executable-folder>/../include/c++/v1
// The build folders do not have an `include/c++/v1`; create a new
// local folder hierarchy that meets this requirement.
@@ -187,7 +187,7 @@
// RUN: cp %clang %t/install/bin/clang
// RUN: mkdir -p %t/install/include/c++/v1
-// Headers in (1) and in (2) -> (1) is preferred over (2)
+// Headers in (2) and in (3) -> (2) is preferred over (3)
// RUN: rm -rf %t/symlinked1
// RUN: mkdir -p %t/symlinked1/bin
// RUN: ln -sf %t/install/bin/clang %t/symlinked1/bin/clang
@@ -196,16 +196,16 @@
// RUN: %t/symlinked1/bin/clang -### %s -fsyntax-only 2>&1 \
// RUN: --target=x86_64-apple-darwin \
// RUN: -stdlib=libc++ \
-// RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
+// RUN: -isysroot %S/Inputs/basic_darwin_sdk_no_libcxx \
// RUN: | FileCheck -DSYMLINKED=%t/symlinked1 \
// RUN: -DTOOLCHAIN=%t/install \
-// RUN: -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
+// RUN: -DSYSROOT=%S/Inputs/basic_darwin_sdk_no_libcxx \
// RUN: --check-prefix=CHECK-SYMLINKED-INCLUDE-CXX-V1 %s
-// CHECK-SYMLINKED-INCLUDE-CXX-V1: "-internal-isystem" "[[SYMLINKED]]/bin/../include/c++/v1"
// CHECK-SYMLINKED-INCLUDE-CXX-V1-NOT: "-internal-isystem" "[[TOOLCHAIN]]/bin/../include/c++/v1"
// CHECK-SYMLINKED-INCLUDE-CXX-V1-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
+// CHECK-SYMLINKED-INCLUDE-CXX-V1: "-internal-isystem" "[[SYMLINKED]]/bin/../include/c++/v1"
-// Headers in (2) and in (3) -> (2) is preferred over (3)
+// Headers in (1) and in (3) -> (1) is preferred over (3)
// RUN: rm -rf %t/symlinked2
// RUN: mkdir -p %t/symlinked2/bin
// RUN: ln -sf %t/install/bin/clang %t/symlinked2/bin/clang
@@ -216,16 +216,17 @@
// RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
// RUN: | FileCheck -DTOOLCHAIN=%t/install \
// RUN: -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
-// RUN: --check-prefix=CHECK-TOOLCHAIN-INCLUDE-CXX-V1 %s
-// CHECK-TOOLCHAIN-INCLUDE-CXX-V1: "-internal-isystem" "[[TOOLCHAIN]]/bin/../include/c++/v1"
-// CHECK-TOOLCHAIN-INCLUDE-CXX-V1-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
+// RUN: --check-prefix=CHECK-SYSROOT-INCLUDE-CXX-V1 %s
+// CHECK-SYSROOT-INCLUDE-CXX-V1-NOT: "-internal-isystem" "[[TOOLCHAIN]]/bin/../include/c++/v1"
+// CHECK-SYSROOT-INCLUDE-CXX-V1: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
-// Headers in (2) and nowhere else -> (2) is used
+// Headers in (3) and nowhere else -> (3) is used
// RUN: %t/symlinked2/bin/clang -### %s -fsyntax-only 2>&1 \
// RUN: --target=x86_64-apple-darwin \
// RUN: -stdlib=libc++ \
-// RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
+// RUN: -isysroot %S/Inputs/basic_darwin_sdk_no_libcxx \
// RUN: | FileCheck -DTOOLCHAIN=%t/install \
// RUN: -DSYSROOT=%S/Inputs/basic_darwin_sdk_no_libcxx \
// RUN: --check-prefix=CHECK-TOOLCHAIN-NO-SYSROOT %s
+// CHECK-TOOLCHAIN-NO-SYSROOT-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
// CHECK-TOOLCHAIN-NO-SYSROOT: "-internal-isystem" "[[TOOLCHAIN]]/bin/../include/c++/v1"
diff --git a/clang/test/Tooling/clang-check-mac-libcxx-abspath.cpp b/clang/test/Tooling/clang-check-mac-libcxx-abspath.cpp
index 476ba3ce0849a..e2152e82e621c 100644
--- a/clang/test/Tooling/clang-check-mac-libcxx-abspath.cpp
+++ b/clang/test/Tooling/clang-check-mac-libcxx-abspath.cpp
@@ -3,12 +3,13 @@
//
// RUN: rm -rf %t
// RUN: mkdir %t
+// RUN: mkdir %t/fake-sysroot
//
// Install the mock libc++ (simulates the libc++ directory structure).
// RUN: cp -r %S/Inputs/mock-libcxx %t/
//
// Pretend clang is installed beside the mock library that we provided.
-// RUN: echo '[{"directory":"%t","command":"%t/mock-libcxx/bin/clang++ -stdlib=libc++ -target x86_64-apple-darwin -c test.cpp","file":"test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
+// RUN: echo '[{"directory":"%t","command":"%t/mock-libcxx/bin/clang++ -stdlib=libc++ -target x86_64-apple-darwin -isysroot %t/fake-sysroot -c test.cpp","file":"test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
// RUN: cp "%s" "%t/test.cpp"
// clang-check will produce an error code if the mock library is not found.
// RUN: clang-check -p "%t" "%t/test.cpp"
diff --git a/clang/test/Tooling/clang-check-mac-libcxx-relpath.cpp b/clang/test/Tooling/clang-check-mac-libcxx-relpath.cpp
index 099be5ecd4984..37d9f9d89f152 100644
--- a/clang/test/Tooling/clang-check-mac-libcxx-relpath.cpp
+++ b/clang/test/Tooling/clang-check-mac-libcxx-relpath.cpp
@@ -3,12 +3,13 @@
//
// RUN: rm -rf %t
// RUN: mkdir %t
+// RUN: mkdir -p %t/fake-sysroot
//
// Install the mock libc++ (simulates the libc++ directory structure).
// RUN: cp -r %S/Inputs/mock-libcxx %t/
//
// Pretend clang is installed beside the mock library that we provided.
-// RUN: echo '[{"directory":"%t","command":"mock-libcxx/bin/clang++ -stdlib=libc++ -target x86_64-apple-darwin -c test.cpp","file":"test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
+// RUN: echo '[{"directory":"%t","command":"mock-libcxx/bin/clang++ -stdlib=libc++ -target x86_64-apple-darwin -isysroot %t/fake-sysroot -c test.cpp","file":"test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
// RUN: cp "%s" "%t/test.cpp"
// clang-check will produce an error code if the mock library is not found.
// RUN: clang-check -p "%t" "%t/test.cpp"
``````````
</details>
https://github.com/llvm/llvm-project/pull/80524
More information about the cfe-commits
mailing list