[Clang] [Driver] Canoncalise -internal-isystem include paths
#148745
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sirraide
added
the
clang:driver
llvmbot
added
clang
|
@llvm/pr-subscribers-backend-risc-v @llvm/pr-subscribers-clang-driver Author: None (Sirraide) ChangesThis is another attempt at implementing #143520, but with a different strategy this time: Instead of simplifying paths that contain The motivation for this (as well as the original patch) is that it can significantly shorten file paths to standard library headers, e.g. on my system, /usr/lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/rangesbut with this change, we instead print /usr/include/c++/15/rangesAdditionally, since there was a request to allow disabling this functionality, passing Patch is 119.04 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/148745.diff 35 Files Affected:
diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h
index b8899e78176b4..2217c0728665e 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -228,9 +228,13 @@ class ToolChain {
static void addSystemFrameworkInclude(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args,
const Twine &Path);
+
+public:
static void addSystemInclude(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args,
const Twine &Path);
+
+protected:
static void addExternCSystemInclude(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args,
const Twine &Path);
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 3f9b808b2722e..cc018831e20fa 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1370,19 +1370,47 @@ ToolChain::CXXStdlibType ToolChain::GetCXXStdlibType(const ArgList &Args) const{
return *cxxStdlibType;
}
+static void ResolveAndAddSystemIncludePath(const ArgList &DriverArgs,
+ ArgStringList &CC1Args,
+ const Twine &Path) {
+ bool Canonicalize =
+ DriverArgs.hasFlag(options::OPT_canonical_prefixes,
+ options::OPT_no_canonical_prefixes, true);
+
+ if (!Canonicalize) {
+ CC1Args.push_back(DriverArgs.MakeArgString(Path));
+ return;
+ }
+
+ // We canonicalise system include paths that were added automatically if
+ // that yields a shorter path since those can end up quite long otherwise.
+ //
+ // While we would ideally prefer to use FileManager for this, there doesn't
+ // seem to be a way to obtain one in here, so we just resolve these via the
+ // real file system; most system libraries will hopefully correspond to
+ // actual files.
+ IntrusiveRefCntPtr<vfs::FileSystem> VFS = vfs::getRealFileSystem();
+ SmallString<256> Canonical, PathStorage;
+ StringRef SimplifiedPath = Path.toStringRef(PathStorage);
+ if (!VFS->getRealPath(SimplifiedPath, Canonical) &&
+ Canonical.size() < SimplifiedPath.size())
+ SimplifiedPath = Canonical;
+ CC1Args.push_back(DriverArgs.MakeArgString(SimplifiedPath));
+}
+
/// Utility function to add a system framework directory to CC1 arguments.
void ToolChain::addSystemFrameworkInclude(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args,
const Twine &Path) {
CC1Args.push_back("-internal-iframework");
- CC1Args.push_back(DriverArgs.MakeArgString(Path));
+ ResolveAndAddSystemIncludePath(DriverArgs, CC1Args, Path);
}
/// Utility function to add a system include directory to CC1 arguments.
void ToolChain::addSystemInclude(const ArgList &DriverArgs,
ArgStringList &CC1Args, const Twine &Path) {
CC1Args.push_back("-internal-isystem");
- CC1Args.push_back(DriverArgs.MakeArgString(Path));
+ ResolveAndAddSystemIncludePath(DriverArgs, CC1Args, Path);
}
/// Utility function to add a system include directory with extern "C"
@@ -1397,7 +1425,7 @@ void ToolChain::addExternCSystemInclude(const ArgList &DriverArgs,
ArgStringList &CC1Args,
const Twine &Path) {
CC1Args.push_back("-internal-externc-isystem");
- CC1Args.push_back(DriverArgs.MakeArgString(Path));
+ ResolveAndAddSystemIncludePath(DriverArgs, CC1Args, Path);
}
void ToolChain::addExternCSystemIncludeIfExists(const ArgList &DriverArgs,
@@ -1411,20 +1439,16 @@ void ToolChain::addExternCSystemIncludeIfExists(const ArgList &DriverArgs,
void ToolChain::addSystemFrameworkIncludes(const ArgList &DriverArgs,
ArgStringList &CC1Args,
ArrayRef<StringRef> Paths) {
- for (const auto &Path : Paths) {
- CC1Args.push_back("-internal-iframework");
- CC1Args.push_back(DriverArgs.MakeArgString(Path));
- }
+ for (const auto &Path : Paths)
+ addSystemFrameworkInclude(DriverArgs, CC1Args, Path);
}
/// Utility function to add a list of system include directories to CC1.
void ToolChain::addSystemIncludes(const ArgList &DriverArgs,
ArgStringList &CC1Args,
ArrayRef<StringRef> Paths) {
- for (const auto &Path : Paths) {
- CC1Args.push_back("-internal-isystem");
- CC1Args.push_back(DriverArgs.MakeArgString(Path));
- }
+ for (const auto &Path : Paths)
+ addSystemInclude(DriverArgs, CC1Args, Path);
}
std::string ToolChain::concat(StringRef Path, const Twine &A, const Twine &B,
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index fe1865888bdd0..6931b8a8f861a 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -946,8 +946,7 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
SmallString<128> P(D.ResourceDir);
llvm::sys::path::append(P, "include");
llvm::sys::path::append(P, "openmp_wrappers");
- CmdArgs.push_back("-internal-isystem");
- CmdArgs.push_back(Args.MakeArgString(P));
+ getToolChain().addSystemInclude(Args, CmdArgs, P);
}
CmdArgs.push_back("-include");
@@ -959,7 +958,8 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
// standard library headers and other headers.
SmallString<128> P(D.ResourceDir);
llvm::sys::path::append(P, "include", "llvm_offload_wrappers");
- CmdArgs.append({"-internal-isystem", Args.MakeArgString(P), "-include"});
+ getToolChain().addSystemInclude(Args, CmdArgs, P);
+ CmdArgs.push_back("-include");
if (JA.isDeviceOffloading(Action::OFK_OpenMP))
CmdArgs.push_back("__llvm_offload_device.h");
else
@@ -1132,16 +1132,14 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
SmallString<128> P(llvm::sys::path::parent_path(D.Dir));
llvm::sys::path::append(P, "include");
llvm::sys::path::append(P, getToolChain().getTripleString());
- CmdArgs.push_back("-internal-isystem");
- CmdArgs.push_back(Args.MakeArgString(P));
+ getToolChain().addSystemInclude(Args, CmdArgs, P);
} else if (C.getActiveOffloadKinds() == Action::OFK_OpenMP) {
// TODO: CUDA / HIP include their own headers for some common functions
// implemented here. We'll need to clean those up so they do not conflict.
SmallString<128> P(D.ResourceDir);
llvm::sys::path::append(P, "include");
llvm::sys::path::append(P, "llvm_libc_wrappers");
- CmdArgs.push_back("-internal-isystem");
- CmdArgs.push_back(Args.MakeArgString(P));
+ getToolChain().addSystemInclude(Args, CmdArgs, P);
}
}
diff --git a/clang/test/Driver/aarch64-toolchain.c b/clang/test/Driver/aarch64-toolchain.c
index cfad4b8eb6829..612e064ed1ef5 100644
--- a/clang/test/Driver/aarch64-toolchain.c
+++ b/clang/test/Driver/aarch64-toolchain.c
@@ -2,6 +2,7 @@
// Test interaction with -fuse-ld=lld
// RUN: %clang -### %s -fuse-ld=lld -B%S/Inputs/lld \
+// RUN: -no-canonical-prefixes \
// RUN: --target=aarch64-none-elf --rtlib=libgcc --unwindlib=platform \
// RUN: --gcc-toolchain=%S/Inputs/basic_aarch64_gcc_tree \
// RUN: --sysroot=%S/Inputs/basic_aarch64_gcc_tree/aarch64-none-elf 2>&1 \
@@ -20,6 +21,7 @@
// LLD-AARCH64-BAREMETAL: "{{.*}}/Inputs/basic_aarch64_gcc_tree/lib/gcc/aarch64-none-elf/8.2.1/crtend.o"
// RUN: %clang -### %s -fuse-ld= \
+// RUN: -no-canonical-prefixes \
// RUN: --target=aarch64-none-elf --rtlib=libgcc --unwindlib=platform \
// RUN: --gcc-toolchain=%S/Inputs/basic_aarch64_gcc_tree \
// RUN: --sysroot=%S/Inputs/basic_aarch64_gcc_tree/aarch64-none-elf 2>&1 \
@@ -39,6 +41,7 @@
// C-AARCH64-BAREMETAL: "{{.*}}/Inputs/basic_aarch64_gcc_tree/lib/gcc/aarch64-none-elf/8.2.1/crtend.o"
// RUN: %clang -### %s -fuse-ld= \
+// RUN: -no-canonical-prefixes \
// RUN: --target=aarch64-none-elf --rtlib=libgcc --unwindlib=platform \
// RUN: --gcc-toolchain=%S/Inputs/basic_aarch64_gcc_tree \
// RUN: --sysroot= 2>&1 \
@@ -56,6 +59,7 @@
// C-AARCH64-BAREMETAL-NOSYSROOT: "{{.*}}/Inputs/basic_aarch64_gcc_tree/lib/gcc/aarch64-none-elf/8.2.1/crtend.o"
// RUN: %clangxx -### %s -fuse-ld= \
+// RUN: -no-canonical-prefixes \
// RUN: --target=aarch64-none-elf -stdlib=libstdc++ --rtlib=libgcc --unwindlib=platform \
// RUN: --gcc-toolchain=%S/Inputs/basic_aarch64_gcc_tree \
// RUN: --sysroot=%S/Inputs/basic_aarch64_gcc_tree/aarch64-none-elf 2>&1 \
@@ -76,6 +80,7 @@
// CXX-AARCH64-BAREMETAL: "{{.*}}/Inputs/basic_aarch64_gcc_tree/lib/gcc/aarch64-none-elf/8.2.1/crtend.o"
// RUN: %clangxx -### %s -fuse-ld= \
+// RUN: -no-canonical-prefixes \
// RUN: --target=aarch64-none-elf -stdlib=libstdc++ --rtlib=libgcc --unwindlib=platform \
// RUN: --gcc-toolchain=%S/Inputs/basic_aarch64_gcc_tree \
// RUN: --sysroot= 2>&1 \
@@ -95,6 +100,7 @@
// CXX-AARCH64-BAREMETAL-NOSYSROOT: "{{.*}}/Inputs/basic_aarch64_gcc_tree/lib/gcc/aarch64-none-elf/8.2.1/crtend.o"
// RUN: %clangxx -### %s -fuse-ld= \
+// RUN: -no-canonical-prefixes \
// RUN: --target=aarch64-none-elf -stdlib=libc++ --rtlib=libgcc --unwindlib=platform \
// RUN: --gcc-toolchain=%S/Inputs/basic_aarch64_gcc_tree \
// RUN: --sysroot=%S/Inputs/basic_aarch64_gcc_tree/aarch64-none-elf 2>&1 \
@@ -114,6 +120,7 @@
// CXX-AARCH64-BAREMETAL-LIBCXX: "{{.*}}/Inputs/basic_aarch64_gcc_tree/lib/gcc/aarch64-none-elf/8.2.1/crtend.o"
// RUN: %clangxx -### %s -fuse-ld= \
+// RUN: -no-canonical-prefixes \
// RUN: --target=aarch64-none-elf -stdlib=libc++ --rtlib=libgcc --unwindlib=platform \
// RUN: --gcc-toolchain=%S/Inputs/basic_aarch64_gcc_tree \
// RUN: --sysroot= 2>&1 \
diff --git a/clang/test/Driver/android-installed-libcxx.cpp b/clang/test/Driver/android-installed-libcxx.cpp
index 14856e26e2730..620ec48867a49 100644
--- a/clang/test/Driver/android-installed-libcxx.cpp
+++ b/clang/test/Driver/android-installed-libcxx.cpp
@@ -16,10 +16,12 @@
// RUN: mkdir -p %t2/include/aarch64-none-linux-android23/c++/v1
// RUN: %clang -target aarch64-none-linux-android -ccc-install-dir %/t2/bin \
+// RUN: -no-canonical-prefixes \
// RUN: --sysroot=%t2/sysroot -stdlib=libc++ -fsyntax-only \
// RUN: %s -### 2>&1 | FileCheck --check-prefix=ANDROID-DIR -DDIR=%/t2/bin %s
// RUN: %clang -target aarch64-none-linux-android21 -ccc-install-dir %/t2/bin \
+// RUN: -no-canonical-prefixes \
// RUN: --sysroot=%t2/sysroot -stdlib=libc++ -fsyntax-only \
// RUN: %s -### 2>&1 | FileCheck --check-prefix=ANDROID-DIR -DDIR=%/t2/bin %s
@@ -27,10 +29,12 @@
// ANDROID-DIR-SAME: "-internal-isystem" "[[DIR]][[SEP]]..[[SEP]]include[[SEP]]c++[[SEP]]v1"
// RUN: %clang -target aarch64-none-linux-android23 -ccc-install-dir %/t2/bin \
+// RUN: -no-canonical-prefixes \
// RUN: --sysroot=%t2/sysroot -stdlib=libc++ -fsyntax-only \
// RUN: %s -### 2>&1 | FileCheck --check-prefix=ANDROID23-DIR -DDIR=%/t2/bin %s
// RUN: %clang -target aarch64-none-linux-android28 -ccc-install-dir %/t2/bin \
+// RUN: -no-canonical-prefixes \
// RUN: --sysroot=%t2/sysroot -stdlib=libc++ -fsyntax-only \
// RUN: %s -### 2>&1 | FileCheck --check-prefix=ANDROID23-DIR -DDIR=%/t2/bin %s
diff --git a/clang/test/Driver/arm-toolchain.c b/clang/test/Driver/arm-toolchain.c
index c367594b0a758..e3b25f8ba6cd8 100644
--- a/clang/test/Driver/arm-toolchain.c
+++ b/clang/test/Driver/arm-toolchain.c
@@ -1,6 +1,7 @@
// UNSUPPORTED: system-windows
// RUN: %clang -### %s -fuse-ld=lld -B%S/Inputs/lld \
+// RUN: -no-canonical-prefixes \
// RUN: --target=armv6m-none-eabi --rtlib=libgcc --unwindlib=platform \
// RUN: --gcc-toolchain=%S/Inputs/basic_arm_gcc_tree \
// RUN: --sysroot=%S/Inputs/basic_arm_gcc_tree/armv6m-none-eabi 2>&1 \
@@ -19,6 +20,7 @@
// LLD-ARM-BAREMETAL: "{{.*}}/Inputs/basic_arm_gcc_tree/lib/gcc/armv6m-none-eabi/8.2.1/crtend.o"
// RUN: %clang -### %s -fuse-ld= \
+// RUN: -no-canonical-prefixes \
// RUN: --target=armv6m-none-eabi --rtlib=libgcc --unwindlib=platform \
// RUN: --gcc-toolchain=%S/Inputs/basic_arm_gcc_tree \
// RUN: --sysroot=%S/Inputs/basic_arm_gcc_tree/armv6m-none-eabi 2>&1 \
@@ -38,6 +40,7 @@
// C-ARM-BAREMETAL: "{{.*}}/Inputs/basic_arm_gcc_tree/lib/gcc/armv6m-none-eabi/8.2.1/crtend.o"
// RUN: %clang -### %s -fuse-ld= \
+// RUN: -no-canonical-prefixes \
// RUN: --target=armv6m-none-eabi --rtlib=libgcc --unwindlib=platform \
// RUN: --gcc-toolchain=%S/Inputs/basic_arm_gcc_tree \
// RUN: --sysroot= 2>&1 \
@@ -55,6 +58,7 @@
// C-ARM-BAREMETAL-NOSYSROOT: "{{.*}}/Inputs/basic_arm_gcc_tree/lib/gcc/armv6m-none-eabi/8.2.1/crtend.o"
// RUN: %clangxx -### %s -fuse-ld= \
+// RUN: -no-canonical-prefixes \
// RUN: --target=armv6m-none-eabi -stdlib=libstdc++ --rtlib=libgcc --unwindlib=platform \
// RUN: --gcc-toolchain=%S/Inputs/basic_arm_gcc_tree \
// RUN: --sysroot=%S/Inputs/basic_arm_gcc_tree/armv6m-none-eabi 2>&1 \
@@ -77,6 +81,7 @@
// RUN: %clangxx -### %s -fuse-ld= \
+// RUN: -no-canonical-prefixes \
// RUN: --target=armv6m-none-eabi -stdlib=libstdc++ --rtlib=libgcc --unwindlib=platform \
// RUN: --gcc-toolchain=%S/Inputs/basic_arm_gcc_tree \
// RUN: --sysroot= 2>&1 \
@@ -96,6 +101,7 @@
// CXX-ARM-BAREMETAL-NOSYSROOT: "{{.*}}/Inputs/basic_arm_gcc_tree/lib/gcc/armv6m-none-eabi/8.2.1/crtend.o"
// RUN: %clangxx -### %s -fuse-ld= \
+// RUN: -no-canonical-prefixes \
// RUN: --target=armv6m-none-eabi -stdlib=libc++ --rtlib=libgcc --unwindlib=platform \
// RUN: --gcc-toolchain=%S/Inputs/basic_arm_gcc_tree \
// RUN: --sysroot=%S/Inputs/basic_arm_gcc_tree/armv6m-none-eabi 2>&1 \
@@ -115,6 +121,7 @@
// CXX-ARM-BAREMETAL-LIBCXX: "{{.*}}/Inputs/basic_arm_gcc_tree/lib/gcc/armv6m-none-eabi/8.2.1/crtend.o"
// RUN: %clangxx -### %s -fuse-ld= \
+// RUN: -no-canonical-prefixes \
// RUN: --target=armv6m-none-eabi -stdlib=libc++ --rtlib=libgcc --unwindlib=platform \
// RUN: --gcc-toolchain=%S/Inputs/basic_arm_gcc_tree \
// RUN: --sysroot= 2>&1 \
diff --git a/clang/test/Driver/avr-toolchain.c b/clang/test/Driver/avr-toolchain.c
index 9d17476f30a69..60d4fc5285a51 100644
--- a/clang/test/Driver/avr-toolchain.c
+++ b/clang/test/Driver/avr-toolchain.c
@@ -1,7 +1,7 @@
// UNSUPPORTED: system-windows
// A basic clang -cc1 command-line.
-// RUN: %clang -### %s --target=avr --sysroot=%S/Inputs/basic_avr_tree -resource-dir=%S/Inputs/resource_dir 2>&1 | FileCheck --check-prefix=CHECK1 %s
+// RUN: %clang -### %s -no-canonical-prefixes --target=avr --sysroot=%S/Inputs/basic_avr_tree -resource-dir=%S/Inputs/resource_dir 2>&1 | FileCheck --check-prefix=CHECK1 %s
// CHECK1: "-cc1" "-triple" "avr"
// CHECK1-SAME: "-resource-dir" "[[RESOURCE:[^"]+]]"
// CHECK1-SAME: "-isysroot" "[[SYSROOT:[^"]+/basic_avr_tree]]"
@@ -12,13 +12,13 @@
// CHECK1-SAME: "-o" "a.out"
// CHECK1-SAME: {{^}} "--gc-sections"
-// RUN: %clang -### %s --target=avr --sysroot=%S/Inputs/basic_avr_tree_2/opt/local -S 2>&1 | FileCheck --check-prefix=CHECK2 %s
+// RUN: %clang -### %s -no-canonical-prefixes --target=avr --sysroot=%S/Inputs/basic_avr_tree_2/opt/local -S 2>&1 | FileCheck --check-prefix=CHECK2 %s
// CHECK2: "-cc1" "-triple" "avr"
// CHECK2-SAME: "-isysroot" "[[SYSROOT:[^"]+/basic_avr_tree_2/opt/local]]"
// CHECK2-SAME: "-internal-isystem"
// CHECK2-SAME: {{^}} "[[SYSROOT]]/lib/gcc/avr/10.3.0/../../../../avr/include"
-// RUN: %clang -### %s --target=avr --sysroot=%S/Inputs/basic_avr_tree_2 -S 2>&1 | FileCheck --check-prefix=CHECK3 %s
+// RUN: %clang -### %s -no-canonical-prefixes --target=avr --sysroot=%S/Inputs/basic_avr_tree_2 -S 2>&1 | FileCheck --check-prefix=CHECK3 %s
// CHECK3: "-cc1" "-triple" "avr"
// CHECK3-SAME: "-isysroot" "[[SYSROOT:[^"]+/basic_avr_tree_2]]"
// CHECK3-SAME: "-internal-isystem"
@@ -32,8 +32,8 @@
// CHECK4-NOT: "-fno-use-init-array"
// CHECK4-NOT: "-fno-use-cxa-atexit"
-// RUN: %clang -### %s --target=avr --sysroot=%S/Inputs/basic_avr_tree 2>&1 -nostdinc | FileCheck --check-prefix=NOSTDINC %s
-// RUN: %clang -### %s --target=avr --sysroot=%S/Inputs/basic_avr_tree 2>&1 -nostdlibinc | FileCheck --check-prefix=NOSTDINC %s
+// RUN: %clang -### %s -no-canonical-prefixes --target=avr --sysroot=%S/Inputs/basic_avr_tree 2>&1 -nostdinc | FileCheck --check-prefix=NOSTDINC %s
+// RUN: %clang -### %s -no-canonical-prefixes --target=avr --sysroot=%S/Inputs/basic_avr_tree 2>&1 -nostdlibinc | FileCheck --check-prefix=NOSTDINC %s
// NOSTDINC-NOT: "-internal-isystem" {{".*avr/include"}}
// RUN: %clang -### --target=avr --sysroot=%S/Inputs/basic_avr_tree -mmcu=atmega328 %s 2>&1 | FileCheck --check-prefix=NOWARN %s
diff --git a/clang/test/Driver/baremetal.cpp b/clang/test/Driver/baremetal.cpp
index 4dc320191317e..aa92c3171588a 100644
--- a/clang/test/Driver/baremetal.cpp
+++ b/clang/test/Driver/baremetal.cpp
@@ -5,6 +5,7 @@
// CHECK-STATIC-LIB: {{.*}}llvm-ar{{.*}}" "rcsD"
// RUN: %clang %s -### --target=armv6m-none-eabi -o %t.out 2>&1 \
+// RUN: -no-canonical-prefixes \
// RUN: -T semihosted.lds \
// RUN: -L some/directory/user/asked/for \
// RUN: --sysroot=%S/Inputs/baremetal_arm \
@@ -33,6 +34,7 @@
// CHECK-V6M-LIBINC-NOT: "-internal-isystem"
// RUN: %clang %s -### --target=armv6m-none-eabi -o %t.out 2>&1 \
+// RUN: -no-canonical-prefixes \
// RUN: -ccc-install-dir %S/Inputs/basic_baremetal_tree/bin \
// RUN: | FileCheck --check-prefix=CHECK-V6M-TREE %s
// CHECK-V6M-TREE: InstalledDir: [[INSTALLED_DIR:.+]]
diff --git a/clang/test/Driver/cygwin.cpp b/clang/test/Driver/cygwin.cpp
index dd75c48ddc6b3..88ef84fb9d1ee 100644
--- a/clang/test/Driver/cygwin.cpp
+++ b/clang/test/Driver/cygwin.cpp
@@ -1,4 +1,5 @@
// RUN: %clang -### %s --target=i686-pc-windows-cygnus --sysroot=%S/Inputs/basic_cygwin_tree \
+// RUN: -no-canonical-prefixes \
// RUN: -resource-dir=%S/Inputs/resource_dir \
// RUN: --stdlib=platform 2>&1 | FileCheck --check-prefix=CHECK %s
// CHECK: "-cc1"
@@ -31,6 +32,7 @@
// CHECK-SHARED-SAME: "-shared"
// RUN: %clang -### -o %t %s 2>&1 -no-integrated-as -fuse-ld=ld \
+// RUN: -no-canonical-prefixes \
// RUN: --gcc-toolchain=%S/Inputs/basic_cross_cygwin_tree/usr \
// RUN: --target=i686-pc-cygwin \
// RUN: | FileCheck --check-prefix=CHECK-CROSS %s
@@ -38,6 +40,7 @@
// CHECK-CROSS: "{{.*}}/Inputs/basic_cross_cygwin_tree/usr/lib/gcc/i686-pc-msys/10/../../../../i686-pc-msys/bin{{(/|\\\\)}}as" "--32"
// RUN: %clang -### %s --target=x86_64-pc-windows-cygnus --sysroot=%S/Inputs/basic_cygwin_tree \
+// RUN: -no-canonical-prefixes \
// RUN: -resource-dir=%S/Inputs/resource_dir \
// RUN: --stdlib=platform 2>&1 | FileCheck --check-prefix=CHECK-64 %s
// CHECK-64: "-cc1"
diff --git a/clang/test/Driver/darwin-header-search-libcxx-2.cpp b/clang/test/Driver/darwin-header-search-libcxx-2.cpp
index 055b653614dab..81992a2092c5e 100644
--- a/clang/test/Driver/darwin-header-search-libcxx-2.cpp
+++ b/clang/test/Driver/darwin-header-search-libcxx-2.cpp
@@ -50,7 +50,7 @@
// 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: "-internal-isystem" "[[TOOLCHAIN]]/include/c++/v1"
// CHECK-TOOLCHAIN-INCLUDE-CXX-V1-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
// Headers in (2) and nowhere else -> (2) is used
@@ -61,4 +61,4 @@
// 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: "-internal-isystem" "[[TOOLCHAIN]]/bin/../include/c++/v1"
+// CHECK-TOOLCHAIN-NO-SYSROOT: "-internal-isystem" "[[TOOLCHAIN]]/include/c++/v1"
diff --git a/clang/test/Driver/darwin-header-search-libcxx.cpp b/clang/test/Driver/darwi...
[truncated]
|
|
It looks like precommit CI found relevant failures: |
There was a problem hiding this comment.
I think the changes look reasonable, but I'd like to hear from @jyknight whether he's okay with this approach and that it's not disruptive for his downstream.
jyknight
reviewed
Jul 15, 2025
There was a problem hiding this comment.
The plan of applying only to "internal-isystem"-related flags feels reasonable, although more expansive than I had originally proposed (to apply canonicalization only to finding the GCC installation directories).
I'll note that this does have the potentially-odd result of canonicalizing some of the include paths found from --sysroot, without actually canonicalizing sysroot itself. But I don't know of anything concrete that'd be negatively-impacted by that.
So I think this seems OK, modulo some comments.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is another attempt at implementing #143520, but with a different strategy this time: Instead of simplifying paths that contain
..when we print them, we now canonicalise any include directories added viaaddSystemInclude()and friends (i.e. all the-internal-options such as-internal-isystem) in the driver. If the canonical path ends up being shorter, we forward it to the frontend instead of forwarding the original path.The motivation for this (as well as the original patch) is that it can significantly shorten file paths to standard library headers, e.g. on my system,
<ranges>is currently printed as/usr/lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/rangesbut with this change, we instead print
/usr/include/c++/15/rangesAdditionally, since there was a request to allow disabling this functionality, passing
-no-canonical-prefixesnow also disables this canonicalisation. This also greatly simplified fixing all of the tests: most of the changes are just adding-no-canonical-prefixesto a few hundred RUN lines.