[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-03-19 Thread Evan Wilde via cfe-commits 

https://github.com/etcwilde closed 
https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-03-19 Thread Evan Wilde via cfe-commits 

https://github.com/etcwilde updated 
https://github.com/llvm/llvm-project/pull/82084

>From 0d01d10b4f162694a0de205fc2cf9ffd1adf54e0 Mon Sep 17 00:00:00 2001
From: Evan Wilde 
Date: Fri, 16 Feb 2024 16:39:10 -0800
Subject: [PATCH] Support sysroot-relative header search paths

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`.
---
 clang/lib/Frontend/CompilerInvocation.cpp | 43 +++
 clang/test/Preprocessor/sysroot-prefix.c  | 25 +
 2 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 2a21a9d619dc0b..0df6a82ccd8933 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3191,6 +3191,22 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   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 PrefixHeaderPath = [IsSysrootSpecified,
+   ](const llvm::opt::Arg *A,
+  bool IsFramework = false) -> std::string {
+assert(A->getNumValues() && "Unexpected empty search path flag!");
+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.
@@ -3202,16 +3218,7 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
 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(PrefixHeaderPath(A, IsFramework), Group, IsFramework,
  /*IgnoreSysroot*/ true);
 IsIndexHeaderMap = false;
   }
@@ -3229,12 +3236,18 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   }
 
   for (const auto *A : Args.filtered(OPT_idirafter))
-Opts.AddPath(A->getValue(), frontend::After, false, true);
+Opts.AddPath(PrefixHeaderPath(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(A->getValue(), frontend::System, false,
- !A->getOption().matches(OPT_iwithsysroot));
+Opts.AddPath(PrefixHeaderPath(A), frontend::Quoted, false, true);
+
+  for (const auto *A : Args.filtered(OPT_isystem, OPT_iwithsysroot)) {
+if (A->getOption().matches(OPT_iwithsysroot)) {
+  Opts.AddPath(A->getValue(), frontend::System, false,
+   /*IgnoreSysRoot=*/false);
+  continue;
+}
+Opts.AddPath(PrefixHeaderPath(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..eff71f5e3d5a36 100644
--- a/clang/test/Preprocessor/sysroot-prefix.c
+++ b/clang/test/Preprocessor/sysroot-prefix.c
@@ -4,6 +4,16 @@
 // 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 |

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-03-19 Thread Evan Wilde via cfe-commits 


@@ -3191,6 +3191,22 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   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 PrefixHeaderPath = [IsSysrootSpecified,
+   ](const llvm::opt::Arg *A,
+  bool IsFramework = false) -> std::string {
+assert(A->getNumValues() != 0 && "Unexpected empty search path flag!");

etcwilde wrote:

Heh, that's all the Swift I've been writing lately coming through.

https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-03-19 Thread Saleem Abdulrasool via cfe-commits 

https://github.com/compnerd edited 
https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-03-19 Thread Saleem Abdulrasool via cfe-commits 

https://github.com/compnerd approved this pull request.


https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-03-19 Thread Saleem Abdulrasool via cfe-commits 


@@ -3191,6 +3191,22 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   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 PrefixHeaderPath = [IsSysrootSpecified,
+   ](const llvm::opt::Arg *A,
+  bool IsFramework = false) -> std::string {
+assert(A->getNumValues() != 0 && "Unexpected empty search path flag!");

compnerd wrote:

I'd likely drop the `!= 0`.

https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-03-18 Thread Evan Wilde via cfe-commits 

https://github.com/etcwilde updated 
https://github.com/llvm/llvm-project/pull/82084

>From 9e665d05743022350e06f4ea357ecfecde82efb8 Mon Sep 17 00:00:00 2001
From: Evan Wilde 
Date: Fri, 16 Feb 2024 16:39:10 -0800
Subject: [PATCH] Support sysroot-relative header search paths

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`.
---
 clang/lib/Frontend/CompilerInvocation.cpp | 43 +++
 clang/test/Preprocessor/sysroot-prefix.c  | 25 +
 2 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 2a21a9d619dc0b..650e05b2bf33a0 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3191,6 +3191,22 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   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 PrefixHeaderPath = [IsSysrootSpecified,
+   ](const llvm::opt::Arg *A,
+  bool IsFramework = false) -> std::string {
+assert(A->getNumValues() != 0 && "Unexpected empty search path flag!");
+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.
@@ -3202,16 +3218,7 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
 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(PrefixHeaderPath(A, IsFramework), Group, IsFramework,
  /*IgnoreSysroot*/ true);
 IsIndexHeaderMap = false;
   }
@@ -3229,12 +3236,18 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   }
 
   for (const auto *A : Args.filtered(OPT_idirafter))
-Opts.AddPath(A->getValue(), frontend::After, false, true);
+Opts.AddPath(PrefixHeaderPath(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(A->getValue(), frontend::System, false,
- !A->getOption().matches(OPT_iwithsysroot));
+Opts.AddPath(PrefixHeaderPath(A), frontend::Quoted, false, true);
+
+  for (const auto *A : Args.filtered(OPT_isystem, OPT_iwithsysroot)) {
+if (A->getOption().matches(OPT_iwithsysroot)) {
+  Opts.AddPath(A->getValue(), frontend::System, false,
+   /*IgnoreSysRoot=*/false);
+  continue;
+}
+Opts.AddPath(PrefixHeaderPath(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..eff71f5e3d5a36 100644
--- a/clang/test/Preprocessor/sysroot-prefix.c
+++ b/clang/test/Preprocessor/sysroot-prefix.c
@@ -4,6 +4,16 @@
 // 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 |

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-03-18 Thread Jan Svoboda via cfe-commits 

https://github.com/jansvoboda11 approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-03-18 Thread Evan Wilde via cfe-commits 


@@ -3218,6 +3218,21 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   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,
+](const llvm::opt::Arg *A,
+   bool IsFramework = false) -> std::string {
+if (IsSysrootSpecified && !IsFramework && A->getValue()[0] == '=') {

etcwilde wrote:

> I think that Prefix or Expand would both be better than Convert. We aren't 
> really converting this.

There, changed to `PrefixHeaderPath`.

https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-03-18 Thread Evan Wilde via cfe-commits 


@@ -3218,6 +3218,21 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   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,
+](const llvm::opt::Arg *A,
+   bool IsFramework = false) -> std::string {
+if (IsSysrootSpecified && !IsFramework && A->getValue()[0] == '=') {

etcwilde wrote:

Maybe? The thing coming from `Args.filtered` should be populated though? Even 
if it isn't, we shouldn't be entering the `for`-loop though. Perhaps 
`Args.filtered` should result in iterator that emits a const reference? That 
seems like a bigger, unrelated change though.

Added an assert and a test case. The arg parser will error out earlier if 
nothing is passed to `-I`; `argument to '-I' is missing`. Is that sufficient?

https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-03-18 Thread Evan Wilde via cfe-commits 

https://github.com/etcwilde edited 
https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-03-18 Thread Evan Wilde via cfe-commits 


@@ -3256,12 +3262,14 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   }
 
   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))

etcwilde wrote:

Fixed

https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-03-18 Thread Evan Wilde via cfe-commits 

https://github.com/etcwilde updated 
https://github.com/llvm/llvm-project/pull/82084

>From 4c147d0508df51ca713b182dd8b85130cafb1f6c Mon Sep 17 00:00:00 2001
From: Evan Wilde 
Date: Fri, 16 Feb 2024 16:39:10 -0800
Subject: [PATCH] Support sysroot-relative header search paths

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`.
---
 clang/lib/Frontend/CompilerInvocation.cpp | 43 +++
 clang/test/Preprocessor/sysroot-prefix.c  | 25 +
 2 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 2a21a9d619dc0b..650e05b2bf33a0 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3191,6 +3191,22 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   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 PrefixHeaderPath = [IsSysrootSpecified,
+   ](const llvm::opt::Arg *A,
+  bool IsFramework = false) -> std::string {
+assert(A->getNumValues() != 0 && "Unexpected empty search path flag!");
+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.
@@ -3202,16 +3218,7 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
 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(PrefixHeaderPath(A, IsFramework), Group, IsFramework,
  /*IgnoreSysroot*/ true);
 IsIndexHeaderMap = false;
   }
@@ -3229,12 +3236,18 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   }
 
   for (const auto *A : Args.filtered(OPT_idirafter))
-Opts.AddPath(A->getValue(), frontend::After, false, true);
+Opts.AddPath(PrefixHeaderPath(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(A->getValue(), frontend::System, false,
- !A->getOption().matches(OPT_iwithsysroot));
+Opts.AddPath(PrefixHeaderPath(A), frontend::Quoted, false, true);
+
+  for (const auto *A : Args.filtered(OPT_isystem, OPT_iwithsysroot)) {
+if (A->getOption().matches(OPT_iwithsysroot)) {
+  Opts.AddPath(A->getValue(), frontend::System, false,
+   /*IgnoreSysRoot=*/false);
+  continue;
+}
+Opts.AddPath(PrefixHeaderPath(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..5c84380e5c6a07 100644
--- a/clang/test/Preprocessor/sysroot-prefix.c
+++ b/clang/test/Preprocessor/sysroot-prefix.c
@@ -4,6 +4,16 @@
 // 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 |

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-02-17 Thread Saleem Abdulrasool via cfe-commits 


@@ -3218,6 +3218,21 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   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,
+](const llvm::opt::Arg *A,
+   bool IsFramework = false) -> std::string {
+if (IsSysrootSpecified && !IsFramework && A->getValue()[0] == '=') {

compnerd wrote:

路‍♂️ clang-format does what clang-format wants.

https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-02-17 Thread Evan Wilde via cfe-commits 


@@ -3229,16 +3244,7 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
 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,

etcwilde wrote:

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.

https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-02-17 Thread Evan Wilde via cfe-commits 


@@ -3218,6 +3218,21 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   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,
+](const llvm::opt::Arg *A,
+   bool IsFramework = false) -> std::string {
+if (IsSysrootSpecified && !IsFramework && A->getValue()[0] == '=') {

etcwilde wrote:

I just followed the whims of `git clang-format` here. ¯\\\_(ツ)\_/¯

https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-02-17 Thread Saleem Abdulrasool via cfe-commits 


@@ -3218,6 +3218,21 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   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,
+](const llvm::opt::Arg *A,
+   bool IsFramework = false) -> std::string {
+if (IsSysrootSpecified && !IsFramework && A->getValue()[0] == '=') {

compnerd wrote:

This feels double indented?

https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-02-17 Thread Saleem Abdulrasool via cfe-commits 


@@ -3218,6 +3218,21 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   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,

compnerd wrote:

I think that `Prefix` or `Expand` would both be better than `Convert`. We 
aren't really converting this.

https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-02-17 Thread Saleem Abdulrasool via cfe-commits 


@@ -3218,6 +3218,21 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   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,
+](const llvm::opt::Arg *A,
+   bool IsFramework = false) -> std::string {
+if (IsSysrootSpecified && !IsFramework && A->getValue()[0] == '=') {

compnerd wrote:

Might be nice to take `const llvm::opt::Arg &` instead. It also feels unsafe as 
we do not know if the argument has a value before you get it - you should do a 
`A->hasValue()` or assert it perhaps?

https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-02-16 Thread Evan Wilde via cfe-commits 


@@ -3256,12 +3262,14 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   }
 
   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))

etcwilde wrote:

Good catch, it was not.

https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-02-16 Thread Jan Svoboda via cfe-commits 


@@ -3229,16 +3244,7 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
 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,

jansvoboda11 wrote:

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?

https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-02-16 Thread Jan Svoboda via cfe-commits 


@@ -3256,12 +3262,14 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   }
 
   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))

jansvoboda11 wrote:

Handling `OPT_isystem` in a loop separate from `OPT_iwithsysroot` is a behavior 
change. Is that intentional?

Previously "-isystem A -iwithsysroot B -isystem C" would result in "A B C", now 
it results in "B A C".

https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-02-16 Thread Evan Wilde via cfe-commits 


@@ -3218,6 +3218,21 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions 
, ArgList ,
   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,

etcwilde wrote:

I don't love this name. Perhaps `ExpandHeaderSysroot`?

https://github.com/llvm/llvm-project/pull/82084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-02-16 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Evan Wilde (etcwilde)


Changes

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/82084.diff


2 Files Affected:

- (modified) clang/lib/Frontend/CompilerInvocation.cpp (+22-14) 
- (modified) clang/test/Preprocessor/sysroot-prefix.c (+14) 


``diff
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 
, ArgList ,
   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,
+](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 
, ArgList ,
 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 
, ArgList ,
   }
 
   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

[clang] Add support for sysroot-relative system header search paths (PR #82084)

2024-02-16 Thread Evan Wilde via cfe-commits 

https://github.com/etcwilde created 
https://github.com/llvm/llvm-project/pull/82084

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.

>From 37bcd5380893cb489c012508613919fa05254a94 Mon Sep 17 00:00:00 2001
From: Evan Wilde 
Date: Fri, 16 Feb 2024 16:39:10 -0800
Subject: [PATCH] Support sysroot-relative header search paths

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`.
---
 clang/lib/Frontend/CompilerInvocation.cpp | 36 ++-
 clang/test/Preprocessor/sysroot-prefix.c  | 14 +
 2 files changed, 36 insertions(+), 14 deletions(-)

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 
, ArgList ,
   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,
+](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 
, ArgList ,
 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 
, ArgList ,
   }
 
   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