-
Notifications
You must be signed in to change notification settings - Fork 10.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for sysroot-relative system header search paths #82084
Add support for sysroot-relative system header search paths #82084
Conversation
@llvm/pr-subscribers-clang Author: Evan Wilde (etcwilde) ChangesClang supported header searchpaths of the form This implements the Full diff: https://github.com/llvm/llvm-project/pull/82084.diff 2 Files Affected:
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index bcb31243056b7e..4b5667f6ae5a5d 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3218,6 +3218,21 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
bool IsIndexHeaderMap = false;
bool IsSysrootSpecified =
Args.hasArg(OPT__sysroot_EQ) || Args.hasArg(OPT_isysroot);
+
+ // Expand a leading `=` to the sysroot if one was passed (and it's not a
+ // framework flag).
+ auto ConvertHeaderPath = [IsSysrootSpecified,
+ &Opts](const llvm::opt::Arg *A,
+ bool IsFramework = false) -> std::string {
+ if (IsSysrootSpecified && !IsFramework && A->getValue()[0] == '=') {
+ SmallString<32> Buffer;
+ llvm::sys::path::append(Buffer, Opts.Sysroot,
+ llvm::StringRef(A->getValue()).substr(1));
+ return std::string(Buffer);
+ }
+ return A->getValue();
+ };
+
for (const auto *A : Args.filtered(OPT_I, OPT_F, OPT_index_header_map)) {
if (A->getOption().matches(OPT_index_header_map)) {
// -index-header-map applies to the next -I or -F.
@@ -3229,16 +3244,7 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
IsIndexHeaderMap ? frontend::IndexHeaderMap : frontend::Angled;
bool IsFramework = A->getOption().matches(OPT_F);
- std::string Path = A->getValue();
-
- if (IsSysrootSpecified && !IsFramework && A->getValue()[0] == '=') {
- SmallString<32> Buffer;
- llvm::sys::path::append(Buffer, Opts.Sysroot,
- llvm::StringRef(A->getValue()).substr(1));
- Path = std::string(Buffer);
- }
-
- Opts.AddPath(Path, Group, IsFramework,
+ Opts.AddPath(ConvertHeaderPath(A, IsFramework), Group, IsFramework,
/*IgnoreSysroot*/ true);
IsIndexHeaderMap = false;
}
@@ -3256,12 +3262,14 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
}
for (const auto *A : Args.filtered(OPT_idirafter))
- Opts.AddPath(A->getValue(), frontend::After, false, true);
+ Opts.AddPath(ConvertHeaderPath(A), frontend::After, false, true);
for (const auto *A : Args.filtered(OPT_iquote))
- Opts.AddPath(A->getValue(), frontend::Quoted, false, true);
- for (const auto *A : Args.filtered(OPT_isystem, OPT_iwithsysroot))
+ Opts.AddPath(ConvertHeaderPath(A), frontend::Quoted, false, true);
+ for (const auto *A : Args.filtered(OPT_iwithsysroot))
Opts.AddPath(A->getValue(), frontend::System, false,
- !A->getOption().matches(OPT_iwithsysroot));
+ /*IgnoreSysRoot=*/false);
+ for (const auto *A : Args.filtered(OPT_isystem))
+ Opts.AddPath(ConvertHeaderPath(A), frontend::System, false, true);
for (const auto *A : Args.filtered(OPT_iframework))
Opts.AddPath(A->getValue(), frontend::System, true, true);
for (const auto *A : Args.filtered(OPT_iframeworkwithsysroot))
diff --git a/clang/test/Preprocessor/sysroot-prefix.c b/clang/test/Preprocessor/sysroot-prefix.c
index 08c72f53b44e9f..ce19790819b732 100644
--- a/clang/test/Preprocessor/sysroot-prefix.c
+++ b/clang/test/Preprocessor/sysroot-prefix.c
@@ -4,6 +4,12 @@
// RUN: %clang_cc1 -v -isysroot /var/empty -I =null -E %s -o /dev/null 2>&1 | FileCheck -check-prefix CHECK-ISYSROOT_SYSROOT_NULL %s
// RUN: %clang_cc1 -v -isysroot /var/empty -isysroot /var/empty/root -I =null -E %s -o /dev/null 2>&1 | FileCheck -check-prefix CHECK-ISYSROOT_ISYSROOT_SYSROOT_NULL %s
// RUN: %clang_cc1 -v -isysroot /var/empty/root -isysroot /var/empty -I =null -E %s -o /dev/null 2>&1 | FileCheck -check-prefix CHECK-ISYSROOT_ISYSROOT_SWAPPED_SYSROOT_NULL %s
+// RUN: %clang_cc1 -v -isystem=/usr/include -isysroot /var/empty -E %s -o /dev/null 2>&1 | FileCheck -check-prefix CHECK-ISYSROOT_ISYSTEM_SYSROOT %s
+// RUN: %clang_cc1 -v -isystem=/usr/include -E %s -o /dev/null 2>&1 | FileCheck -check-prefix CHECK-ISYSROOT_ISYSTEM_NO_SYSROOT %s
+// RUN: %clang_cc1 -v -iquote=/usr/include -isysroot /var/empty -E %s -o /dev/null 2>&1 | FileCheck -check-prefix CHECK-ISYSROOT_IQUOTE_SYSROOT %s
+// RUN: %clang_cc1 -v -iquote=/usr/include -E %s -o /dev/null 2>&1 | FileCheck -check-prefix CHECK-ISYSROOT_IQUOTE_NO_SYSROOT %s
+// RUN: %clang_cc1 -v -idirafter=/usr/include -isysroot /var/empty -E %s -o /dev/null 2>&1 | FileCheck -check-prefix CHECK-ISYSROOT_IDIRAFTER_SYSROOT %s
+// RUN: %clang_cc1 -v -idirafter=/usr/include -E %s -o /dev/null 2>&1 | FileCheck -check-prefix CHECK-ISYSROOT_IDIRAFTER_NO_SYSROOT %s
// CHECK-ISYSROOT_NO_SYSROOT: ignoring nonexistent directory "/var/empty/include"
// CHECK-ISYSROOT_NO_SYSROOT-NOT: ignoring nonexistent directory "/var/empty/var/empty/include"
@@ -23,3 +29,11 @@
// CHECK-ISYSROOT_ISYSROOT_SWAPPED_SYSROOT_NULL: ignoring nonexistent directory "/var/empty{{.}}null"
// CHECK-ISYSROOT_ISYSROOT_SWAPPED_SYSROOT_NULL-NOT: ignoring nonexistent directory "=null"
+// CHECK-ISYSROOT_ISYSTEM_SYSROOT: ignoring nonexistent directory "/var/empty/usr/include"
+// CHECK-ISYSROOT_ISYSTEM_NO_SYSROOT: ignoring nonexistent directory "=/usr/include"
+
+// CHECK-ISYSROOT_IQUOTE_SYSROOT: ignoring nonexistent directory "/var/empty/usr/include"
+// CHECK-ISYSROOT_IQUOTE_NO_SYSROOT: ignoring nonexistent directory "=/usr/include"
+
+// CHECK-ISYSROOT_IDIRAFTER_SYSROOT: ignoring nonexistent directory "/var/empty/usr/include"
+// CHECK-ISYSROOT_IDIRAFTER_NO_SYSROOT: ignoring nonexistent directory "=/usr/include"
|
} | ||
|
||
Opts.AddPath(Path, Group, IsFramework, | ||
Opts.AddPath(ConvertHeaderPath(A, IsFramework), Group, IsFramework, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of prefixing the path with sysroot here, would it make sense to defer to InitHeaderSearch::AddPath()
by calling Opts.AddPath(..., /*IgnoreSysRoot=*/false)
? Note that this function only adds the prefix to absolute paths. Does that match GCC behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had considered doing something like checking whether the first character is an =
and if the sysroot option is set and passing true
to IgnoreSysRoot
when that's the case. It unfortunately does not match behavior. I don't know how useful it is, but --sysroot /foo/bar -isystem=baz
will result in the searchpath /foo/barbaz
in gcc. (that's actually a good testcase, I should add that).
Given that we still need to strip the leading =
from the searchpath with -isystem
though not with -iwithsysroot
, and that we want to match the -I=
behavior, I decided to stick with this to minimize the impact of the patch. I could try to hunt down all of the places where the UserEntries
list is iterated and ensure that they're doing the appropriate sysroot prefixing for the corresponding flags.
37bcd53
to
4c147d0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
4c147d0
to
9e665d0
Compare
Clang supported header searchpaths of the form `-I =/path`, relative to the sysroot if one is passed, but did not implement that behavior for `-iquote`, `-isystem`, or `-idirafter`.
9e665d0
to
0d01d10
Compare
Clang supported header searchpaths of the form `-I =/path`, relative to the sysroot if one is passed, but did not implement that behavior for `-iquote`, `-isystem`, or `-idirafter`. This implements the `=` portion of the behavior implemented by GCC for these flags described in https://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html.
Clang supported header searchpaths of the form
-I =/path
, relative to the sysroot if one is passed, but did not implement that behavior for-iquote
,-isystem
, or-idirafter
.This implements the
=
portion of the behavior implemented by GCC for these flags described in https://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html.