[clang] [clang] Match -isysroot behaviour with system compiler on Darwin (PR #80524)
Daniel RodrÃguez Troitiño via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 2 17:46:09 PST 2024
https://github.com/drodriguez created https://github.com/llvm/llvm-project/pull/80524
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.
>From 32310bb312432bc215bec823c26f6194b0dcd447 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Rodri=CC=81guez=20Troitin=CC=83o?=
<danielrodriguez at meta.com>
Date: Fri, 2 Feb 2024 17:26:39 -0800
Subject: [PATCH] [clang] Match -isysroot behaviour with system compiler on
Darwin
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.
---
clang/lib/Driver/ToolChains/Darwin.cpp | 32 +++++++--------
.../Driver/darwin-header-search-libcxx.cpp | 41 ++++++++++---------
.../clang-check-mac-libcxx-abspath.cpp | 3 +-
.../clang-check-mac-libcxx-relpath.cpp | 3 +-
4 files changed, 41 insertions(+), 38 deletions(-)
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"
More information about the cfe-commits
mailing list