[clang] [llvm] [Clang] Match MSVC handling of duplicate header search paths in Microsoft compatibility modes. (PR #105738)
Tom Honermann via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 18 21:00:00 PDT 2024
https://github.com/tahonermann updated https://github.com/llvm/llvm-project/pull/105738
>From b775b7288035d83281434e27341b98a76623802f Mon Sep 17 00:00:00 2001
From: Tom Honermann <tom.honermann at intel.com>
Date: Thu, 22 Aug 2024 09:44:56 -0700
Subject: [PATCH 1/3] [Clang] Match MSVC handling of duplicate header search
paths in Microsoft compatibility modes.
Clang has historically mimicked gcc behavior for header file searching in
which user search paths are ordered before system search paths, user search
paths that duplicate a (later) system search path are ignored, and search
paths that duplicate an earlier search path of the same user/system kind are
ignored.
MSVC behavior differs in that user search paths are ordered before system
search paths (like gcc), and search paths that duplicate an earlier search
path are ignored regardless of user/system kind (similar to gcc, but without
the preference for system search paths over duplicate user search paths).
The gcc and MSVC differences are observable for driver invocations that pass,
e.g., `-Idir1 -isystem dir2 -isystem dir1`. The gcc behavior will result in
`dir2` being searched before `dir1` while the MSVC behavior will result in
`dir1` being searched before `dir2`.
This patch modifies Clang to match the MSVC behavior for handling of duplicate
header search paths when running in Microsoft compatibility mode (e.g., when
invoked with the `-fms-compatibility` option explicitly or implicitly enabled).
---
clang/docs/ReleaseNotes.rst | 14 +++
clang/lib/Lex/InitHeaderSearch.cpp | 21 ++--
clang/test/Driver/header-search-duplicates.c | 76 +++++++++++++++
.../microsoft-header-search-duplicates.c | 97 +++++++++++++++++++
4 files changed, 199 insertions(+), 9 deletions(-)
create mode 100644 clang/test/Driver/header-search-duplicates.c
create mode 100644 clang/test/Driver/microsoft-header-search-duplicates.c
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1da8c82d52e618..03c27342f31f66 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -638,6 +638,20 @@ Windows Support
When `-fms-compatibility-version=18.00` or prior is set on the command line this Microsoft extension is still
allowed as VS2013 and prior allow it.
+- Clang now matches MSVC behavior for handling of duplicate header search paths
+ when running in Microsoft compatibility mode. Historically, Clang has
+ mimicked gcc behavior in which user search paths are ordered before
+ system search paths, user search paths that duplicate a (later) system search
+ path are ignored, and search paths that duplicate an earlier search path of
+ the same user/system kind are ignored. The MSVC behavior is that user search
+ paths are ordered before system search paths (like gcc), and search paths that
+ duplicate an earlier search path are ignored regardless of user/system kind
+ (similar to gcc, but without the preference for system search paths over
+ duplicate user search paths). These differences are observable for driver
+ invocations that pass, e.g., `-Idir1 -isystem dir2 -isystem dir1`. The gcc
+ behavior will search `dir2` before `dir1` and the MSVC behavior will search
+ `dir1` before `dir2`.
+
LoongArch Support
^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp b/clang/lib/Lex/InitHeaderSearch.cpp
index 2218db15013d92..3f487f3a4c1c05 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -368,7 +368,8 @@ void InitHeaderSearch::AddDefaultIncludePaths(
/// If there are duplicate directory entries in the specified search list,
/// remove the later (dead) ones. Returns the number of non-system headers
/// removed, which is used to update NumAngled.
-static unsigned RemoveDuplicates(std::vector<DirectoryLookupInfo> &SearchList,
+static unsigned RemoveDuplicates(const LangOptions &Lang,
+ std::vector<DirectoryLookupInfo> &SearchList,
unsigned First, bool Verbose) {
llvm::SmallPtrSet<const DirectoryEntry *, 8> SeenDirs;
llvm::SmallPtrSet<const DirectoryEntry *, 8> SeenFrameworkDirs;
@@ -394,14 +395,15 @@ static unsigned RemoveDuplicates(std::vector<DirectoryLookupInfo> &SearchList,
continue;
}
- // If we have a normal #include dir/framework/headermap that is shadowed
- // later in the chain by a system include location, we actually want to
- // ignore the user's request and drop the user dir... keeping the system
- // dir. This is weird, but required to emulate GCC's search path correctly.
+ // When not in MSVC compatibility mode, if we have a normal
+ // #include dir/framework/headermap that is shadowed later in the chain by
+ // a system include location, we actually want to ignore the user's request
+ // and drop the user dir... keeping the system dir. This is weird, but
+ // required to emulate GCC's search path correctly.
//
// Since dupes of system dirs are rare, just rescan to find the original
// that we're nuking instead of using a DenseMap.
- if (CurEntry.getDirCharacteristic() != SrcMgr::C_User) {
+ if (!Lang.MSVCCompat && CurEntry.getDirCharacteristic() != SrcMgr::C_User) {
// Find the dir that this is the same of.
unsigned FirstDir;
for (FirstDir = First;; ++FirstDir) {
@@ -484,14 +486,14 @@ void InitHeaderSearch::Realize(const LangOptions &Lang) {
SearchList.push_back(Include);
// Deduplicate and remember index.
- RemoveDuplicates(SearchList, 0, Verbose);
+ RemoveDuplicates(Lang, SearchList, 0, Verbose);
unsigned NumQuoted = SearchList.size();
for (auto &Include : IncludePath)
if (Include.Group == Angled || Include.Group == IndexHeaderMap)
SearchList.push_back(Include);
- RemoveDuplicates(SearchList, NumQuoted, Verbose);
+ RemoveDuplicates(Lang, SearchList, NumQuoted, Verbose);
unsigned NumAngled = SearchList.size();
for (auto &Include : IncludePath)
@@ -510,7 +512,8 @@ void InitHeaderSearch::Realize(const LangOptions &Lang) {
// Remove duplicates across both the Angled and System directories. GCC does
// this and failing to remove duplicates across these two groups breaks
// #include_next.
- unsigned NonSystemRemoved = RemoveDuplicates(SearchList, NumQuoted, Verbose);
+ unsigned NonSystemRemoved =
+ RemoveDuplicates(Lang, SearchList, NumQuoted, Verbose);
NumAngled -= NonSystemRemoved;
Headers.SetSearchPaths(extractLookups(SearchList), NumQuoted, NumAngled,
diff --git a/clang/test/Driver/header-search-duplicates.c b/clang/test/Driver/header-search-duplicates.c
new file mode 100644
index 00000000000000..ebd49453a1c10c
--- /dev/null
+++ b/clang/test/Driver/header-search-duplicates.c
@@ -0,0 +1,76 @@
+// Test that the gcc-like driver, when not in Microsoft compatibility mode,
+// emulates the gcc behavior of eliding a user header search path when the
+// same path is present as a system header search path.
+// See microsoft-header-search-duplicates.c for Microsoft compatible behavior.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Test the clang driver with a target that does not implicitly enable the
+// -fms-compatibility option. The -nostdinc option is used to suppress default
+// search paths to ease testing.
+// RUN: %clang \
+// RUN: -target x86_64-unknown-linux-gnu \
+// RUN: -v -fsyntax-only \
+// RUN: -nostdinc \
+// RUN: -I%t/include/w \
+// RUN: -isystem %t/include/z \
+// RUN: -I%t/include/x \
+// RUN: -isystem %t/include/y \
+// RUN: -isystem %t/include/x \
+// RUN: -I%t/include/w \
+// RUN: -isystem %t/include/y \
+// RUN: -isystem %t/include/z \
+// RUN: %t/test.c 2>&1 | FileCheck -DPWD=%t %t/test.c
+
+#--- test.c
+#include <a.h>
+#include <b.h>
+#include <c.h>
+
+// The expected behavior is that user search paths are ordered before system
+// search paths, that user search paths that duplicate a (later) system search
+// path are ignored, and that search paths that duplicate an earlier search
+// path of the same user/system kind are ignored.
+// CHECK: ignoring duplicate directory "[[PWD]]/include/w"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/x"
+// CHECK-NEXT: as it is a non-system directory that duplicates a system directory
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/y"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/z"
+// CHECK: #include <...> search starts here:
+// CHECK-NEXT: [[PWD]]/include/w
+// CHECK-NEXT: [[PWD]]/include/z
+// CHECK-NEXT: [[PWD]]/include/y
+// CHECK-NEXT: [[PWD]]/include/x
+// CHECK-NEXT: End of search list.
+
+#--- include/w/b.h
+#define INCLUDE_W_B_H
+#include_next <b.h>
+
+#--- include/w/c.h
+#define INCLUDE_W_C_H
+#include_next <c.h>
+
+#--- include/x/a.h
+#if !defined(INCLUDE_Y_A_H)
+#error 'include/y/a.h' should have been included before 'include/x/a.h'!
+#endif
+
+#--- include/x/b.h
+#if !defined(INCLUDE_W_B_H)
+#error 'include/w/b.h' should have been included before 'include/x/b.h'!
+#endif
+
+#--- include/x/c.h
+#if !defined(INCLUDE_Z_C_H)
+#error 'include/z/c.h' should have been included before 'include/x/c.h'!
+#endif
+
+#--- include/y/a.h
+#define INCLUDE_Y_A_H
+#include_next <a.h>
+
+#--- include/z/c.h
+#define INCLUDE_Z_C_H
+#include_next <c.h>
diff --git a/clang/test/Driver/microsoft-header-search-duplicates.c b/clang/test/Driver/microsoft-header-search-duplicates.c
new file mode 100644
index 00000000000000..0ab3e88b951c5c
--- /dev/null
+++ b/clang/test/Driver/microsoft-header-search-duplicates.c
@@ -0,0 +1,97 @@
+// Test that the cl-like driver and the gcc-like driver, when in Microsoft
+// compatibility mode, retain user header search paths that are duplicated in
+// system header search paths.
+// See header-search-duplicates.c for gcc compatible behavior.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Test the clang driver with a Windows target that implicitly enables the
+// -fms-compatibility option. The -nostdinc option is used to suppress default
+// search paths to ease testing.
+// RUN: %clang \
+// RUN: -target x86_64-pc-windows \
+// RUN: -v -fsyntax-only \
+// RUN: -nostdinc \
+// RUN: -I%t/include/w \
+// RUN: -isystem %t/include/z \
+// RUN: -I%t/include/x \
+// RUN: -isystem %t/include/y \
+// RUN: -isystem %t/include/x \
+// RUN: -I%t/include/w \
+// RUN: -isystem %t/include/y \
+// RUN: -isystem %t/include/z \
+// RUN: %t/test.c 2>&1 | FileCheck -DPWD=%t %t/test.c
+
+// Test the clang-cl driver with a Windows target that implicitly enables the
+// -fms-compatibility option. The /X option is used instead of -nostdinc
+// because the latter option suppresses all system include paths including
+// those specified by -imsvc. The -nobuiltininc option is uesd to suppress
+// the Clang resource directory. The -nostdlibinc option is used to suppress
+// search paths for the Windows SDK, CRT, MFC, ATL, etc...
+// RUN: %clang_cl \
+// RUN: -target x86_64-pc-windows \
+// RUN: -v -fsyntax-only \
+// RUN: /X \
+// RUN: -nobuiltininc \
+// RUN: -nostdlibinc \
+// RUN: -I%t/include/w \
+// RUN: -imsvc %t/include/z \
+// RUN: -I%t/include/x \
+// RUN: -imsvc %t/include/y \
+// RUN: -imsvc %t/include/x \
+// RUN: -I%t/include/w \
+// RUN: -imsvc %t/include/y \
+// RUN: -imsvc %t/include/z \
+// RUN: %t/test.c 2>&1 | FileCheck -DPWD=%t %t/test.c
+
+#--- test.c
+#include <a.h>
+#include <b.h>
+#include <c.h>
+
+// The expected behavior is that user search paths are ordered before system
+// search paths and that search paths that duplicate an earlier search path
+// (regardless of user/system concerns) are ignored.
+// CHECK: ignoring duplicate directory "[[PWD]]/include/w"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/x"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/y"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/z"
+// CHECK-NOT: as it is a non-system directory that duplicates a system directory
+// CHECK: #include <...> search starts here:
+// CHECK-NEXT: [[PWD]]/include/w
+// CHECK-NEXT: [[PWD]]/include/x
+// CHECK-NEXT: [[PWD]]/include/z
+// CHECK-NEXT: [[PWD]]/include/y
+// CHECK-NEXT: End of search list.
+
+#--- include/w/b.h
+#define INCLUDE_W_B_H
+#include_next <b.h>
+
+#--- include/w/c.h
+#define INCLUDE_W_C_H
+#include_next <c.h>
+
+#--- include/x/a.h
+#define INCLUDE_X_A_H
+#include_next <a.h>
+
+#--- include/x/b.h
+#if !defined(INCLUDE_W_B_H)
+#error 'include/w/b.h' should have been included before 'include/x/b.h'!
+#endif
+
+#--- include/x/c.h
+#define INCLUDE_X_C_H
+
+#--- include/y/a.h
+#if !defined(INCLUDE_X_A_H)
+#error 'include/x/a.h' should have been included before 'include/y/a.h'!
+#endif
+
+#--- include/z/c.h
+#include_next <c.h>
+#if !defined(INCLUDE_X_C_H)
+#error 'include/x/c.h' should have been included before 'include/z/c.h'!
+#endif
>From 7c8f4eae75536572af819a01cfd4891de8e0e7fb Mon Sep 17 00:00:00 2001
From: Tom Honermann <tom.honermann at intel.com>
Date: Mon, 9 Sep 2024 11:49:29 -0700
Subject: [PATCH 2/3] WIP
---
clang/include/clang/Driver/Options.td | 5 +-
clang/include/clang/Lex/HeaderSearchOptions.h | 4 +
clang/lib/Driver/Driver.cpp | 3 +-
clang/lib/Driver/ToolChains/Clang.cpp | 3 +-
clang/lib/Driver/ToolChains/MSVC.cpp | 2 +
clang/lib/Frontend/CompilerInvocation.cpp | 26 +-
clang/lib/Lex/InitHeaderSearch.cpp | 64 ++--
clang/test/Driver/header-search-duplicates.c | 116 ++++--
.../microsoft-header-search-duplicates.c | 341 +++++++++++++++---
llvm/include/llvm/Option/ArgList.h | 3 +-
llvm/lib/Option/ArgList.cpp | 5 +-
11 files changed, 450 insertions(+), 122 deletions(-)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 379e75b197cf96..6e5aba2804302c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4563,6 +4563,9 @@ def iapinotes_modules : JoinedOrSeparate<["-"], "iapinotes-modules">, Group<clan
def idirafter : JoinedOrSeparate<["-"], "idirafter">, Group<clang_i_Group>,
Visibility<[ClangOption, CC1Option]>,
HelpText<"Add directory to AFTER include search path">;
+def iexternal : JoinedOrSeparate<["-"], "iexternal">, Group<clang_i_Group>,
+ Visibility<[ClangOption, CC1Option]>,
+ HelpText<"Add directory to external include search path">, MetaVarName<"<directory>">;
def iframework : JoinedOrSeparate<["-"], "iframework">, Group<clang_i_Group>,
Visibility<[ClangOption, CC1Option]>,
HelpText<"Add directory to SYSTEM framework search path">;
@@ -8426,7 +8429,7 @@ def _SLASH_diagnostics_classic : CLFlag<"diagnostics:classic">,
def _SLASH_D : CLJoinedOrSeparate<"D", [CLOption, DXCOption]>,
HelpText<"Define macro">, MetaVarName<"<macro[=value]>">, Alias<D>;
def _SLASH_E : CLFlag<"E">, HelpText<"Preprocess to stdout">, Alias<E>;
-def _SLASH_external_COLON_I : CLJoinedOrSeparate<"external:I">, Alias<isystem>,
+def _SLASH_external_COLON_I : CLJoinedOrSeparate<"external:I">, Alias<iexternal>,
HelpText<"Add directory to include search path with warnings suppressed">,
MetaVarName<"<dir>">;
def _SLASH_fp_contract : CLFlag<"fp:contract">, HelpText<"">, Alias<ffp_contract>, AliasArgs<["on"]>;
diff --git a/clang/include/clang/Lex/HeaderSearchOptions.h b/clang/include/clang/Lex/HeaderSearchOptions.h
index 83a95e9ad90a7f..58893792cb4b77 100644
--- a/clang/include/clang/Lex/HeaderSearchOptions.h
+++ b/clang/include/clang/Lex/HeaderSearchOptions.h
@@ -38,6 +38,10 @@ enum IncludeDirGroup {
/// Like Angled, but marks header maps used when building frameworks.
IndexHeaderMap,
+ /// Like Angled, but marks system directories while retaining relative order
+ /// with user directories.
+ External,
+
/// Like Angled, but marks system directories.
System,
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 9878a9dad78d40..a8a5f0a4f445e2 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1298,7 +1298,8 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
// Check for missing include directories.
if (!Diags.isIgnored(diag::warn_missing_include_dirs, SourceLocation())) {
- for (auto IncludeDir : Args.getAllArgValues(options::OPT_I_Group)) {
+ for (auto IncludeDir :
+ Args.getAllArgValues(options::OPT_I_Group, options::OPT_iexternal)) {
if (!VFS->exists(IncludeDir))
Diag(diag::warn_missing_include_dirs) << IncludeDir;
}
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 3fc39296f44281..aca75494a848c6 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1185,7 +1185,8 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
Args.addAllArgs(CmdArgs,
{options::OPT_D, options::OPT_U, options::OPT_I_Group,
options::OPT_F, options::OPT_index_header_map,
- options::OPT_embed_dir_EQ});
+ options::OPT_embed_dir_EQ,
+ options::OPT_iexternal});
// Add -Wp, and -Xpreprocessor if using the preprocessor.
diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp
index 80799d1e715f07..b453e406e435f5 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -660,6 +660,8 @@ void MSVCToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
return false;
};
+ // FIXME: /external:env won't cause user paths already present in the
+ // FIXME: search path to be dropped like /external:I does.
// Add %INCLUDE%-like dirs via /external:env: flags.
for (const auto &Var :
DriverArgs.getAllArgValues(options::OPT__SLASH_external_env)) {
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index db7c791059a32e..4de73e0889fcdc 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3194,8 +3194,10 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
auto It = Opts.UserEntries.begin();
auto End = Opts.UserEntries.end();
- // Add -I..., -F..., and -index-header-map options in order.
- for (; It < End && Matches(*It, {frontend::IndexHeaderMap, frontend::Angled},
+ // Add the -I..., -F..., -index-header-map, and MSVC /external:I options
+ // in order.
+ for (; It < End && Matches(*It, {frontend::IndexHeaderMap, frontend::Angled,
+ frontend::External},
std::nullopt, true);
++It) {
OptSpecifier Opt = [It, Matches]() {
@@ -3207,13 +3209,15 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
return OPT_F;
if (Matches(*It, frontend::Angled, false, true))
return OPT_I;
+ if (Matches(*It, frontend::External, std::nullopt, true))
+ return OPT_iexternal;
llvm_unreachable("Unexpected HeaderSearchOptions::Entry.");
}();
if (It->Group == frontend::IndexHeaderMap)
GenerateArg(Consumer, OPT_index_header_map);
GenerateArg(Consumer, Opt, It->Path);
- };
+ }
// Note: some paths that came from "[-iprefix=xx] -iwithprefixbefore=yy" may
// have already been generated as "-I[xx]yy". If that's the case, their
@@ -3323,8 +3327,8 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
llvm::CachedHashString(MacroDef.split('=').first));
}
- // Add -I..., -F..., and -index-header-map options in order.
- bool IsIndexHeaderMap = false;
+ // Add the -I..., -F..., -index-header-map, and MSVC /external:I options
+ // options in order.
bool IsSysrootSpecified =
Args.hasArg(OPT__sysroot_EQ) || Args.hasArg(OPT_isysroot);
@@ -3343,15 +3347,19 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
return A->getValue();
};
- for (const auto *A : Args.filtered(OPT_I, OPT_F, OPT_index_header_map)) {
+ bool IsIndexHeaderMap = false;
+ for (const auto *A : Args.filtered(OPT_I, OPT_F, OPT_index_header_map,
+ OPT_iexternal)) {
+ frontend::IncludeDirGroup Group =
+ IsIndexHeaderMap ? frontend::IndexHeaderMap : frontend::Angled;
+
if (A->getOption().matches(OPT_index_header_map)) {
// -index-header-map applies to the next -I or -F.
IsIndexHeaderMap = true;
continue;
}
-
- frontend::IncludeDirGroup Group =
- IsIndexHeaderMap ? frontend::IndexHeaderMap : frontend::Angled;
+ if (A->getOption().matches(OPT_iexternal))
+ Group = frontend::External;
bool IsFramework = A->getOption().matches(OPT_F);
Opts.AddPath(PrefixHeaderPath(A, IsFramework), Group, IsFramework,
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp b/clang/lib/Lex/InitHeaderSearch.cpp
index 3f487f3a4c1c05..d4f3bb2247a01d 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -366,8 +366,8 @@ void InitHeaderSearch::AddDefaultIncludePaths(
}
/// If there are duplicate directory entries in the specified search list,
-/// remove the later (dead) ones. Returns the number of non-system headers
-/// removed, which is used to update NumAngled.
+/// identify and remove the ones to be ignored and issue a diagnostic.
+/// Returns the number of non-system search paths emoved.
static unsigned RemoveDuplicates(const LangOptions &Lang,
std::vector<DirectoryLookupInfo> &SearchList,
unsigned First, bool Verbose) {
@@ -377,41 +377,43 @@ static unsigned RemoveDuplicates(const LangOptions &Lang,
unsigned NonSystemRemoved = 0;
for (unsigned i = First; i != SearchList.size(); ++i) {
unsigned DirToRemove = i;
+ bool NonSystemDirRemoved = false;
const DirectoryLookup &CurEntry = SearchList[i].Lookup;
+ // If the current entry is for a previously unseen location, cache it and
+ // continue with the next entry.
if (CurEntry.isNormalDir()) {
- // If this isn't the first time we've seen this dir, remove it.
if (SeenDirs.insert(CurEntry.getDir()).second)
continue;
} else if (CurEntry.isFramework()) {
- // If this isn't the first time we've seen this framework dir, remove it.
if (SeenFrameworkDirs.insert(CurEntry.getFrameworkDir()).second)
continue;
} else {
assert(CurEntry.isHeaderMap() && "Not a headermap or normal dir?");
- // If this isn't the first time we've seen this headermap, remove it.
if (SeenHeaderMaps.insert(CurEntry.getHeaderMap()).second)
continue;
}
- // When not in MSVC compatibility mode, if we have a normal
- // #include dir/framework/headermap that is shadowed later in the chain by
- // a system include location, we actually want to ignore the user's request
- // and drop the user dir... keeping the system dir. This is weird, but
- // required to emulate GCC's search path correctly.
+ // Pruning of duplicate search locations is intended to emulate the behavior
+ // exhibited by GCC (by default) or MSVC (in Microsoft compatibility mode)
+ // to ensure that #include and #include_next directives produce the same
+ // results as these other compilers.
//
- // Since dupes of system dirs are rare, just rescan to find the original
- // that we're nuking instead of using a DenseMap.
- if (!Lang.MSVCCompat && CurEntry.getDirCharacteristic() != SrcMgr::C_User) {
- // Find the dir that this is the same of.
+ // GCC and MSVC both prune duplicate user search locations that follow a
+ // previous matching user search location. Both compilers also prune user
+ // search locations that are also present as system search locations
+ // regardless of the order in which they appear. The compilers differ with
+ // respect to pruning system search locations that duplicate a previous
+ // system search location; GCC preserves the first such occurences while
+ // MSVC preserves the last one.
+ if (CurEntry.getDirCharacteristic() != SrcMgr::C_User) {
+ // Find the matching search entry.
unsigned FirstDir;
- for (FirstDir = First;; ++FirstDir) {
- assert(FirstDir != i && "Didn't find dupe?");
-
+ for (FirstDir = First; FirstDir < i; ++FirstDir) {
const DirectoryLookup &SearchEntry = SearchList[FirstDir].Lookup;
- // If these are different lookup types, then they can't be the dupe.
+ // Different lookup types are not considered duplicate entries.
if (SearchEntry.getLookupType() != CurEntry.getLookupType())
continue;
@@ -428,21 +430,34 @@ static unsigned RemoveDuplicates(const LangOptions &Lang,
if (isSame)
break;
}
+ assert(FirstDir < i && "Expected duplicate search location not found");
- // If the first dir in the search path is a non-system dir, zap it
- // instead of the system one.
- if (SearchList[FirstDir].Lookup.getDirCharacteristic() == SrcMgr::C_User)
+ if (Lang.MSVCCompat) {
+ // In Microsoft compatibility mode, a later system search location entry
+ // suppresses a previous user or system search location.
DirToRemove = FirstDir;
+ if (SearchList[FirstDir].Lookup.getDirCharacteristic() ==
+ SrcMgr::C_User)
+ NonSystemDirRemoved = true;
+ } else {
+ // In GCC compatibility mode, a later system search location entry
+ // suppresses a previous user search location.
+ if (SearchList[FirstDir].Lookup.getDirCharacteristic() ==
+ SrcMgr::C_User) {
+ DirToRemove = FirstDir;
+ NonSystemDirRemoved = true;
+ }
+ }
}
if (Verbose) {
llvm::errs() << "ignoring duplicate directory \""
<< CurEntry.getName() << "\"\n";
- if (DirToRemove != i)
+ if (NonSystemDirRemoved)
llvm::errs() << " as it is a non-system directory that duplicates "
<< "a system directory\n";
}
- if (DirToRemove != i)
+ if (NonSystemDirRemoved)
++NonSystemRemoved;
// This is reached if the current entry is a duplicate. Remove the
@@ -490,7 +505,8 @@ void InitHeaderSearch::Realize(const LangOptions &Lang) {
unsigned NumQuoted = SearchList.size();
for (auto &Include : IncludePath)
- if (Include.Group == Angled || Include.Group == IndexHeaderMap)
+ if (Include.Group == Angled || Include.Group == IndexHeaderMap ||
+ Include.Group == External)
SearchList.push_back(Include);
RemoveDuplicates(Lang, SearchList, NumQuoted, Verbose);
diff --git a/clang/test/Driver/header-search-duplicates.c b/clang/test/Driver/header-search-duplicates.c
index ebd49453a1c10c..8ce50a8455ab40 100644
--- a/clang/test/Driver/header-search-duplicates.c
+++ b/clang/test/Driver/header-search-duplicates.c
@@ -1,6 +1,5 @@
-// Test that the gcc-like driver, when not in Microsoft compatibility mode,
-// emulates the gcc behavior of eliding a user header search path when the
-// same path is present as a system header search path.
+// Test that pruning of header search paths emulates GCC behavior when not in
+// Microsoft compatibility mode.
// See microsoft-header-search-duplicates.c for Microsoft compatible behavior.
// RUN: rm -rf %t
@@ -13,64 +12,123 @@
// RUN: -target x86_64-unknown-linux-gnu \
// RUN: -v -fsyntax-only \
// RUN: -nostdinc \
-// RUN: -I%t/include/w \
-// RUN: -isystem %t/include/z \
-// RUN: -I%t/include/x \
-// RUN: -isystem %t/include/y \
+// RUN: -isystem %t/include/S \
+// RUN: -isystem %t/include/w \
+// RUN: -I%t/include/v \
// RUN: -isystem %t/include/x \
-// RUN: -I%t/include/w \
+// RUN: -I%t/include/z \
// RUN: -isystem %t/include/y \
// RUN: -isystem %t/include/z \
+// RUN: -I%t/include/x \
+// RUN: -isystem %t/include/y \
+// RUN: -I%t/include/v \
+// RUN: -isystem %t/include/w \
+// RUN: -I%t/include/U \
// RUN: %t/test.c 2>&1 | FileCheck -DPWD=%t %t/test.c
#--- test.c
#include <a.h>
#include <b.h>
#include <c.h>
+#include <d.h>
+#include <e.h>
+#include <f.h>
+#include <g.h>
// The expected behavior is that user search paths are ordered before system
// search paths, that user search paths that duplicate a (later) system search
// path are ignored, and that search paths that duplicate an earlier search
// path of the same user/system kind are ignored.
-// CHECK: ignoring duplicate directory "[[PWD]]/include/w"
+// CHECK: ignoring duplicate directory "[[PWD]]/include/v"
// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/x"
// CHECK-NEXT: as it is a non-system directory that duplicates a system directory
-// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/y"
// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/z"
+// CHECK-NEXT: as it is a non-system directory that duplicates a system directory
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/y"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/w"
// CHECK: #include <...> search starts here:
+// CHECK-NEXT: [[PWD]]/include/v
+// CHECK-NEXT: [[PWD]]/include/U
+// CHECK-NEXT: [[PWD]]/include/S
// CHECK-NEXT: [[PWD]]/include/w
-// CHECK-NEXT: [[PWD]]/include/z
-// CHECK-NEXT: [[PWD]]/include/y
// CHECK-NEXT: [[PWD]]/include/x
+// CHECK-NEXT: [[PWD]]/include/y
+// CHECK-NEXT: [[PWD]]/include/z
// CHECK-NEXT: End of search list.
+#--- include/v/a.h
+
+#--- include/U/a.h
+#error 'include/U/a.h' should not have been included!
+
+#--- include/U/b.h
+
+#--- include/S/a.h
+#error 'include/U/a.h' should not have been included!
+
+#--- include/S/b.h
+#error 'include/S/b.h' should not have been included!
+
+#--- include/S/c.h
+
+#--- include/w/a.h
+#error 'include/w/a.h' should not have been included!
+
#--- include/w/b.h
-#define INCLUDE_W_B_H
-#include_next <b.h>
+#error 'include/w/b.h' should not have been included!
#--- include/w/c.h
-#define INCLUDE_W_C_H
-#include_next <c.h>
+#error 'include/w/c.h' should not have been included!
+
+#--- include/w/d.h
#--- include/x/a.h
-#if !defined(INCLUDE_Y_A_H)
-#error 'include/y/a.h' should have been included before 'include/x/a.h'!
-#endif
+#error 'include/x/a.h' should not have been included!
#--- include/x/b.h
-#if !defined(INCLUDE_W_B_H)
-#error 'include/w/b.h' should have been included before 'include/x/b.h'!
-#endif
+#error 'include/x/b.h' should not have been included!
#--- include/x/c.h
-#if !defined(INCLUDE_Z_C_H)
-#error 'include/z/c.h' should have been included before 'include/x/c.h'!
-#endif
+#error 'include/x/c.h' should not have been included!
+
+#--- include/x/d.h
+#error 'include/x/d.h' should not have been included!
+
+#--- include/x/e.h
#--- include/y/a.h
-#define INCLUDE_Y_A_H
-#include_next <a.h>
+#error 'include/y/a.h' should not have been included!
+
+#--- include/y/b.h
+#error 'include/y/b.h' should not have been included!
+
+#--- include/y/c.h
+#error 'include/y/c.h' should not have been included!
+
+#--- include/y/d.h
+#error 'include/y/d.h' should not have been included!
+
+#--- include/y/e.h
+#error 'include/y/e.h' should not have been included!
+
+#--- include/y/f.h
+
+#--- include/z/a.h
+#error 'include/z/a.h' should not have been included!
+
+#--- include/z/b.h
+#error 'include/z/b.h' should not have been included!
#--- include/z/c.h
-#define INCLUDE_Z_C_H
-#include_next <c.h>
+#error 'include/z/c.h' should not have been included!
+
+#--- include/z/d.h
+#error 'include/z/d.h' should not have been included!
+
+#--- include/z/e.h
+#error 'include/z/e.h' should not have been included!
+
+#--- include/z/f.h
+#error 'include/z/f.h' should not have been included!
+
+#--- include/z/g.h
diff --git a/clang/test/Driver/microsoft-header-search-duplicates.c b/clang/test/Driver/microsoft-header-search-duplicates.c
index 0ab3e88b951c5c..3a01df8e90e4f4 100644
--- a/clang/test/Driver/microsoft-header-search-duplicates.c
+++ b/clang/test/Driver/microsoft-header-search-duplicates.c
@@ -1,7 +1,6 @@
-// Test that the cl-like driver and the gcc-like driver, when in Microsoft
-// compatibility mode, retain user header search paths that are duplicated in
-// system header search paths.
-// See header-search-duplicates.c for gcc compatible behavior.
+// Test that pruning of header search paths emulates MSVC behavior when in
+// Microsoft compatibility mode.
+// See header-search-duplicates.c for GCC compatible behavior.
// RUN: rm -rf %t
// RUN: split-file %s %t
@@ -9,89 +8,323 @@
// Test the clang driver with a Windows target that implicitly enables the
// -fms-compatibility option. The -nostdinc option is used to suppress default
// search paths to ease testing.
-// RUN: %clang \
-// RUN: -target x86_64-pc-windows \
-// RUN: -v -fsyntax-only \
-// RUN: -nostdinc \
-// RUN: -I%t/include/w \
-// RUN: -isystem %t/include/z \
-// RUN: -I%t/include/x \
-// RUN: -isystem %t/include/y \
-// RUN: -isystem %t/include/x \
-// RUN: -I%t/include/w \
-// RUN: -isystem %t/include/y \
-// RUN: -isystem %t/include/z \
-// RUN: %t/test.c 2>&1 | FileCheck -DPWD=%t %t/test.c
+// XUN: env INCLUDE="%t/include/p;%t/include/o" \
+// XUN: env EXTRA_INCLUDE="%t/include/t;%t/include/q;%t/include/r" \
+// XUN %clang \
+// XUN: -target x86_64-pc-windows \
+// XUN: -v -fsyntax-only \
+// XUN: -nostdinc \
+// XUN: -isystem %t/include/s \
+// XUN: -isystem %t/include/w \
+// XUN: -I%t/include/v \
+// XUN: -isystem %t/include/x \
+// XUN: -I%t/include/z \
+// XUN: -isystem %t/include/y \
+// XUN: -I%t/include/r \
+// XUN: -external:env:EXTRA_INCLUDE \
+// XUN: -I%t/include/t \
+// XUN: -isystem %t/include/z \
+// XUN: -I%t/include/o \
+// XUN: -I%t/include/x \
+// XUN: -isystem %t/include/y \
+// XUN: -I%t/include/v \
+// XUN: -isystem %t/include/w \
+// XUN: -I%t/include/u \
+// XUN: %t/test.c 2>&1 | FileCheck -DPWD=%t %t/test.c
// Test the clang-cl driver with a Windows target that implicitly enables the
// -fms-compatibility option. The /X option is used instead of -nostdinc
// because the latter option suppresses all system include paths including
-// those specified by -imsvc. The -nobuiltininc option is uesd to suppress
+// those specified by /external:I. The -nobuiltininc option is uesd to suppress
// the Clang resource directory. The -nostdlibinc option is used to suppress
// search paths for the Windows SDK, CRT, MFC, ATL, etc...
+// RUN: env INCLUDE="%t/include/p;%t/include/o" \
+// RUN: env EXTRA_INCLUDE="%t/include/t;%t/include/q;%t/include/r" \
// RUN: %clang_cl \
// RUN: -target x86_64-pc-windows \
// RUN: -v -fsyntax-only \
-// RUN: /X \
// RUN: -nobuiltininc \
// RUN: -nostdlibinc \
-// RUN: -I%t/include/w \
-// RUN: -imsvc %t/include/z \
-// RUN: -I%t/include/x \
-// RUN: -imsvc %t/include/y \
-// RUN: -imsvc %t/include/x \
-// RUN: -I%t/include/w \
-// RUN: -imsvc %t/include/y \
-// RUN: -imsvc %t/include/z \
+// RUN: /external:I %t/include/s \
+// RUN: /external:I %t/include/w \
+// RUN: /I%t/include/v \
+// RUN: /external:I %t/include/x \
+// RUN: /I%t/include/z \
+// RUN: /external:I %t/include/y \
+// RUN: /I%t/include/r \
+// RUN: /external:env:EXTRA_INCLUDE \
+// RUN: /I%t/include/t \
+// RUN: /external:I %t/include/z \
+// RUN: /I%t/include/o \
+// RUN: /I%t/include/x \
+// RUN: /external:I %t/include/y \
+// RUN: /I%t/include/v \
+// RUN: /external:I %t/include/w \
+// RUN: /I%t/include/u \
// RUN: %t/test.c 2>&1 | FileCheck -DPWD=%t %t/test.c
#--- test.c
#include <a.h>
#include <b.h>
#include <c.h>
+#include <d.h>
+#include <e.h>
+#include <f.h>
+#include <g.h>
+#include <h.h>
+#include <i.h>
+#include <j.h>
+#include <k.h>
+#include <l.h>
-// The expected behavior is that user search paths are ordered before system
-// search paths and that search paths that duplicate an earlier search path
-// (regardless of user/system concerns) are ignored.
-// CHECK: ignoring duplicate directory "[[PWD]]/include/w"
+// The expected behavior is that user search paths that duplicate a system search
+// path are ignored, that user search paths that duplicate a previous user
+// search path are ignored, and that system search search paths that duplicate
+// a later system search path are ignored.
+// CHECK: ignoring duplicate directory "[[PWD]]/include/v"
// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/x"
-// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/y"
+// CHECK-NEXT: as it is a non-system directory that duplicates a system directory
// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/z"
-// CHECK-NOT: as it is a non-system directory that duplicates a system directory
+// CHECK-NEXT: as it is a non-system directory that duplicates a system directory
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/t"
+// CHECK-NEXT: as it is a non-system directory that duplicates a system directory
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/r"
+// CHECK-NEXT: as it is a non-system directory that duplicates a system directory
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/y"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/w"
// CHECK: #include <...> search starts here:
-// CHECK-NEXT: [[PWD]]/include/w
+// CHECK-NEXT: [[PWD]]/include/s
+// CHECK-NEXT: [[PWD]]/include/v
// CHECK-NEXT: [[PWD]]/include/x
+// CHECK-NEXT: [[PWD]]/include/r
+// CHECK-NEXT: [[PWD]]/include/t
// CHECK-NEXT: [[PWD]]/include/z
+// CHECK-NEXT: [[PWD]]/include/o
// CHECK-NEXT: [[PWD]]/include/y
+// CHECK-NEXT: [[PWD]]/include/w
+// CHECK-NEXT: [[PWD]]/include/u
+// CHECK-NEXT: [[PWD]]/include/q
+// CHECK-NEXT: [[PWD]]/include/p
// CHECK-NEXT: End of search list.
-#--- include/w/b.h
-#define INCLUDE_W_B_H
-#include_next <b.h>
+#--- include/s/a.h
-#--- include/w/c.h
-#define INCLUDE_W_C_H
-#include_next <c.h>
+#--- include/v/a.h
+#error 'include/v/a.h' should not have been included!
+
+#--- include/v/b.h
#--- include/x/a.h
-#define INCLUDE_X_A_H
-#include_next <a.h>
+#error 'include/x/a.h' should not have been included!
#--- include/x/b.h
-#if !defined(INCLUDE_W_B_H)
-#error 'include/w/b.h' should have been included before 'include/x/b.h'!
-#endif
+#error 'include/x/b.h' should not have been included!
#--- include/x/c.h
-#define INCLUDE_X_C_H
-#--- include/y/a.h
-#if !defined(INCLUDE_X_A_H)
-#error 'include/x/a.h' should have been included before 'include/y/a.h'!
-#endif
+#--- include/r/a.h
+#error 'include/r/a.h' should not have been included!
+
+#--- include/r/b.h
+#error 'include/r/b.h' should not have been included!
+
+#--- include/r/c.h
+#error 'include/r/c.h' should not have been included!
+
+#--- include/r/d.h
+
+#--- include/t/a.h
+#error 'include/t/a.h' should not have been included!
+
+#--- include/t/b.h
+#error 'include/t/b.h' should not have been included!
+
+#--- include/t/c.h
+#error 'include/t/c.h' should not have been included!
+
+#--- include/t/d.h
+#error 'include/t/cdh' should not have been included!
+
+#--- include/t/e.h
+
+#--- include/z/a.h
+#error 'include/z/a.h' should not have been included!
+
+#--- include/z/b.h
+#error 'include/z/b.h' should not have been included!
#--- include/z/c.h
-#include_next <c.h>
-#if !defined(INCLUDE_X_C_H)
-#error 'include/x/c.h' should have been included before 'include/z/c.h'!
-#endif
+#error 'include/z/c.h' should not have been included!
+
+#--- include/z/d.h
+#error 'include/z/d.h' should not have been included!
+
+#--- include/z/e.h
+#error 'include/z/e.h' should not have been included!
+
+#--- include/z/f.h
+
+#--- include/o/a.h
+#error 'include/o/a.h' should not have been included!
+
+#--- include/o/b.h
+#error 'include/o/b.h' should not have been included!
+
+#--- include/o/c.h
+#error 'include/o/c.h' should not have been included!
+
+#--- include/o/d.h
+#error 'include/o/d.h' should not have been included!
+
+#--- include/o/e.h
+#error 'include/o/e.h' should not have been included!
+
+#--- include/o/f.h
+#error 'include/o/f.h' should not have been included!
+
+#--- include/o/g.h
+
+#--- include/y/a.h
+#error 'include/y/a.h' should not have been included!
+
+#--- include/y/b.h
+#error 'include/y/b.h' should not have been included!
+
+#--- include/y/c.h
+#error 'include/y/c.h' should not have been included!
+
+#--- include/y/d.h
+#error 'include/y/d.h' should not have been included!
+
+#--- include/y/e.h
+#error 'include/y/e.h' should not have been included!
+
+#--- include/y/f.h
+#error 'include/y/f.h' should not have been included!
+
+#--- include/y/g.h
+#error 'include/y/g.h' should not have been included!
+
+#--- include/y/h.h
+
+#--- include/w/a.h
+#error 'include/w/a.h' should not have been included!
+
+#--- include/w/b.h
+#error 'include/w/b.h' should not have been included!
+
+#--- include/w/c.h
+#error 'include/w/c.h' should not have been included!
+
+#--- include/w/d.h
+#error 'include/w/d.h' should not have been included!
+
+#--- include/w/e.h
+#error 'include/w/e.h' should not have been included!
+
+#--- include/w/f.h
+#error 'include/w/f.h' should not have been included!
+
+#--- include/w/g.h
+#error 'include/w/g.h' should not have been included!
+
+#--- include/w/h.h
+#error 'include/w/h.h' should not have been included!
+
+#--- include/w/i.h
+
+#--- include/u/a.h
+#error 'include/u/a.h' should not have been included!
+
+#--- include/u/b.h
+#error 'include/u/b.h' should not have been included!
+
+#--- include/u/c.h
+#error 'include/u/c.h' should not have been included!
+
+#--- include/u/d.h
+#error 'include/u/d.h' should not have been included!
+
+#--- include/u/e.h
+#error 'include/u/e.h' should not have been included!
+
+#--- include/u/f.h
+#error 'include/u/f.h' should not have been included!
+
+#--- include/u/g.h
+#error 'include/u/g.h' should not have been included!
+
+#--- include/u/h.h
+#error 'include/u/h.h' should not have been included!
+
+#--- include/u/i.h
+#error 'include/u/i.h' should not have been included!
+
+#--- include/u/j.h
+
+#--- include/q/a.h
+#error 'include/q/a.h' should not have been included!
+
+#--- include/q/b.h
+#error 'include/q/b.h' should not have been included!
+
+#--- include/q/c.h
+#error 'include/q/c.h' should not have been included!
+
+#--- include/q/d.h
+#error 'include/q/d.h' should not have been included!
+
+#--- include/q/e.h
+#error 'include/q/e.h' should not have been included!
+
+#--- include/q/f.h
+#error 'include/q/f.h' should not have been included!
+
+#--- include/q/g.h
+#error 'include/q/g.h' should not have been included!
+
+#--- include/q/h.h
+#error 'include/q/h.h' should not have been included!
+
+#--- include/q/i.h
+#error 'include/q/i.h' should not have been included!
+
+#--- include/q/j.h
+#error 'include/q/j.h' should not have been included!
+
+#--- include/q/k.h
+
+#--- include/p/a.h
+#error 'include/p/a.h' should not have been included!
+
+#--- include/p/b.h
+#error 'include/p/b.h' should not have been included!
+
+#--- include/p/c.h
+#error 'include/p/c.h' should not have been included!
+
+#--- include/p/d.h
+#error 'include/p/d.h' should not have been included!
+
+#--- include/p/e.h
+#error 'include/p/e.h' should not have been included!
+
+#--- include/p/f.h
+#error 'include/p/f.h' should not have been included!
+
+#--- include/p/g.h
+#error 'include/p/g.h' should not have been included!
+
+#--- include/p/h.h
+#error 'include/p/h.h' should not have been included!
+
+#--- include/p/i.h
+#error 'include/p/i.h' should not have been included!
+
+#--- include/p/j.h
+#error 'include/p/j.h' should not have been included!
+
+#--- include/p/k.h
+#error 'include/p/k.h' should not have been included!
+
+#--- include/p/l.h
diff --git a/llvm/include/llvm/Option/ArgList.h b/llvm/include/llvm/Option/ArgList.h
index 09812f976d0166..072a7a371d7a26 100644
--- a/llvm/include/llvm/Option/ArgList.h
+++ b/llvm/include/llvm/Option/ArgList.h
@@ -288,7 +288,8 @@ class ArgList {
/// getAllArgValues - Get the values of all instances of the given argument
/// as strings.
- std::vector<std::string> getAllArgValues(OptSpecifier Id) const;
+ std::vector<std::string>
+ getAllArgValues(OptSpecifier Id0, OptSpecifier Id1 = 0U) const;
/// @}
/// @name Translation Utilities
diff --git a/llvm/lib/Option/ArgList.cpp b/llvm/lib/Option/ArgList.cpp
index 6e164150d2e5e9..345754dec9d7b9 100644
--- a/llvm/lib/Option/ArgList.cpp
+++ b/llvm/lib/Option/ArgList.cpp
@@ -95,9 +95,10 @@ StringRef ArgList::getLastArgValue(OptSpecifier Id, StringRef Default) const {
return Default;
}
-std::vector<std::string> ArgList::getAllArgValues(OptSpecifier Id) const {
+std::vector<std::string>
+ArgList::getAllArgValues(OptSpecifier Id0, OptSpecifier Id1) const {
SmallVector<const char *, 16> Values;
- AddAllArgValues(Values, Id);
+ AddAllArgValues(Values, Id0, Id1);
return std::vector<std::string>(Values.begin(), Values.end());
}
>From 70e165e1c8c01931ce5eb4647e408fee05cd5d67 Mon Sep 17 00:00:00 2001
From: Tom Honermann <tom.honermann at intel.com>
Date: Fri, 18 Oct 2024 20:56:33 -0700
Subject: [PATCH 3/3] WIP to be squashed.
This passes all tests, but:
- Cleanup is needed; comments and doc are not in sync with code.
- There are a number of FIXMEs.
- Should env vars be expanded by the driver? Or ok to pass to cc1?
- New tests need to be refined to not be a single monolithic impossible
to understand monstrosity.
---
clang/include/clang/Driver/Options.td | 16 +++++--
clang/include/clang/Driver/ToolChain.h | 3 ++
clang/include/clang/Lex/HeaderSearchOptions.h | 16 ++++++-
clang/lib/Driver/Driver.cpp | 3 +-
clang/lib/Driver/ToolChain.cpp | 11 +++++
clang/lib/Driver/ToolChains/Clang.cpp | 3 +-
clang/lib/Driver/ToolChains/MSVC.cpp | 45 ++++++++---------
clang/lib/Frontend/CompilerInvocation.cpp | 29 +++++++++--
clang/lib/Lex/InitHeaderSearch.cpp | 42 ++++++++++++----
clang/test/Driver/cl-include.c | 32 +++++--------
clang/test/Driver/cl-options.c | 6 ++-
.../microsoft-header-search-duplicates.c | 48 +++++++++++--------
llvm/include/llvm/Option/ArgList.h | 7 ++-
llvm/lib/Option/ArgList.cpp | 7 ---
14 files changed, 170 insertions(+), 98 deletions(-)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 6e5aba2804302c..ec1f9199fff0de 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4563,9 +4563,15 @@ def iapinotes_modules : JoinedOrSeparate<["-"], "iapinotes-modules">, Group<clan
def idirafter : JoinedOrSeparate<["-"], "idirafter">, Group<clang_i_Group>,
Visibility<[ClangOption, CC1Option]>,
HelpText<"Add directory to AFTER include search path">;
-def iexternal : JoinedOrSeparate<["-"], "iexternal">, Group<clang_i_Group>,
+def iexternal : JoinedOrSeparate<["-"], "iexternal">, Group<I_Group>,
Visibility<[ClangOption, CC1Option]>,
- HelpText<"Add directory to external include search path">, MetaVarName<"<directory>">;
+ HelpText<"Add directory to include search path with warnings suppressed">, MetaVarName<"<dir>">;
+def iexternal_after : JoinedOrSeparate<["-"], "iexternal-after">, Group<clang_i_Group>,
+ Visibility<[ClangOption, CC1Option]>,
+ HelpText<"Add directory to include search path with warnings suppressed">, MetaVarName<"<dir>">;
+def iexternal_env_EQ : Joined<["-"], "iexternal-env=">, Group<clang_i_Group>,
+ Visibility<[ClangOption, CC1Option]>,
+ HelpText<"Add dirs in env var <var> to include search path with warnings suppressed">, MetaVarName<"<var>">;
def iframework : JoinedOrSeparate<["-"], "iframework">, Group<clang_i_Group>,
Visibility<[ClangOption, CC1Option]>,
HelpText<"Add directory to SYSTEM framework search path">;
@@ -8432,6 +8438,9 @@ def _SLASH_E : CLFlag<"E">, HelpText<"Preprocess to stdout">, Alias<E>;
def _SLASH_external_COLON_I : CLJoinedOrSeparate<"external:I">, Alias<iexternal>,
HelpText<"Add directory to include search path with warnings suppressed">,
MetaVarName<"<dir>">;
+def _SLASH_external_env : CLJoined<"external:env:">, Alias<iexternal_env_EQ>,
+ HelpText<"Add dirs in env var <var> to include search path with warnings suppressed">,
+ MetaVarName<"<var>">;
def _SLASH_fp_contract : CLFlag<"fp:contract">, HelpText<"">, Alias<ffp_contract>, AliasArgs<["on"]>;
def _SLASH_fp_except : CLFlag<"fp:except">, HelpText<"">, Alias<ffp_exception_behavior_EQ>, AliasArgs<["strict"]>;
def _SLASH_fp_except_ : CLFlag<"fp:except-">, HelpText<"">, Alias<ffp_exception_behavior_EQ>, AliasArgs<["ignore"]>;
@@ -8661,9 +8670,6 @@ def _SLASH_volatile_Group : OptionGroup<"</volatile group>">,
def _SLASH_EH : CLJoined<"EH">, HelpText<"Set exception handling model">;
def _SLASH_EP : CLFlag<"EP">,
HelpText<"Disable linemarker output and preprocess to stdout">;
-def _SLASH_external_env : CLJoined<"external:env:">,
- HelpText<"Add dirs in env var <var> to include search path with warnings suppressed">,
- MetaVarName<"<var>">;
def _SLASH_FA : CLJoined<"FA">,
HelpText<"Output assembly code file during compilation">;
def _SLASH_Fa : CLJoined<"Fa">,
diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h
index 5347e29be91439..bcac7e17eda881 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -239,6 +239,9 @@ class ToolChain {
static void addSystemIncludes(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args,
ArrayRef<StringRef> Paths);
+ static void addExternalAfterIncludes(const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args,
+ ArrayRef<StringRef> Paths);
static std::string concat(StringRef Path, const Twine &A, const Twine &B = "",
const Twine &C = "", const Twine &D = "");
diff --git a/clang/include/clang/Lex/HeaderSearchOptions.h b/clang/include/clang/Lex/HeaderSearchOptions.h
index 58893792cb4b77..924a41e7c6fc64 100644
--- a/clang/include/clang/Lex/HeaderSearchOptions.h
+++ b/clang/include/clang/Lex/HeaderSearchOptions.h
@@ -38,10 +38,22 @@ enum IncludeDirGroup {
/// Like Angled, but marks header maps used when building frameworks.
IndexHeaderMap,
- /// Like Angled, but marks system directories while retaining relative order
- /// with user directories.
+ /// Like Angled, but marks system-like directories while retaining relative
+ /// order with user directories.
+ // FIXME: External is intended to match the semantics of the MSVC /external:I
+ // FIXME: option. MSVC handles such paths similarly to system paths, but
+ // FIXME: allows for separate specification of diagnostic level via the
+ // FIXME: /external:Wn option. That option does not affect the diagnostic
+ // FIXME: level used for system paths (those present in the INCLUDE env var).
+ // FIXME: Supporting the diagnostic level controls will likely require adding
+ // FIXME: an additional C_External enumerator to SrcMgr::CharacteristicKind.
External,
+ // Like External, but searched after other external directories but before
+ // system directories. This is intended to match the semantics of the MSVC
+ // /external:env option.
+ ExternalAfter,
+
/// Like Angled, but marks system directories.
System,
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index a8a5f0a4f445e2..af4104e73cd9de 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1299,7 +1299,8 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
// Check for missing include directories.
if (!Diags.isIgnored(diag::warn_missing_include_dirs, SourceLocation())) {
for (auto IncludeDir :
- Args.getAllArgValues(options::OPT_I_Group, options::OPT_iexternal)) {
+ Args.getAllArgValues(options::OPT_I_Group,
+ options::OPT_iexternal_after)) {
if (!VFS->exists(IncludeDir))
Diag(diag::warn_missing_include_dirs) << IncludeDir;
}
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 4df31770950858..3e83046189504d 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1265,6 +1265,17 @@ void ToolChain::addExternCSystemIncludeIfExists(const ArgList &DriverArgs,
}
}
+/// Utility function to add a list of directories to the end of the external
+/// include path list for CC1.
+/*static*/ void ToolChain::addExternalAfterIncludes(const ArgList &DriverArgs,
+ ArgStringList &CC1Args,
+ ArrayRef<StringRef> Paths) {
+ for (const auto &Path : Paths) {
+ CC1Args.push_back("-iexternal-after");
+ CC1Args.push_back(DriverArgs.MakeArgString(Path));
+ }
+}
+
/*static*/ std::string ToolChain::concat(StringRef Path, const Twine &A,
const Twine &B, const Twine &C,
const Twine &D) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index aca75494a848c6..3fc39296f44281 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1185,8 +1185,7 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
Args.addAllArgs(CmdArgs,
{options::OPT_D, options::OPT_U, options::OPT_I_Group,
options::OPT_F, options::OPT_index_header_map,
- options::OPT_embed_dir_EQ,
- options::OPT_iexternal});
+ options::OPT_embed_dir_EQ});
// Add -Wp, and -Xpreprocessor if using the preprocessor.
diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp
index b453e406e435f5..410a61502629eb 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -644,30 +644,11 @@ void MSVCToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
"include");
}
+ // FIXME: Why does the -imsvc option exist?
// Add %INCLUDE%-like directories from the -imsvc flag.
for (const auto &Path : DriverArgs.getAllArgValues(options::OPT__SLASH_imsvc))
addSystemInclude(DriverArgs, CC1Args, Path);
- auto AddSystemIncludesFromEnv = [&](StringRef Var) -> bool {
- if (auto Val = llvm::sys::Process::GetEnv(Var)) {
- SmallVector<StringRef, 8> Dirs;
- StringRef(*Val).split(Dirs, ";", /*MaxSplit=*/-1, /*KeepEmpty=*/false);
- if (!Dirs.empty()) {
- addSystemIncludes(DriverArgs, CC1Args, Dirs);
- return true;
- }
- }
- return false;
- };
-
- // FIXME: /external:env won't cause user paths already present in the
- // FIXME: search path to be dropped like /external:I does.
- // Add %INCLUDE%-like dirs via /external:env: flags.
- for (const auto &Var :
- DriverArgs.getAllArgValues(options::OPT__SLASH_external_env)) {
- AddSystemIncludesFromEnv(Var);
- }
-
// Add DIA SDK include if requested.
if (const Arg *A = DriverArgs.getLastArg(options::OPT__SLASH_diasdkdir,
options::OPT__SLASH_winsysroot)) {
@@ -684,14 +665,26 @@ void MSVCToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
if (DriverArgs.hasArg(options::OPT_nostdlibinc))
return;
- // Honor %INCLUDE% and %EXTERNAL_INCLUDE%. It should have essential search
- // paths set by vcvarsall.bat. Skip if the user expressly set a vctoolsdir.
+ // Add system paths from the %INCLUDE% environment variable if neither a
+ // vctoolsdir or winsysroot directory has been explicitly specified.
+ // If any paths are present in %INCLUDE%, skip adding additional system
+ // directories.
+ // FIXME: Paths specified by the INCLUDE environment variable should be
+ // FIXME: handled as system paths distinct from external paths added by
+ // FIXME: /external:I or /external:env since /external:Wn options do not
+ // FIXME: apply to these paths. These paths are currently mapped to
+ // FIXME: -iexternal-after to avoid include path ordering issues when
+ // FIXME: duplicate paths are present.
if (!DriverArgs.getLastArg(options::OPT__SLASH_vctoolsdir,
options::OPT__SLASH_winsysroot)) {
- bool Found = AddSystemIncludesFromEnv("INCLUDE");
- Found |= AddSystemIncludesFromEnv("EXTERNAL_INCLUDE");
- if (Found)
- return;
+ if (auto Val = llvm::sys::Process::GetEnv("INCLUDE")) {
+ SmallVector<StringRef, 8> Dirs;
+ StringRef(*Val).split(Dirs, ";", /*MaxSplit=*/-1, /*KeepEmpty=*/false);
+ if (!Dirs.empty()) {
+ addExternalAfterIncludes(DriverArgs, CC1Args, Dirs);
+ return;
+ }
+ }
}
// When built with access to the proper Windows APIs, try to actually find
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 4de73e0889fcdc..a2c3cfa151b617 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3209,7 +3209,7 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
return OPT_F;
if (Matches(*It, frontend::Angled, false, true))
return OPT_I;
- if (Matches(*It, frontend::External, std::nullopt, true))
+ if (Matches(*It, frontend::External, false, true))
return OPT_iexternal;
llvm_unreachable("Unexpected HeaderSearchOptions::Entry.");
}();
@@ -3219,6 +3219,11 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
GenerateArg(Consumer, Opt, It->Path);
}
+ // Add the paths for the MSVC /external:env and -iexternal-after options
+ // in order.
+ for (; It < End && Matches(*It, {frontend::ExternalAfter}, false, true); ++It)
+ GenerateArg(Consumer, OPT_iexternal_after, It->Path);
+
// Note: some paths that came from "[-iprefix=xx] -iwithprefixbefore=yy" may
// have already been generated as "-I[xx]yy". If that's the case, their
// position on command line was such that this has no semantic impact on
@@ -3327,8 +3332,6 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
llvm::CachedHashString(MacroDef.split('=').first));
}
- // Add the -I..., -F..., -index-header-map, and MSVC /external:I options
- // options in order.
bool IsSysrootSpecified =
Args.hasArg(OPT__sysroot_EQ) || Args.hasArg(OPT_isysroot);
@@ -3347,6 +3350,8 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
return A->getValue();
};
+ // Add the -I..., -F..., -index-header-map, and MSVC /external:I options
+ // options in order.
bool IsIndexHeaderMap = false;
for (const auto *A : Args.filtered(OPT_I, OPT_F, OPT_index_header_map,
OPT_iexternal)) {
@@ -3367,6 +3372,24 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
IsIndexHeaderMap = false;
}
+ // Add the MSVC /external:env and -iexternal-after options in order.
+ for (const auto *A :
+ Args.filtered(OPT_iexternal_after, OPT_iexternal_env_EQ)) {
+ if (A->getOption().matches(OPT_iexternal_after))
+ Opts.AddPath(A->getValue(), frontend::ExternalAfter,
+ /*IsFramework=*/false, /*IgnoreSysRoot=*/true);
+ else {
+ if (auto Val = llvm::sys::Process::GetEnv(A->getValue())) {
+ SmallVector<StringRef, 8> Dirs;
+ StringRef(*Val).split(Dirs, ";", /*MaxSplit=*/-1, /*KeepEmpty=*/false);
+ for (const auto &Dir : Dirs) {
+ Opts.AddPath(Dir, frontend::ExternalAfter, /*IsFramework=*/false,
+ /*IgnoreSysRoot=*/true);
+ }
+ }
+ }
+ }
+
// Add -iprefix/-iwithprefix/-iwithprefixbefore options.
StringRef Prefix = ""; // FIXME: This isn't the correct default prefix.
for (const auto *A :
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp b/clang/lib/Lex/InitHeaderSearch.cpp
index d4f3bb2247a01d..ebe2b30d80ac15 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -154,6 +154,8 @@ bool InitHeaderSearch::AddUnmappedPath(const Twine &Path, IncludeDirGroup Group,
} else if (Group == ExternCSystem) {
Type = SrcMgr::C_ExternCSystem;
} else {
+ // Group in External, ExternalAfter, System, (Obj)C(XX)System, After.
+ // FIXME: Assert that the group is known?
Type = SrcMgr::C_System;
}
@@ -367,10 +369,11 @@ void InitHeaderSearch::AddDefaultIncludePaths(
/// If there are duplicate directory entries in the specified search list,
/// identify and remove the ones to be ignored and issue a diagnostic.
-/// Returns the number of non-system search paths emoved.
+/// Returns the number of non-system search paths removed.
static unsigned RemoveDuplicates(const LangOptions &Lang,
std::vector<DirectoryLookupInfo> &SearchList,
- unsigned First, bool Verbose) {
+ unsigned First, bool Verbose,
+ bool RemovePreviousAllowed = true) {
llvm::SmallPtrSet<const DirectoryEntry *, 8> SeenDirs;
llvm::SmallPtrSet<const DirectoryEntry *, 8> SeenFrameworkDirs;
llvm::SmallPtrSet<const HeaderMap *, 8> SeenHeaderMaps;
@@ -405,9 +408,10 @@ static unsigned RemoveDuplicates(const LangOptions &Lang,
// search locations that are also present as system search locations
// regardless of the order in which they appear. The compilers differ with
// respect to pruning system search locations that duplicate a previous
- // system search location; GCC preserves the first such occurences while
+ // system search location; GCC preserves the first such occurence while
// MSVC preserves the last one.
- if (CurEntry.getDirCharacteristic() != SrcMgr::C_User) {
+ if (RemovePreviousAllowed &&
+ CurEntry.getDirCharacteristic() != SrcMgr::C_User) {
// Find the matching search entry.
unsigned FirstDir;
for (FirstDir = First; FirstDir < i; ++FirstDir) {
@@ -495,23 +499,40 @@ void InitHeaderSearch::Realize(const LangOptions &Lang) {
std::vector<DirectoryLookupInfo> SearchList;
SearchList.reserve(IncludePath.size());
- // Quoted arguments go first.
+ // Add search paths for quoted inclusion first.
for (auto &Include : IncludePath)
if (Include.Group == Quoted)
SearchList.push_back(Include);
- // Deduplicate and remember index.
+ // Remove duplicate search paths within the quoted inclusion list.
RemoveDuplicates(Lang, SearchList, 0, Verbose);
unsigned NumQuoted = SearchList.size();
+ // FIXME: GCC Ignore the last path for quoted inclusion when it matches the
+ // FIXME: first path for angled inclusion. This is observable when using
+ // FIXME: #include_next.
+
+ // Add search paths for angled inclusion next.
for (auto &Include : IncludePath)
if (Include.Group == Angled || Include.Group == IndexHeaderMap ||
Include.Group == External)
SearchList.push_back(Include);
+ // Remove duplicate search paths within the angled inclusion list.
+ // This may leave paths duplicated across the quoted and angled inclusion
+ // sections.
RemoveDuplicates(Lang, SearchList, NumQuoted, Verbose);
unsigned NumAngled = SearchList.size();
+ // Add search paths for external-after inclusion next and remove duplicates
+ // within them.
+ for (auto &Include : IncludePath)
+ if (Include.Group == ExternalAfter)
+ SearchList.push_back(Include);
+ RemoveDuplicates(Lang, SearchList, NumQuoted, Verbose,
+ /*RemovePreviousAllowed*/ false);
+
+ // Add search paths for language dependent system paths next.
for (auto &Include : IncludePath)
if (Include.Group == System || Include.Group == ExternCSystem ||
(!Lang.ObjC && !Lang.CPlusPlus && Include.Group == CSystem) ||
@@ -520,14 +541,15 @@ void InitHeaderSearch::Realize(const LangOptions &Lang) {
(Lang.ObjC && !Lang.CPlusPlus && Include.Group == ObjCSystem) ||
(Lang.ObjC && Lang.CPlusPlus && Include.Group == ObjCXXSystem))
SearchList.push_back(Include);
-
+ // Add search paths for system paths to be searched after other system paths.
for (auto &Include : IncludePath)
if (Include.Group == After)
SearchList.push_back(Include);
- // Remove duplicates across both the Angled and System directories. GCC does
- // this and failing to remove duplicates across these two groups breaks
- // #include_next.
+ // Remove duplicate search paths across both the angled inclusion list and
+ // the system search paths. This duplicate removal is necessary to ensure
+ // that header lookup in #include_next directives doesn't resolve to the
+ // same file.
unsigned NonSystemRemoved =
RemoveDuplicates(Lang, SearchList, NumQuoted, Verbose);
NumAngled -= NonSystemRemoved;
diff --git a/clang/test/Driver/cl-include.c b/clang/test/Driver/cl-include.c
index ca9e7db1e6f07c..a4a6cb4335f08a 100644
--- a/clang/test/Driver/cl-include.c
+++ b/clang/test/Driver/cl-include.c
@@ -7,37 +7,31 @@
// RUN: %clang_cl -nobuiltininc -### -- %s 2>&1 | FileCheck %s --check-prefix=NOBUILTIN
// NOBUILTIN-NOT: "-internal-isystem" "{{.*lib.*clang.*include}}"
-// RUN: env INCLUDE=/my/system/inc env EXTERNAL_INCLUDE=/my/system/inc2 %clang_cl -### -- %s 2>&1 | FileCheck %s --check-prefix=STDINC
-// STDINC: "-internal-isystem" "/my/system/inc"
-// STDINC: "-internal-isystem" "/my/system/inc2"
+// RUN: env INCLUDE=/my/system/inc %clang_cl -### -- %s 2>&1 | FileCheck %s --check-prefix=STDINC
+// STDINC: "-iexternal-after" "/my/system/inc"
// -nostdinc suppresses all of %INCLUDE%, clang resource dirs, and -imsvc dirs.
-// RUN: env INCLUDE=/my/system/inc env EXTERNAL_INCLUDE=/my/system/inc2 %clang_cl -nostdinc -imsvc /my/other/inc -### -- %s 2>&1 | FileCheck %s --check-prefix=NOSTDINC
+// RUN: env INCLUDE=/my/system/inc %clang_cl -nostdinc -imsvc /my/other/inc -### -- %s 2>&1 | FileCheck %s --check-prefix=NOSTDINC
// NOSTDINC: argument unused{{.*}}-imsvc
-// NOSTDINC-NOT: "-internal-isystem" "/my/system/inc"
-// NOSTDINC-NOT: "-internal-isystem" "/my/system/inc2"
+// NOSTDINC-NOT: "-iexternal-after" "/my/system/inc"
// NOSTDINC-NOT: "-internal-isystem" "{{.*lib.*clang.*include}}"
-// NOSTDINC-NOT: "-internal-isystem" "/my/other/inc"
-// /X suppresses %INCLUDE% and %EXTERNAL_INCLUDE% but not clang resource dirs, -imsvc dirs, or /external: flags.
-// RUN: env INCLUDE=/my/system/inc env EXTERNAL_INCLUDE=/my/system/inc2 env FOO=/my/other/inc2 %clang_cl /X -imsvc /my/other/inc /external:env:FOO -### -- %s 2>&1 | FileCheck %s --check-prefix=SLASHX
+// /X suppresses %INCLUDE% but not clang resource dirs, -imsvc dirs, or /external: flags.
+// RUN: env INCLUDE=/my/system/inc env FOO=/my/other/inc2 %clang_cl /X -imsvc /my/other/inc /external:env:FOO -### -- %s 2>&1 | FileCheck %s --check-prefix=SLASHX
// SLASHX-NOT: "argument unused{{.*}}-imsvc"
-// SLASHX-NOT: "-internal-isystem" "/my/system/inc"
-// SLASHX-NOT: "-internal-isystem" "/my/system/inc2"
+// SLASHX-NOT: "-external-after" "/my/system/inc"
+// SLASHX: "-iexternal-env=FOO"
// SLASHX: "-internal-isystem" "{{.*lib.*clang.*include}}"
// SLASHX: "-internal-isystem" "/my/other/inc"
-// SLASHX: "-internal-isystem" "/my/other/inc2"
-// /winsysroot suppresses %EXTERNAL_INCLUDE% but not -imsvc dirs or /external: flags.
-// RUN: env env EXTERNAL_INCLUDE=/my/system/inc env FOO=/my/other/inc2 %clang_cl /winsysroot /foo -imsvc /my/other/inc /external:env:FOO -### -- %s 2>&1 | FileCheck %s --check-prefix=SYSROOT
+// /winsysroot does not suppress -imsvc dirs or /external: flags.
+// RUN: env FOO=/my/other/inc2 %clang_cl /winsysroot /foo -imsvc /my/other/inc /external:env:FOO -### -- %s 2>&1 | FileCheck %s --check-prefix=SYSROOT
// SYSROOT-NOT: "argument unused{{.*}}-imsvc"
// SYSROOT-NOT: "argument unused{{.*}}/external:"
-// SYSROOT-NOT: "/my/system/inc"
+// SYSROOT: "-iexternal-env=FOO"
// SYSROOT: "-internal-isystem" "/my/other/inc"
-// SYSROOT: "-internal-isystem" "/my/other/inc2"
// SYSROOT: "-internal-isystem" "/foo{{.*}}"
// RUN: env "FOO=/dir1;/dir2" env "BAR=/dir3" %clang_cl /external:env:FOO /external:env:BAR -### -- %s 2>&1 | FileCheck %s --check-prefix=EXTERNAL_ENV
-// EXTERNAL_ENV: "-internal-isystem" "/dir1"
-// EXTERNAL_ENV: "-internal-isystem" "/dir2"
-// EXTERNAL_ENV: "-internal-isystem" "/dir3"
+// EXTERNAL_ENV: "-iexternal-env=FOO"
+// EXTERNAL_ENV: "-iexternal-env=BAR"
diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 8191fda97788c1..ec194cadae3d8a 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -40,7 +40,10 @@
// RUN: %clang_cl /external:Ipath -### -- %s 2>&1 | FileCheck -check-prefix=EXTERNAL_I %s
// RUN: %clang_cl /external:I path -### -- %s 2>&1 | FileCheck -check-prefix=EXTERNAL_I %s
-// EXTERNAL_I: "-isystem" "path"
+// EXTERNAL_I: "-iexternal" "path"
+
+// RUN: env EXTPATH="path1;path2" %clang_cl /external:env:EXTPATH -### -- %s 2>&1 | FileCheck -check-prefix=EXTERNAL_ENV %s
+// EXTERNAL_ENV: "-iexternal-env=EXTPATH"
// RUN: %clang_cl /fp:fast /fp:except -### -- %s 2>&1 | FileCheck -check-prefix=fpexcept %s
// fpexcept-NOT: -funsafe-math-optimizations
@@ -443,7 +446,6 @@
// RUN: /experimental:preprocessor \
// RUN: /exportHeader /headerName:foo \
// RUN: /external:anglebrackets \
-// RUN: /external:env:var \
// RUN: /external:W0 \
// RUN: /external:W1 \
// RUN: /external:W2 \
diff --git a/clang/test/Driver/microsoft-header-search-duplicates.c b/clang/test/Driver/microsoft-header-search-duplicates.c
index 3a01df8e90e4f4..05157270590d36 100644
--- a/clang/test/Driver/microsoft-header-search-duplicates.c
+++ b/clang/test/Driver/microsoft-header-search-duplicates.c
@@ -33,11 +33,9 @@
// XUN: %t/test.c 2>&1 | FileCheck -DPWD=%t %t/test.c
// Test the clang-cl driver with a Windows target that implicitly enables the
-// -fms-compatibility option. The /X option is used instead of -nostdinc
-// because the latter option suppresses all system include paths including
-// those specified by /external:I. The -nobuiltininc option is uesd to suppress
-// the Clang resource directory. The -nostdlibinc option is used to suppress
-// search paths for the Windows SDK, CRT, MFC, ATL, etc...
+// -fms-compatibility option. The -nobuiltininc option is uesd to suppress the
+// Clang resource directory. The -nostdlibinc option is used to suppress search
+// paths for the Windows SDK, CRT, MFC, ATL, etc...
// RUN: env INCLUDE="%t/include/p;%t/include/o" \
// RUN: env EXTRA_INCLUDE="%t/include/t;%t/include/q;%t/include/r" \
// RUN: %clang_cl \
@@ -77,21 +75,31 @@
#include <k.h>
#include <l.h>
-// The expected behavior is that user search paths that duplicate a system search
-// path are ignored, that user search paths that duplicate a previous user
-// search path are ignored, and that system search search paths that duplicate
-// a later system search path are ignored.
-// CHECK: ignoring duplicate directory "[[PWD]]/include/v"
-// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/x"
-// CHECK-NEXT: as it is a non-system directory that duplicates a system directory
-// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/z"
-// CHECK-NEXT: as it is a non-system directory that duplicates a system directory
-// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/t"
-// CHECK-NEXT: as it is a non-system directory that duplicates a system directory
-// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/r"
-// CHECK-NEXT: as it is a non-system directory that duplicates a system directory
-// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/y"
-// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/w"
+// Header search paths are processed as follows:
+// 1) Paths specified by the /I and /external:I options are processed in order.
+// 1.1) Paths specified by /I that duplicate a path specified by /external:I
+// are ignored regardless of the option order.
+// 1.2) Paths specified by /I that duplicate a prior /I option are ignored.
+// 1.3) Paths specified by /external:I that duplicate a later /external:I
+// option are ignored.
+// 2) Paths specified by the /external:env options are processed in order.
+// Paths that duplicate a path from a /I option, an external:I option, a
+// prior /external:env option, or a prior path from the current /external:env
+// option are ignored.
+// 3) Paths specified by the %INCLUDE% environment variable are processed in
+// order. Paths that duplicate any other path are ignored.
+
+// XHECK: ignoring duplicate directory "[[PWD]]/include/v"
+// XHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/x"
+// XHECK-NEXT: as it is a non-system directory that duplicates a system directory
+// XHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/z"
+// XHECK-NEXT: as it is a non-system directory that duplicates a system directory
+// XHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/t"
+// XHECK-NEXT: as it is a non-system directory that duplicates a system directory
+// XHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/r"
+// XHECK-NEXT: as it is a non-system directory that duplicates a system directory
+// XHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/y"
+// XHECK-NEXT: ignoring duplicate directory "[[PWD]]/include/w"
// CHECK: #include <...> search starts here:
// CHECK-NEXT: [[PWD]]/include/s
// CHECK-NEXT: [[PWD]]/include/v
diff --git a/llvm/include/llvm/Option/ArgList.h b/llvm/include/llvm/Option/ArgList.h
index 072a7a371d7a26..08d8e9d5b73d11 100644
--- a/llvm/include/llvm/Option/ArgList.h
+++ b/llvm/include/llvm/Option/ArgList.h
@@ -288,8 +288,13 @@ class ArgList {
/// getAllArgValues - Get the values of all instances of the given argument
/// as strings.
+ template<typename ...OptSpecifiers>
std::vector<std::string>
- getAllArgValues(OptSpecifier Id0, OptSpecifier Id1 = 0U) const;
+ getAllArgValues(OptSpecifiers ...Ids) const {
+ SmallVector<const char *, 16> Values;
+ AddAllArgValues(Values, Ids...);
+ return std::vector<std::string>(Values.begin(), Values.end());
+ }
/// @}
/// @name Translation Utilities
diff --git a/llvm/lib/Option/ArgList.cpp b/llvm/lib/Option/ArgList.cpp
index 345754dec9d7b9..3bbff6c75e78aa 100644
--- a/llvm/lib/Option/ArgList.cpp
+++ b/llvm/lib/Option/ArgList.cpp
@@ -95,13 +95,6 @@ StringRef ArgList::getLastArgValue(OptSpecifier Id, StringRef Default) const {
return Default;
}
-std::vector<std::string>
-ArgList::getAllArgValues(OptSpecifier Id0, OptSpecifier Id1) const {
- SmallVector<const char *, 16> Values;
- AddAllArgValues(Values, Id0, Id1);
- return std::vector<std::string>(Values.begin(), Values.end());
-}
-
void ArgList::addOptInFlag(ArgStringList &Output, OptSpecifier Pos,
OptSpecifier Neg) const {
if (Arg *A = getLastArg(Pos, Neg))
More information about the cfe-commits
mailing list