[clang] [Driver] Unify InstalledDir and Dir (PR #80527)
https://github.com/MaskRay closed https://github.com/llvm/llvm-project/pull/80527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Unify InstalledDir and Dir (PR #80527)
https://github.com/mstorsjo approved this pull request. If nobody else wants to chime in here, I guess I'll go ahead and approve it. LGTM! https://github.com/llvm/llvm-project/pull/80527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Unify InstalledDir and Dir (PR #80527)
MaskRay wrote: Ping:) https://github.com/llvm/llvm-project/pull/80527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Unify InstalledDir and Dir (PR #80527)
MaskRay wrote: > From my point of view, I think this sounds fine - it looks like you've looked > through this and thought deeper about it than I have at least. I'm not deep > enough into this to comfortably press approved on this for now though, so > hopefully someone else can chime in as well. Thanks. Yes, I think should be done. --- Ping https://github.com/llvm/llvm-project/pull/80527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Unify InstalledDir and Dir (PR #80527)
mstorsjo wrote: From my point of view, I think this sounds fine - it looks like you've looked through this and thought deeper about it than I have at least. I'm not deep enough into this to comfortably press approved on this for now though, so hopefully someone else can chime in as well. https://github.com/llvm/llvm-project/pull/80527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Unify InstalledDir and Dir (PR #80527)
MaskRay wrote: Ping:) @ldionne I know you are discussing a `-isysroot` related issue (don't recall which one off my head) for Apple platforms. I think unifying InstalledDir and Dir will help. https://github.com/llvm/llvm-project/pull/80527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Unify InstalledDir and Dir (PR #80527)
https://github.com/MaskRay edited https://github.com/llvm/llvm-project/pull/80527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Unify InstalledDir and Dir (PR #80527)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Fangrui Song (MaskRay)
Changes
`Driver::ClangExecutable` is derived from:
* (-canonical-prefixes default): `realpath` on the executable path
* (-no-canonical-prefixes) argv[0] (consult PATH if argv[0] is a word)
`Dir` and `ResourceDir` are derived from `ClangExecutable`. Both
variables are used to derive certain include and library paths.
`InstalledDir` is a related concept used to derive certain other paths.
`InstalledDir` is weird in the -canonical-prefixes mode: Clang
calls `make_absolute` but does not follow symlinks
(FIXME from 9ade6a9a747c49383ac747bd8ffaa6a0beaef71c).
This causes some search and library paths to be mix-and-matched.
I believe `InstalledDir` was a partial solution to
`-no-canonical-prefixes` and should be eventually removed.
This patch removes `SetInstallDir` and relies on Driver::Driver to set
`InstalledDir` to `Dir`. The behavior for regular file `clang` or
`-no-canonical-prefixes` is unchanged.
If a user creates a symlink to the regular file `clang` and uses the
default `-canonical-prefixes`, they now consistently get search and
library paths relative to the regular file `clang`, not mix-and-match
paths.
If a user creates a symlink to the regular file `clang` and replaces
some directorys from the actual installation, they should change the
symlink to a wrapper that calls the underlying clang with
`-no-canonical-prefixes`.
---
Full diff: https://github.com/llvm/llvm-project/pull/80527.diff
7 Files Affected:
- (modified) clang/include/clang/Driver/Driver.h (+1-2)
- (modified) clang/test/Driver/darwin-header-search-libcxx.cpp (+1-1)
- (modified) clang/test/Driver/mingw-sysroot.cpp (+7-5)
- (modified) clang/test/Driver/no-canonical-prefixes.c (+1-1)
- (modified) clang/test/Driver/program-path-priority.c (+8-5)
- (modified) clang/test/Driver/rocm-detect.hip (+2-2)
- (modified) clang/tools/driver/driver.cpp (-23)
``diff
diff --git a/clang/include/clang/Driver/Driver.h
b/clang/include/clang/Driver/Driver.h
index 3ee1bcf2a69c9..afd388dfaf017 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -160,7 +160,7 @@ class Driver {
/// Target and driver mode components extracted from clang executable name.
ParsedClangName ClangNameParts;
- /// The path to the installed clang directory, if any.
+ /// TODO: Remove this in favor of Dir.
std::string InstalledDir;
/// The path to the compiler resource directory.
@@ -419,7 +419,6 @@ class Driver {
return InstalledDir.c_str();
return Dir.c_str();
}
- void setInstalledDir(StringRef Value) { InstalledDir = std::string(Value); }
bool isSaveTempsEnabled() const { return SaveTemps != SaveTempsNone; }
bool isSaveTempsObj() const { return SaveTemps == SaveTempsObj; }
diff --git a/clang/test/Driver/darwin-header-search-libcxx.cpp
b/clang/test/Driver/darwin-header-search-libcxx.cpp
index 70cc06090a993..5695f53683bab 100644
--- a/clang/test/Driver/darwin-header-search-libcxx.cpp
+++ b/clang/test/Driver/darwin-header-search-libcxx.cpp
@@ -193,7 +193,7 @@
// RUN: ln -sf %t/install/bin/clang %t/symlinked1/bin/clang
// RUN: mkdir -p %t/symlinked1/include/c++/v1
-// RUN: %t/symlinked1/bin/clang -### %s -fsyntax-only 2>&1 \
+// RUN: %t/symlinked1/bin/clang -### %s -no-canonical-prefixes -fsyntax-only
2>&1 \
// RUN: --target=x86_64-apple-darwin \
// RUN: -stdlib=libc++ \
// RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
diff --git a/clang/test/Driver/mingw-sysroot.cpp
b/clang/test/Driver/mingw-sysroot.cpp
index 50152b2ca210d..5d512e6669709 100644
--- a/clang/test/Driver/mingw-sysroot.cpp
+++ b/clang/test/Driver/mingw-sysroot.cpp
@@ -50,10 +50,12 @@
// CHECK_TESTROOT_GCC_EXPLICIT: "-internal-isystem"
"{{[^"]+}}/testroot-gcc{{/|}}include"
-// If there's a matching sysroot next to the clang binary itself, prefer that
+// If -no-canonical-prefixes and there's a matching sysroot next to the clang
binary itself, prefer that
// over a gcc in the path:
-// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%"
%T/testroot-clang/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32
-rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck
-check-prefix=CHECK_TESTROOT_CLANG %s
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%"
%T/testroot-clang/bin/x86_64-w64-mingw32-clang --target=x86_64-w64-mingw32
-rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck
-check-prefix=CHECK_TESTROOT_GCC2 %s
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%"
%T/testroot-clang/bin/x86_64-w64-mingw32-clang --target=x86_64-w64-mingw32
-rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s
-no-canonical-prefixes 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG %s
+// CHECK_TESTROOT_GCC2:
"{{[^"]+}}/testroot-gcc{{/|}}x86_64-w64-mingw32{{/|}}include"
// CHECK_TESTROOT_CLANG:
[clang] [Driver] Unify InstalledDir and Dir (PR #80527)
https://github.com/MaskRay created
https://github.com/llvm/llvm-project/pull/80527
`Driver::ClangExecutable` is derived from:
* (-canonical-prefixes default): `realpath` on the executable path
* (-no-canonical-prefixes) argv[0] (consult PATH if argv[0] is a word)
`Dir` and `ResourceDir` are derived from `ClangExecutable`. Both
variables are used to derive certain include and library paths.
`InstalledDir` is a related concept used to derive certain other paths.
`InstalledDir` is weird in the -canonical-prefixes mode: Clang
calls `make_absolute` but does not follow symlinks
(FIXME from 9ade6a9a747c49383ac747bd8ffaa6a0beaef71c).
This causes some search and library paths to be mix-and-matched.
I believe `InstalledDir` was a partial solution to
`-no-canonical-prefixes` and should be eventually removed.
This patch removes `SetInstallDir` and relies on Driver::Driver to set
`InstalledDir` to `Dir`. The behavior for regular file `clang` or
`-no-canonical-prefixes` is unchanged.
If a user creates a symlink to the regular file `clang` and uses the
default `-canonical-prefixes`, they now consistently get search and
library paths relative to the regular file `clang`, not mix-and-match
paths.
If a user creates a symlink to the regular file `clang` and replaces
some directorys from the actual installation, they should change the
symlink to a wrapper that calls the underlying clang with
`-no-canonical-prefixes`.
>From 0d249fb7ee4c53922d2a620822feb550f477e9b9 Mon Sep 17 00:00:00 2001
From: Fangrui Song
Date: Fri, 2 Feb 2024 20:38:17 -0800
Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?=
=?UTF-8?q?l=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.4
---
clang/include/clang/Driver/Driver.h | 3 +--
.../Driver/darwin-header-search-libcxx.cpp| 2 +-
clang/test/Driver/mingw-sysroot.cpp | 12 ++
clang/test/Driver/no-canonical-prefixes.c | 2 +-
clang/test/Driver/program-path-priority.c | 13 +++
clang/test/Driver/rocm-detect.hip | 4 ++--
clang/tools/driver/driver.cpp | 23 ---
7 files changed, 20 insertions(+), 39 deletions(-)
diff --git a/clang/include/clang/Driver/Driver.h
b/clang/include/clang/Driver/Driver.h
index 3ee1bcf2a69c9..afd388dfaf017 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -160,7 +160,7 @@ class Driver {
/// Target and driver mode components extracted from clang executable name.
ParsedClangName ClangNameParts;
- /// The path to the installed clang directory, if any.
+ /// TODO: Remove this in favor of Dir.
std::string InstalledDir;
/// The path to the compiler resource directory.
@@ -419,7 +419,6 @@ class Driver {
return InstalledDir.c_str();
return Dir.c_str();
}
- void setInstalledDir(StringRef Value) { InstalledDir = std::string(Value); }
bool isSaveTempsEnabled() const { return SaveTemps != SaveTempsNone; }
bool isSaveTempsObj() const { return SaveTemps == SaveTempsObj; }
diff --git a/clang/test/Driver/darwin-header-search-libcxx.cpp
b/clang/test/Driver/darwin-header-search-libcxx.cpp
index 70cc06090a993..5695f53683bab 100644
--- a/clang/test/Driver/darwin-header-search-libcxx.cpp
+++ b/clang/test/Driver/darwin-header-search-libcxx.cpp
@@ -193,7 +193,7 @@
// RUN: ln -sf %t/install/bin/clang %t/symlinked1/bin/clang
// RUN: mkdir -p %t/symlinked1/include/c++/v1
-// RUN: %t/symlinked1/bin/clang -### %s -fsyntax-only 2>&1 \
+// RUN: %t/symlinked1/bin/clang -### %s -no-canonical-prefixes -fsyntax-only
2>&1 \
// RUN: --target=x86_64-apple-darwin \
// RUN: -stdlib=libc++ \
// RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
diff --git a/clang/test/Driver/mingw-sysroot.cpp
b/clang/test/Driver/mingw-sysroot.cpp
index 50152b2ca210d..5d512e6669709 100644
--- a/clang/test/Driver/mingw-sysroot.cpp
+++ b/clang/test/Driver/mingw-sysroot.cpp
@@ -50,10 +50,12 @@
// CHECK_TESTROOT_GCC_EXPLICIT: "-internal-isystem"
"{{[^"]+}}/testroot-gcc{{/|}}include"
-// If there's a matching sysroot next to the clang binary itself, prefer that
+// If -no-canonical-prefixes and there's a matching sysroot next to the clang
binary itself, prefer that
// over a gcc in the path:
-// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%"
%T/testroot-clang/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32
-rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck
-check-prefix=CHECK_TESTROOT_CLANG %s
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%"
%T/testroot-clang/bin/x86_64-w64-mingw32-clang --target=x86_64-w64-mingw32
-rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck
-check-prefix=CHECK_TESTROOT_GCC2 %s
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%"
%T/testroot-clang/bin/x86_64-w64-mingw32-clang --target=x86_64-w64-mingw32
-rtlib=compiler-rt
