[clang] [llvm] [Clang] Match MSVC handling of duplicate header search paths in Microsoft compatibility modes. (PR #105738)

Tom Honermann via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 7 13:15:56 PST 2025


https://github.com/tahonermann updated https://github.com/llvm/llvm-project/pull/105738

>From 8ecdbe842c2e42be9247788ed16d6021e9488f19 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/4] [Clang] Match MSVC handling of duplicate header search
 paths in Microsoft compatibility modes.

Clang has historically matched GCC's behavior for header search path order
and pruning of duplicate paths. That traditional behavior is to order user
search paths before system search paths, to ignore user search paths that
duplicate a (later) system search path, and to ignore search paths that
duplicate an earlier search path of the same user/system kind. This differs
from MSVC and can result in inconsistent header file resolution for `#include`
directives.

MSVC orders header search paths as follows:
1) Paths specified by the `/I` and `/external:I` options are processed in
   the order that they appear. Paths specified by `/I` that duplicate a path
   specified by `/external:I` are ignored regardless of the order of the
   options. Paths specified by `/I` that duplicate a path from a prior `/I`
   option are ignored. Paths specified by `/external:I` that duplicate a
   path from a later `/external:I` option are ignored.
2) Paths specified by `/external:env` are processed in the order that they
   appear. Paths that duplicate a path from a `/I` or `/external:I` option
   are ignored regardless of the order of the options. Paths that duplicate a
   path from a prior `/external:env` option or an earlier path from the same
   `/external:env` option are ignored.
3) Paths specified by the `INCLUDE` environment variable are processed in
   the order they appear. Paths that duplicate a path from a `/I`,
   `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate an earlier path in the `INCLUDE` environment variable are
   ignored.
4) Paths specified by the `EXTERNAL_INCLUDE` environment variable are
   processed in the order they appear. Paths that duplicate a path from a
   `/I`, `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate a path from the `INCLUDE` environment variable are ignored.
   Paths that duplicate an earlier path in the `EXTERNAL_INCLUDE
   environment variable are ignored.

Prior to this change, Clang handled the `/external:I` and `/external:env`
options and the paths present in the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variables as though they were specified with the `-isystem` option.
The GCC behavior described above then lead to a command line such as
`/external:I dir1 /Idir2` having a header search order of `dir2` followed by
`dir1`; contrary to MSVC behavior.

This change adds support for the MSVC external path concept for both the `clang`
and `clang-cl` drivers with the following option syntax. These options match the
MSVC behavior described above for both drivers.
                 clang                    clang-cl
                 --------------------     -------------------
                 -iexternal <dir>         /external:I <dir>
                 -iexternal-env=<ENV>     /external:env:<ENV>
Paths specified by these options are still treated as system paths. That is,
whether warnings are issued in header files found via these paths remains
subject to use of the `-Wsystem-headers` and `-Wno-system-headers` options.
In the future, it would make sense to add a separate option that matches the
MSVC `/external:Wn` option to control such warnings.

The MSVC behavior described above implies that (system) paths present in the
`INCLUDE` and `EXTERNAL_INCLUDE` environment variables do not suppress matching
user paths specified via `/I`. This contrasts with GCC's behavior of suppressing
user paths that match a system path regardless of how each is specified. Since
the `clang-cl` driver maps paths from the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variable to `-internal-isystem`, matching MSVC behavior requires
suppressing that aspect of the GCC behavior. With this change, system paths
will no longer suppress user paths when the `-fms-compatibility` option is
explicitly or implicitly enabled. This will affect header search path ordering
for options like `-isystem` when duplicate user paths are present. Should
motivation arise for preserving such suppression of user paths when compiling
with `-fms-compatibility` enabled, it would make sense to introduce a new
option for the `clang-cl` driver to map paths in these environment variabless
to that would match MSVC behavior without impacting other system path options.
---
 clang/docs/ReleaseNotes.rst                   |  38 ++
 clang/include/clang/Driver/Options.td         |  17 +-
 clang/include/clang/Driver/ToolChain.h        |  11 +-
 clang/include/clang/Lex/HeaderSearchOptions.h |  10 +
 clang/lib/Driver/Driver.cpp                   |   4 +-
 clang/lib/Driver/ToolChain.cpp                |  45 +++
 clang/lib/Driver/ToolChains/Clang.cpp         |  15 +-
 clang/lib/Driver/ToolChains/MSVC.cpp          |  28 +-
 clang/lib/Frontend/CompilerInvocation.cpp     |  30 +-
 clang/lib/Lex/InitHeaderSearch.cpp            | 172 ++++----
 clang/test/Driver/cl-include.c                |  20 +-
 clang/test/Driver/cl-options.c                |   7 +-
 clang/test/Driver/header-search-duplicates.c  | 370 ++++++++++++++++++
 .../microsoft-header-search-duplicates.c      | 363 +++++++++++++++++
 llvm/include/llvm/Option/ArgList.h            |   7 +-
 llvm/lib/Option/ArgList.cpp                   |   6 -
 16 files changed, 1016 insertions(+), 127 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 a541d399d1e749..ec3edc8c940301 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -944,6 +944,44 @@ 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 regarding the 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. This ordering is not compatible with
+  the ordering that MSVC uses when paths are duplicated across ``/I`` options
+  and the ``INCLUDE`` environment variable.
+
+  The order that MSVC uses and that Clang now replicates when the
+  ``-fms-compatibility`` option is enabled follows.
+
+  - Paths specified by the ``/I`` and ``/external:I`` options are processed in
+    the order that they appear. Paths specified by ``/I`` that duplicate a path
+    specified by ``/external:I`` are ignored regardless of the order of the
+    options. Paths specified by ``/I`` that duplicate a path from a prior ``/I``
+    option are ignored. Paths specified by ``/external:I`` that duplicate a
+    path from a later ``/external:I`` option are ignored.
+
+  - Paths specified by ``/external:env`` are processed in the order that they
+    appear. Paths that duplicate a path from a ``/I`` or ``/external:I`` option
+    are ignored regardless of the order of the options. Paths that duplicate a
+    path from a prior ``/external:env`` option or an earlier path from the same
+    ``/external:env`` option are ignored.
+
+  - Paths specified by the ``INCLUDE`` environment variable are processed in
+    the order they appear. Paths that duplicate a path from a ``/I``,
+    ``/external:I``, or ``/external:env`` option are ignored. Paths that
+    duplicate an earlier path in the ``INCLUDE`` environment variable are
+    ignored.
+
+  - Paths specified by the ``EXTERNAL_INCLUDE`` environment variable are
+    processed in the order they appear. Paths that duplicate a path from a
+    ``/I``, ``/external:I``, or ``/external:env`` option are ignored. Paths that
+    duplicate a path from the ``INCLUDE`` environment variable are ignored.
+    Paths that duplicate an earlier path in the ``EXTERNAL_INCLUDE``
+    environment variable are ignored.
+
 LoongArch Support
 ^^^^^^^^^^^^^^^^^
 
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 88862ae9edb29d..bf84c7fc71b764 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4587,6 +4587,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 : Separate<["-"], "iexternal">, Group<clang_i_Group>,
+  Visibility<[ClangOption, CC1Option]>,
+  HelpText<"Add directory to include search path with warnings suppressed">, MetaVarName<"<dir>">;
+def iexternal_after : Separate<["-"], "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]>,
+  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">;
@@ -8502,9 +8511,12 @@ 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_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"]>;
@@ -8740,9 +8752,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..6687c3b3833fde 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -224,6 +224,7 @@ class ToolChain {
   /// \return The subdirectory path if it exists.
   std::optional<std::string> getTargetSubDirPath(StringRef BaseDir) const;
 
+public:
   /// \name Utilities for implementing subclasses.
   ///@{
   static void addSystemInclude(const llvm::opt::ArgList &DriverArgs,
@@ -239,12 +240,20 @@ class ToolChain {
   static void addSystemIncludes(const llvm::opt::ArgList &DriverArgs,
                                 llvm::opt::ArgStringList &CC1Args,
                                 ArrayRef<StringRef> Paths);
+  static bool addSystemIncludesFromEnv(const llvm::opt::ArgList &DriverArgs,
+                                       llvm::opt::ArgStringList &CC1Args,
+                                       StringRef Var);
+  static void addExternalAfterIncludes(const llvm::opt::ArgList &DriverArgs,
+                                       llvm::opt::ArgStringList &CC1Args,
+                                       ArrayRef<StringRef> Paths);
+  static bool addExternalIncludesFromEnv(const llvm::opt::ArgList &DriverArgs,
+                                         llvm::opt::ArgStringList &CC1Args,
+                                         StringRef Var);
 
   static std::string concat(StringRef Path, const Twine &A, const Twine &B = "",
                             const Twine &C = "", const Twine &D = "");
   ///@}
 
-public:
   virtual ~ToolChain();
 
   // Accessors
diff --git a/clang/include/clang/Lex/HeaderSearchOptions.h b/clang/include/clang/Lex/HeaderSearchOptions.h
index 7a16926c186d2c..0aa0e533b1ded8 100644
--- a/clang/include/clang/Lex/HeaderSearchOptions.h
+++ b/clang/include/clang/Lex/HeaderSearchOptions.h
@@ -35,6 +35,16 @@ enum IncludeDirGroup {
   /// Paths for '\#include <>' added by '-I'.
   Angled,
 
+  /// Like Angled, but marks system directories while retaining relative order
+  /// with user directories. This group is intended to match the semantics of
+  /// the MSVC /external:I option.
+  External,
+
+  /// Like External, but searched after other external directories but before
+  /// system directories. This group 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 fb73b62cf2daed..ed0eee61c280b9 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1306,7 +1306,9 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
     if (VFS->setCurrentWorkingDirectory(WD->getValue()))
       Diag(diag::err_drv_unable_to_set_working_directory) << WD->getValue();
 
-  // Check for missing include directories.
+  // Check for missing include directories. Diagnostics should not be issued
+  // for directories specified with -iexternal, -iexternal-env=, or
+  // -iexternal-after since those options may be used to specify partial paths.
   if (!Diags.isIgnored(diag::warn_missing_include_dirs, SourceLocation())) {
     for (auto IncludeDir : Args.getAllArgValues(options::OPT_I_Group)) {
       if (!VFS->exists(IncludeDir))
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 9f174fbda398b5..1b49ef20b8020f 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1301,6 +1301,51 @@ void ToolChain::addExternCSystemIncludeIfExists(const ArgList &DriverArgs,
   }
 }
 
+/// Utility function to add a list of ';' delimited directories specified in
+/// an environment variable to the system include path list for CC1. Returns
+/// true if the variable is set and not empty.
+/*static*/ bool ToolChain::addSystemIncludesFromEnv(const ArgList &DriverArgs,
+                                                    ArgStringList &CC1Args,
+                                                    StringRef Var) {
+  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;
+}
+
+/// 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));
+  }
+}
+
+/// Utility function to add a list of ';' delimited directories specified in
+/// an environment variable to the external include path list for CC1. Returns
+/// true if the variable is set and not empty.
+/*static*/ bool ToolChain::addExternalIncludesFromEnv(const ArgList &DriverArgs,
+                                                      ArgStringList &CC1Args,
+                                                      StringRef Var) {
+  if (auto Val = llvm::sys::Process::GetEnv(Var)) {
+    SmallVector<StringRef, 8> Dirs;
+    StringRef(*Val).split(Dirs, ";", /*MaxSplit=*/-1, /*KeepEmpty=*/false);
+    if (!Dirs.empty()) {
+      addExternalAfterIncludes(DriverArgs, CC1Args, Dirs);
+      return true;
+    }
+  }
+  return false;
+}
+
 /*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 d3206c3e8e25ed..2b7aa738b33b28 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1198,6 +1198,13 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
     } else if (A->getOption().matches(options::OPT_ibuiltininc)) {
       // This is used only by the driver. No need to pass to cc1.
       continue;
+    } else if (A->getOption().matches(options::OPT_iexternal)) {
+      // This option has to retain relative order with other -I options.
+      continue;
+    } else if (A->getOption().matches(options::OPT_iexternal_env_EQ)) {
+      A->claim();
+      ToolChain::addExternalIncludesFromEnv(Args, CmdArgs, A->getValue());
+      continue;
     }
 
     // Not translated, render as usual.
@@ -1205,9 +1212,9 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
     A->render(Args, CmdArgs);
   }
 
-  Args.addAllArgs(CmdArgs,
-                  {options::OPT_D, options::OPT_U, options::OPT_I_Group,
-                   options::OPT_F, options::OPT_embed_dir_EQ});
+  Args.addAllArgs(CmdArgs, {options::OPT_D, options::OPT_U,
+                            options::OPT_I_Group, options::OPT_F,
+                            options::OPT_iexternal, options::OPT_embed_dir_EQ});
 
   // Add -Wp, and -Xpreprocessor if using the preprocessor.
 
@@ -8678,7 +8685,7 @@ void ClangAs::ConstructJob(Compilation &C, const JobAction &JA,
   (void)Args.hasArg(options::OPT_force__cpusubtype__ALL);
 
   // Pass along any -I options so we get proper .include search paths.
-  Args.AddAllArgs(CmdArgs, options::OPT_I_Group);
+  Args.addAllArgs(CmdArgs, {options::OPT_I_Group, options::OPT_iexternal});
 
   // Pass along any --embed-dir or similar options so we get proper embed paths.
   Args.AddAllArgs(CmdArgs, options::OPT_embed_dir_EQ);
diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp
index 80799d1e715f07..1590f572da97cb 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -648,24 +648,6 @@ void MSVCToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
   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;
-  };
-
-  // 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)) {
@@ -682,12 +664,14 @@ 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 paths from the INCLUDE and EXTERNAL_INCLUDE environment variables if
+  // neither a vctoolsdir or winsysroot directory has been explicitly specified.
+  // If any paths are present in these environment variables, then skip adding
+  // additional system directories.
   if (!DriverArgs.getLastArg(options::OPT__SLASH_vctoolsdir,
                              options::OPT__SLASH_winsysroot)) {
-    bool Found = AddSystemIncludesFromEnv("INCLUDE");
-    Found |= AddSystemIncludesFromEnv("EXTERNAL_INCLUDE");
+    bool Found = addSystemIncludesFromEnv(DriverArgs, CC1Args, "INCLUDE");
+    Found |= addSystemIncludesFromEnv(DriverArgs, CC1Args, "EXTERNAL_INCLUDE");
     if (Found)
       return;
   }
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 98136b7a455d9c..30e715e92f47b0 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3200,19 +3200,27 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
   auto It = Opts.UserEntries.begin();
   auto End = Opts.UserEntries.end();
 
-  // Add -I... and -F... options in order.
-  for (; It < End && Matches(*It, {frontend::Angled}, std::nullopt, true);
+  // Add the -I..., -F..., and -iexternal options in order.
+  for (; It < End && Matches(*It, {frontend::Angled, frontend::External},
+                             std::nullopt, true);
        ++It) {
     OptSpecifier Opt = [It, Matches]() {
       if (Matches(*It, frontend::Angled, true, true))
         return OPT_F;
       if (Matches(*It, frontend::Angled, false, true))
         return OPT_I;
+      if (Matches(*It, frontend::External, false, true))
+        return OPT_iexternal;
       llvm_unreachable("Unexpected HeaderSearchOptions::Entry.");
     }();
 
     GenerateArg(Consumer, Opt, It->Path);
-  };
+  }
+
+  // Add the paths for the -iexternal-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
@@ -3322,7 +3330,6 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
         llvm::CachedHashString(MacroDef.split('=').first));
   }
 
-  // Add -I... and -F... options in order.
   bool IsSysrootSpecified =
       Args.hasArg(OPT__sysroot_EQ) || Args.hasArg(OPT_isysroot);
 
@@ -3341,12 +3348,21 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
     return A->getValue();
   };
 
-  for (const auto *A : Args.filtered(OPT_I, OPT_F)) {
+  // Add the -I..., -F..., and -iexternal options in order.
+  for (const auto *A : Args.filtered(OPT_I, OPT_F, OPT_iexternal)) {
+    frontend::IncludeDirGroup Group = frontend::Angled;
+    if (A->getOption().matches(OPT_iexternal))
+      Group = frontend::External;
     bool IsFramework = A->getOption().matches(OPT_F);
-    Opts.AddPath(PrefixHeaderPath(A, IsFramework), frontend::Angled,
-                 IsFramework, /*IgnoreSysroot=*/true);
+    Opts.AddPath(PrefixHeaderPath(A, IsFramework), Group, IsFramework,
+                 /*IgnoreSysroot=*/true);
   }
 
+  // Add the -iexternal-env= and -iexternal-after options in order.
+  for (const auto *A : Args.filtered(OPT_iexternal_after))
+    Opts.AddPath(A->getValue(), 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 ea02f5dfb62644..54c7195f6ee0a9 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -151,6 +151,7 @@ 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.
     Type = SrcMgr::C_System;
   }
 
@@ -362,90 +363,108 @@ void InitHeaderSearch::AddDefaultIncludePaths(
   AddDefaultCIncludePaths(triple, HSOpts);
 }
 
-/// 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,
-                                 unsigned First, bool Verbose) {
+/// Remove duplicate paths from a partitioned search list with a diagnostic
+/// issued if Verbose is true. Partitioning is at the discretion of the
+/// caller and may be used to, for example, indicate a division between user
+/// and system search paths. If partitioning is not needed, then call with
+/// Part1Begin equal to Part2Begin. The return value is the number of items
+/// removed from the first partition.
+static unsigned RemoveDuplicates(const LangOptions &Lang,
+                                 std::vector<DirectoryLookupInfo> &SearchList,
+                                 unsigned Part1Begin, unsigned Part2Begin,
+                                 bool Verbose) {
   llvm::SmallPtrSet<const DirectoryEntry *, 8> SeenDirs;
   llvm::SmallPtrSet<const DirectoryEntry *, 8> SeenFrameworkDirs;
   llvm::SmallPtrSet<const HeaderMap *, 8> SeenHeaderMaps;
-  unsigned NonSystemRemoved = 0;
-  for (unsigned i = First; i != SearchList.size(); ++i) {
-    unsigned DirToRemove = i;
-
+  unsigned NumPart1DirsRemoved = 0;
+  for (unsigned i = Part1Begin; i != SearchList.size(); ++i) {
+    IncludeDirGroup CurGroup = SearchList[i].Group;
     const DirectoryLookup &CurEntry = SearchList[i].Lookup;
+    SrcMgr::CharacteristicKind CurSrcKind = CurEntry.getDirCharacteristic();
 
+    // 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;
     }
 
-    // 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) {
-      // Find the dir that this is the same of.
-      unsigned FirstDir;
-      for (FirstDir = First;; ++FirstDir) {
-        assert(FirstDir != i && "Didn't find dupe?");
-
-        const DirectoryLookup &SearchEntry = SearchList[FirstDir].Lookup;
-
-        // If these are different lookup types, then they can't be the dupe.
-        if (SearchEntry.getLookupType() != CurEntry.getLookupType())
-          continue;
-
-        bool isSame;
-        if (CurEntry.isNormalDir())
-          isSame = SearchEntry.getDir() == CurEntry.getDir();
-        else if (CurEntry.isFramework())
-          isSame = SearchEntry.getFrameworkDir() == CurEntry.getFrameworkDir();
-        else {
-          assert(CurEntry.isHeaderMap() && "Not a headermap or normal dir?");
-          isSame = SearchEntry.getHeaderMap() == CurEntry.getHeaderMap();
-        }
-
-        if (isSame)
-          break;
+    // Find the previous matching search entry.
+    unsigned PrevIndex;
+    for (PrevIndex = Part1Begin; PrevIndex < i; ++PrevIndex) {
+      const DirectoryLookup &SearchEntry = SearchList[PrevIndex].Lookup;
+
+      // Different lookup types are not considered duplicate entries.
+      if (SearchEntry.getLookupType() != CurEntry.getLookupType())
+        continue;
+
+      bool isSame;
+      if (CurEntry.isNormalDir())
+        isSame = SearchEntry.getDir() == CurEntry.getDir();
+      else if (CurEntry.isFramework())
+        isSame = SearchEntry.getFrameworkDir() == CurEntry.getFrameworkDir();
+      else {
+        assert(CurEntry.isHeaderMap() && "Not a headermap or normal dir?");
+        isSame = SearchEntry.getHeaderMap() == CurEntry.getHeaderMap();
       }
 
-      // 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)
-        DirToRemove = FirstDir;
+      if (isSame)
+        break;
+    }
+    assert(PrevIndex < i && "Expected duplicate search location not found");
+    const DirectoryLookup &PrevEntry = SearchList[PrevIndex].Lookup;
+    SrcMgr::CharacteristicKind PrevSrcKind = PrevEntry.getDirCharacteristic();
+
+    // By default, a search path that follows a previous matching search path
+    // is removed. Exceptions exist for paths from the External include group
+    // and for uesr paths that match a later system path.
+    unsigned DirToRemove = i;
+    if (CurGroup == frontend::External) {
+      // A path that matches a later path specified by -iexternal is always
+      // suppressed.
+      DirToRemove = PrevIndex;
+    } else if (!Lang.MSVCCompat && PrevSrcKind == SrcMgr::C_User &&
+               CurSrcKind != SrcMgr::C_User) {
+      // When not in Microsoft compatibility mode, a user path that matches
+      // a later system path is suppressed.
+      DirToRemove = PrevIndex;
     }
 
+    // If requested, issue a diagnostic about the ignored directory.
     if (Verbose) {
+      bool NonSystemDirRemoved = false;
+      if (DirToRemove == i)
+        NonSystemDirRemoved =
+            PrevSrcKind != SrcMgr::C_User && CurSrcKind == SrcMgr::C_User;
+      else
+        NonSystemDirRemoved =
+            PrevSrcKind == SrcMgr::C_User && CurSrcKind != SrcMgr::C_User;
+
       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)
-      ++NonSystemRemoved;
 
-    // This is reached if the current entry is a duplicate.  Remove the
-    // DirToRemove (usually the current dir).
+    // Remove the duplicate entry from the search list.
     SearchList.erase(SearchList.begin()+DirToRemove);
     --i;
+
+    // Adjust the partition boundaries if necessary.
+    if (DirToRemove < Part2Begin) {
+      ++NumPart1DirsRemoved;
+      --Part2Begin;
+    }
   }
-  return NonSystemRemoved;
+  return NumPart1DirsRemoved;
 }
 
 /// Extract DirectoryLookups from DirectoryLookupInfos.
@@ -475,22 +494,32 @@ 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);
+  // Remove duplicate search paths within the quoted inclusion list.
+  RemoveDuplicates(Lang, SearchList, 0, 0, Verbose);
+  unsigned EndQuoted = SearchList.size();
 
-  // Deduplicate and remember index.
-  RemoveDuplicates(SearchList, 0, Verbose);
-  unsigned NumQuoted = SearchList.size();
-
+  // Add search paths for angled inclusion next. Note that user paths and
+  // external paths may be interleaved; though external paths are treated like
+  // system paths, they are not reordered to the end of the search list.
   for (auto &Include : IncludePath)
-    if (Include.Group == Angled)
+    if (Include.Group == Angled || Include.Group == External)
       SearchList.push_back(Include);
+  // Add external search paths that must come at the end of the angled
+  // inclusion list.
+  for (auto &Include : IncludePath)
+    if (Include.Group == ExternalAfter)
+      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, EndQuoted, EndQuoted, Verbose);
+  unsigned EndAngled = SearchList.size();
 
-  RemoveDuplicates(SearchList, NumQuoted, Verbose);
-  unsigned NumAngled = SearchList.size();
-
+  // 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) ||
@@ -499,18 +528,21 @@ 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.
-  unsigned NonSystemRemoved = RemoveDuplicates(SearchList, NumQuoted, Verbose);
-  NumAngled -= NonSystemRemoved;
+  // 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. This may result in earlier user paths being removed, and thus
+  // requires updating the EndAngled index.
+  unsigned NonSystemRemoved =
+      RemoveDuplicates(Lang, SearchList, EndQuoted, EndAngled, Verbose);
+  EndAngled -= NonSystemRemoved;
 
-  Headers.SetSearchPaths(extractLookups(SearchList), NumQuoted, NumAngled,
+  Headers.SetSearchPaths(extractLookups(SearchList), EndQuoted, EndAngled,
                          mapToUserEntries(SearchList));
 
   Headers.SetSystemHeaderPrefixes(SystemHeaderPrefixes);
@@ -519,7 +551,7 @@ void InitHeaderSearch::Realize(const LangOptions &Lang) {
   if (Verbose) {
     llvm::errs() << "#include \"...\" search starts here:\n";
     for (unsigned i = 0, e = SearchList.size(); i != e; ++i) {
-      if (i == NumQuoted)
+      if (i == EndQuoted)
         llvm::errs() << "#include <...> search starts here:\n";
       StringRef Name = SearchList[i].Lookup.getName();
       const char *Suffix;
diff --git a/clang/test/Driver/cl-include.c b/clang/test/Driver/cl-include.c
index ca9e7db1e6f07c..df5e2aeb4d4a1a 100644
--- a/clang/test/Driver/cl-include.c
+++ b/clang/test/Driver/cl-include.c
@@ -8,36 +8,38 @@
 // 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" "{{.*lib.*clang.*include}}"
 // STDINC: "-internal-isystem" "/my/system/inc"
 // STDINC: "-internal-isystem" "/my/system/inc2"
 
 // -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
 // NOSTDINC: argument unused{{.*}}-imsvc
-// NOSTDINC-NOT: "-internal-isystem" "/my/system/inc"
-// NOSTDINC-NOT: "-internal-isystem" "/my/system/inc2"
 // NOSTDINC-NOT: "-internal-isystem" "{{.*lib.*clang.*include}}"
 // NOSTDINC-NOT: "-internal-isystem" "/my/other/inc"
+// NOSTDINC-NOT: "-internal-isystem" "/my/system/inc"
+// NOSTDINC-NOT: "-internal-isystem" "/my/system/inc2"
 
 // /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
 // SLASHX-NOT: "argument unused{{.*}}-imsvc"
 // SLASHX-NOT: "-internal-isystem" "/my/system/inc"
 // SLASHX-NOT: "-internal-isystem" "/my/system/inc2"
+// SLASHX: "-iexternal-after" "/my/other/inc2"
 // 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 suppresses %INCLUDE% and %EXTERNAL_INCLUDE% but not -imsvc dirs or /external: flags.
+// RUN: env INCLUDE=/my/system/inc env EXTERNAL_INCLUDE=/my/system/inc2 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-NOT: "/my/system/inc2"
+// SYSROOT: "-iexternal-after" "/my/other/inc2"
 // 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-after" "/dir1"
+// EXTERNAL_ENV: "-iexternal-after" "/dir2"
+// EXTERNAL_ENV: "-iexternal-after" "/dir3"
diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 477e8489e74280..434acf09ae6447 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -40,7 +40,11 @@
 
 // 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-after" "path1"
+// EXTERNAL_ENV: "-iexternal-after" "path2"
 
 // RUN: %clang_cl /fp:fast /fp:except -### -- %s 2>&1 | FileCheck -check-prefix=fpexcept %s
 // fpexcept-NOT: -funsafe-math-optimizations
@@ -443,7 +447,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/header-search-duplicates.c b/clang/test/Driver/header-search-duplicates.c
new file mode 100644
index 00000000000000..b539bcdf18e2e7
--- /dev/null
+++ b/clang/test/Driver/header-search-duplicates.c
@@ -0,0 +1,370 @@
+// 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
+// RUN: split-file %s %t
+
+// This test exercises the clang driver using a target that does not implicitly
+// enable the -fms-compatibility option. The -nostdinc option is used to
+// suppress default search paths to ease testing.
+
+// Header search paths are categorized into the following general groups.
+// - Quoted: Search paths that are only used to resolve inclusion of header
+//   files specified with quoted inclusion ('#include "X"'). Paths nominated
+//   by the '-iquoted' option are added to this group.
+// - Angled: Search paths used to resolve inclusion of header files specified
+//   with angled inclusion ('#include <X>') or quoted inclusion if a match
+//   was not found in the Quoted group. Paths nominated by the '-I',
+//   '-iexternal', '-iexternal-env=', and '-iwithprefixbefore' options are
+//   added to this group.
+// - System: Search paths used to resolve inclusion of a header file for which
+//   a match is not found in the Quoted or Angled groups. Paths nominated by
+//   the '-dirafter', '-isystem', '-isystem-after', '-iwithprefix', and
+//   related language specific options are added to this group.
+// Duplicate search paths are identified and processed as follows:
+// 1) Paths in the Quoted group that duplicate a previous path in the Quoted
+//    group are removed.
+// 2) Paths in the Angled group that are duplicated by an external path
+//    (as nominated by the '-iexternal' or '-iexternal-env=' options) in the
+//    Angled group (regardless of the relative order of the paths) or by a
+//    path in the System group are removed
+// 3) Paths in the Angled or System groups that duplicate a previous path in
+//    the Angled or System group are removed.
+
+
+// Test 1: Validate ordering and duplicate elimination in the Quoted group.
+// This test exhibits a behavioral difference between GCC and Clang. GCC
+// removes the last path in the quoted group if it matches the first path
+// in the angled group. Clang does not. The difference is observable via
+// '#include_next' as this test demonstrates. Clang's behavior makes use of
+// '#include_next' across the Quoted and Angled groups reliable regardless
+// of whether there is an intervening search path present at the start of
+// the Angled group.
+//
+// RUN: %clang \
+// RUN:     -target x86_64-unknown-linux-gnu -v -fsyntax-only \
+// RUN:     -nostdinc \
+// RUN:     -iquote %t/test1/include/x \
+// RUN:     -iquote %t/test1/include/y \
+// RUN:     -iquote %t/test1/include/x \
+// RUN:     -iquote %t/test1/include/z \
+// RUN:     -I%t/test1/include/z \
+// RUN:     -I%t/test1/include/y \
+// RUN:     %t/test1/t.c 2>&1 | FileCheck -DPWD=%t %t/test1/t.c
+
+#--- test1/t.c
+#include "a.h"
+#include "b.h"
+#include "c.h"
+
+// CHECK:      ignoring duplicate directory "[[PWD]]/test1/include/x"
+// CHECK-NEXT: #include "..." search starts here:
+// CHECK-NEXT: [[PWD]]/test1/include/x
+// CHECK-NEXT: [[PWD]]/test1/include/y
+// CHECK-NEXT: [[PWD]]/test1/include/z
+// CHECK-NEXT: #include <...> search starts here:
+// CHECK-NEXT: [[PWD]]/test1/include/z
+// CHECK-NEXT: [[PWD]]/test1/include/y
+// CHECK-NEXT: End of search list.
+
+#--- test1/include/x/a.h
+
+#--- test1/include/y/a.h
+#error 'test1/include/y/a.h' should not have been included!
+
+#--- test1/include/y/b.h
+#if !defined(Y_B_DEFINED)
+#define Y_B_DEFINED
+#include_next <b.h>
+#endif
+
+#--- test1/include/z/a.h
+#error 'test1/include/z/a.h' should not have been included!
+
+#--- test1/include/z/b.h
+#if !defined(Y_B_DEFINED)
+#error 'Y_B_DEFINED' is not defined in test1/include/z/b.h!
+#endif
+
+#--- test1/include/z/c.h
+#if !defined(Z_C_DEFINED)
+#define Z_C_DEFINED
+#include_next <c.h>
+#endif
+
+
+// Test 2: Validate ordering and duplicate elimination in the Angled group.
+//
+// RUN: %clang \
+// RUN:     -target x86_64-unknown-linux-gnu -v -fsyntax-only \
+// RUN:     -nostdinc \
+// RUN:     -iprefix %t/ \
+// RUN:     -I%t/test2/include/v \
+// RUN:     -iwithprefixbefore test2/include/y \
+// RUN:     -I%t/test2/include/u \
+// RUN:     -iexternal %t/test2/include/v \
+// RUN:     -iwithprefixbefore test2/include/z \
+// RUN:     -iexternal %t/test2/include/w \
+// RUN:     -I%t/test2/include/x \
+// RUN:     -iexternal %t/test2/include/y \
+// RUN:     -iwithprefixbefore test2/include/x \
+// RUN:     %t/test2/t.c 2>&1 | FileCheck -DPWD=%t %t/test2/t.c
+
+#--- test2/t.c
+#include <a.h>
+#include <b.h>
+#include <c.h>
+#include <d.h>
+#include <e.h>
+#include <f.h>
+
+// CHECK:      ignoring duplicate directory "[[PWD]]/test2/include/v"
+// CHECK-NEXT:  as it is a non-system directory that duplicates a system directory
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test2/include/y"
+// CHECK-NEXT:  as it is a non-system directory that duplicates a system directory
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test2/include/x"
+// CHECK-NEXT: #include "..." search starts here:
+// CHECK-NEXT: #include <...> search starts here:
+// CHECK-NEXT: [[PWD]]/test2/include/u
+// CHECK-NEXT: [[PWD]]/test2/include/v
+// CHECK-NEXT: [[PWD]]/test2/include/w
+// CHECK-NEXT: [[PWD]]/test2/include/x
+// CHECK-NEXT: [[PWD]]/test2/include/y
+// CHECK-NEXT: [[PWD]]/test2/include/z
+// CHECK-NEXT: End of search list.
+
+#--- test2/include/u/a.h
+
+#--- test2/include/v/a.h
+#error 'test2/include/v/a.h' should not have been included!
+
+#--- test2/include/v/b.h
+
+#--- test2/include/w/a.h
+#error 'test2/include/w/a.h' should not have been included!
+
+#--- test2/include/w/b.h
+#error 'test2/include/w/b.h' should not have been included!
+
+#--- test2/include/w/c.h
+
+#--- test2/include/x/a.h
+#error 'test2/include/x/a.h' should not have been included!
+
+#--- test2/include/x/b.h
+#error 'test2/include/x/b.h' should not have been included!
+
+#--- test2/include/x/c.h
+#error 'test2/include/x/c.h' should not have been included!
+
+#--- test2/include/x/d.h
+
+#--- test2/include/y/a.h
+#error 'test2/include/y/a.h' should not have been included!
+
+#--- test2/include/y/b.h
+#error 'test2/include/y/b.h' should not have been included!
+
+#--- test2/include/y/c.h
+#error 'test2/include/y/c.h' should not have been included!
+
+#--- test2/include/y/d.h
+#error 'test2/include/y/d.h' should not have been included!
+
+#--- test2/include/y/e.h
+
+#--- test2/include/z/a.h
+#error 'test2/include/z/a.h' should not have been included!
+
+#--- test2/include/z/b.h
+#error 'test2/include/z/b.h' should not have been included!
+
+#--- test2/include/z/c.h
+#error 'test2/include/z/c.h' should not have been included!
+
+#--- test2/include/z/d.h
+#error 'test2/include/z/d.h' should not have been included!
+
+#--- test2/include/z/e.h
+#error 'test2/include/z/e.h' should not have been included!
+
+#--- test2/include/y/f.h
+
+
+// Test 3: Validate ordering and duplicate elimination across the Angled and
+// System groups.
+//
+// RUN: %clang \
+// RUN:     -target x86_64-unknown-linux-gnu -v -fsyntax-only \
+// RUN:     -nostdinc \
+// RUN:     -I%t/test3/include/y \
+// RUN:     -iexternal %t/test3/include/u \
+// RUN:     -I%t/test3/include/v \
+// RUN:     -isystem %t/test3/include/y \
+// RUN:     -iexternal %t/test3/include/w \
+// RUN:     -isystem %t/test3/include/z \
+// RUN:     -I%t/test3/include/x \
+// RUN:     -isystem %t/test3/include/u \
+// RUN:     -iexternal %t/test3/include/x \
+// RUN:     %t/test3/t.c 2>&1 | FileCheck -DPWD=%t %t/test3/t.c
+
+#--- test3/t.c
+#include <a.h>
+#include <b.h>
+#include <c.h>
+#include <d.h>
+#include <e.h>
+#include <f.h>
+
+// CHECK:      ignoring duplicate directory "[[PWD]]/test3/include/x"
+// CHECK-NEXT:  as it is a non-system directory that duplicates a system directory
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test3/include/y"
+// CHECK-NEXT:  as it is a non-system directory that duplicates a system directory
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test3/include/u"
+// CHECK-NEXT: #include "..." search starts here:
+// CHECK-NEXT: #include <...> search starts here:
+// CHECK-NEXT: [[PWD]]/test3/include/u
+// CHECK-NEXT: [[PWD]]/test3/include/v
+// CHECK-NEXT: [[PWD]]/test3/include/w
+// CHECK-NEXT: [[PWD]]/test3/include/x
+// CHECK-NEXT: [[PWD]]/test3/include/y
+// CHECK-NEXT: [[PWD]]/test3/include/z
+// CHECK-NEXT: End of search list.
+
+#--- test3/include/u/a.h
+
+#--- test3/include/v/a.h
+#error 'test3/include/v/a.h' should not have been included!
+
+#--- test3/include/v/b.h
+
+#--- test3/include/w/a.h
+#error 'test3/include/w/a.h' should not have been included!
+
+#--- test3/include/w/b.h
+#error 'test3/include/w/b.h' should not have been included!
+
+#--- test3/include/w/c.h
+
+#--- test3/include/x/a.h
+#error 'test3/include/x/a.h' should not have been included!
+
+#--- test3/include/x/b.h
+#error 'test3/include/x/b.h' should not have been included!
+
+#--- test3/include/x/c.h
+#error 'test3/include/x/c.h' should not have been included!
+
+#--- test3/include/x/d.h
+
+#--- test3/include/y/a.h
+#error 'test3/include/y/a.h' should not have been included!
+
+#--- test3/include/y/b.h
+#error 'test3/include/y/b.h' should not have been included!
+
+#--- test3/include/y/c.h
+#error 'test3/include/y/c.h' should not have been included!
+
+#--- test3/include/y/d.h
+#error 'test3/include/y/d.h' should not have been included!
+
+#--- test3/include/y/e.h
+
+#--- test3/include/z/a.h
+#error 'test3/include/z/a.h' should not have been included!
+
+#--- test3/include/z/b.h
+#error 'test3/include/z/b.h' should not have been included!
+
+#--- test3/include/z/c.h
+#error 'test3/include/z/c.h' should not have been included!
+
+#--- test3/include/z/d.h
+#error 'test3/include/z/d.h' should not have been included!
+
+#--- test3/include/z/e.h
+#error 'test3/include/z/e.h' should not have been included!
+
+#--- test3/include/z/f.h
+
+
+// Test 4: Validate ordering and duplicate elimination across the Angled and
+// System groups.
+//
+// RUN: env EXTRA_INCLUDE="%t/test4/include/w" \
+// RUN: %clang \
+// RUN:     -target x86_64-unknown-linux-gnu -v -fsyntax-only \
+// RUN:     -nostdinc \
+// RUN:     -I%t/test4/include/z \
+// RUN:     -iexternal %t/test4/include/v \
+// RUN:     -iexternal-env=EXTRA_INCLUDE \
+// RUN:     -isystem %t/test4/include/x \
+// RUN:     -isystem %t/test4/include/y \
+// RUN:     -isystem %t/test4/include/x \
+// RUN:     -isystem %t/test4/include/w \
+// RUN:     -isystem %t/test4/include/v \
+// RUN:     -isystem %t/test4/include/z \
+// RUN:     %t/test4/t.c 2>&1 | FileCheck -DPWD=%t %t/test4/t.c
+
+#--- test4/t.c
+#include <a.h>
+#include <b.h>
+#include <c.h>
+#include <d.h>
+#include <e.h>
+
+// CHECK:      ignoring duplicate directory "[[PWD]]/test4/include/x"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test4/include/w"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test4/include/v"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test4/include/z"
+// CHECK-NEXT:  as it is a non-system directory that duplicates a system directory
+// CHECK-NEXT: #include "..." search starts here:
+// CHECK-NEXT: #include <...> search starts here:
+// CHECK-NEXT: [[PWD]]/test4/include/v
+// CHECK-NEXT: [[PWD]]/test4/include/w
+// CHECK-NEXT: [[PWD]]/test4/include/x
+// CHECK-NEXT: [[PWD]]/test4/include/y
+// CHECK-NEXT: [[PWD]]/test4/include/z
+// CHECK-NEXT: End of search list.
+
+#--- test4/include/v/a.h
+
+#--- test4/include/w/a.h
+#error 'test4/include/w/a.h' should not have been included!
+
+#--- test4/include/w/b.h
+
+#--- test4/include/x/a.h
+#error 'test4/include/x/a.h' should not have been included!
+
+#--- test4/include/x/b.h
+#error 'test4/include/x/b.h' should not have been included!
+
+#--- test4/include/x/c.h
+
+#--- test4/include/y/a.h
+#error 'test4/include/y/a.h' should not have been included!
+
+#--- test4/include/y/b.h
+#error 'test4/include/y/b.h' should not have been included!
+
+#--- test4/include/y/c.h
+#error 'test4/include/y/c.h' should not have been included!
+
+#--- test4/include/y/d.h
+
+#--- test4/include/z/a.h
+#error 'test4/include/z/a.h' should not have been included!
+
+#--- test4/include/z/b.h
+#error 'test4/include/z/b.h' should not have been included!
+
+#--- test4/include/z/c.h
+#error 'test4/include/z/c.h' should not have been included!
+
+#--- test4/include/z/d.h
+#error 'test4/include/z/d.h' should not have been included!
+
+#--- test4/include/z/e.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..5a30e3be21ef36
--- /dev/null
+++ b/clang/test/Driver/microsoft-header-search-duplicates.c
@@ -0,0 +1,363 @@
+// Test that pruning of header search paths emulates MSVC behavior when in
+// Microsoft compatibility mode. See header-search-duplicates.c for GCC
+// compatible behavior.
+
+// This test is intended to be usable to validate MSVC behavior using a .bat
+// test driver similar to the following. A failure to compile successfully
+// would indicate a problem with the test or, perhaps, a behavioral difference
+// across MSVC versions.
+//   @echo on
+//   setlocal
+//   rd /s/q test-msvc
+//   split-file microsoft-header-search-duplicates.c test-msvc
+//   pushd test-msvc
+//   REM Validate test 1:
+//   set INCLUDE=...
+//   set EXTERNAL_INCLUDE=...
+//   set EXTRA_INCLUDE=...
+//   cl.exe /c /showIncludes ... test1.c
+//   popd
+
+// This test exercises both the clang and clang-cl drivers using a target that
+// implicitly enables the '-fms-compatibility' option. The '-nobuiltininc',
+// '-nostdinc', and '/X ('-nostdlibinc') options are used to suppress implicit
+// header search paths to ease testing.
+
+// 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 step 1, 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 a path from step 1, step 2, or an earlier
+//    path in the 'INCLUDE' environment variable are ignored.
+// 4) Paths specified by the 'EXTERNALINCLUDE' environment variable are
+//    processed in order. Paths that duplicate a path from step 1, step 2,
+//    step 3, or an earlier path in the 'EXTERNAL_INCLUDE' environment
+//    variable are ignored.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+
+// Test 1: Validate ordering and duplicate elimination for /I.
+//
+// RUN: %clang \
+// RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -nostdinc \
+// RUN:     -I%t/test1/include/y \
+// RUN:     -I%t/test1/include/z \
+// RUN:     -I%t/test1/include/y \
+// RUN:     %t/test1/t.c 2>&1 | FileCheck -DPWD=%t %t/test1/t.c
+// RUN: %clang_cl \
+// RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -nobuiltininc /X \
+// RUN:     /I%t/test1/include/y \
+// RUN:     /I%t/test1/include/z \
+// RUN:     /I%t/test1/include/y \
+// RUN:     %t/test1/t.c 2>&1 | FileCheck -DPWD=%t %t/test1/t.c
+
+#--- test1/t.c
+#include <a.h>
+#include <b.h>
+
+// CHECK:      ignoring duplicate directory "[[PWD]]/test1/include/y"
+// CHECK-NEXT: #include "..." search starts here:
+// CHECK-NEXT: #include <...> search starts here:
+// CHECK-NEXT: [[PWD]]/test1/include/y
+// CHECK-NEXT: [[PWD]]/test1/include/z
+// CHECK-NEXT: End of search list.
+
+#--- test1/include/y/a.h
+
+#--- test1/include/z/a.h
+#error 'test1/include/z/a.h' should not have been included!
+
+#--- test1/include/z/b.h
+
+
+// Test 2: Validate ordering and duplicate elimination for /external:I.
+//
+// RUN: %clang \
+// RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -nostdinc \
+// RUN:     -iexternal %t/test2/include/z \
+// RUN:     -iexternal %t/test2/include/y \
+// RUN:     -iexternal %t/test2/include/z \
+// RUN:     %t/test2/t.c 2>&1 | FileCheck -DPWD=%t %t/test2/t.c
+// RUN: %clang_cl \
+// RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -nobuiltininc /X \
+// RUN:     /external:I %t/test2/include/z \
+// RUN:     /external:I %t/test2/include/y \
+// RUN:     /external:I %t/test2/include/z \
+// RUN:     %t/test2/t.c 2>&1 | FileCheck -DPWD=%t %t/test2/t.c
+
+#--- test2/t.c
+#include <a.h>
+#include <b.h>
+
+// CHECK:      ignoring duplicate directory "[[PWD]]/test2/include/z"
+// CHECK-NEXT: #include "..." search starts here:
+// CHECK-NEXT: #include <...> search starts here:
+// CHECK-NEXT: [[PWD]]/test2/include/y
+// CHECK-NEXT: [[PWD]]/test2/include/z
+// CHECK-NEXT: End of search list.
+
+#--- test2/include/y/a.h
+
+#--- test2/include/z/a.h
+#error 'test2/include/z/a.h' should not have been included!
+
+#--- test2/include/z/b.h
+
+
+// Test 3: Validate ordering and duplicate elimination for /I vs /external:I.
+//
+// RUN: %clang \
+// RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -nostdinc \
+// RUN:     -iexternal %t/test3/include/w \
+// RUN:     -I%t/test3/include/z \
+// RUN:     -I%t/test3/include/x \
+// RUN:     -I%t/test3/include/w \
+// RUN:     -iexternal %t/test3/include/y \
+// RUN:     -iexternal %t/test3/include/z \
+// RUN:     %t/test3/t.c 2>&1 | FileCheck -DPWD=%t %t/test3/t.c
+// RUN: %clang_cl \
+// RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -nobuiltininc /X \
+// RUN:     /external:I %t/test3/include/w \
+// RUN:     /I%t/test3/include/z \
+// RUN:     /I%t/test3/include/x \
+// RUN:     /I%t/test3/include/w \
+// RUN:     /external:I %t/test3/include/y \
+// RUN:     /external:I %t/test3/include/z \
+// RUN:     %t/test3/t.c 2>&1 | FileCheck -DPWD=%t %t/test3/t.c
+
+#--- test3/t.c
+#include <a.h>
+#include <b.h>
+#include <c.h>
+#include <d.h>
+
+// CHECK:      ignoring duplicate directory "[[PWD]]/test3/include/w"
+// CHECK-NEXT:  as it is a non-system directory that duplicates a system directory
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test3/include/z"
+// CHECK-NEXT:  as it is a non-system directory that duplicates a system directory
+// CHECK-NEXT: #include "..." search starts here:
+// CHECK-NEXT: #include <...> search starts here:
+// CHECK-NEXT: [[PWD]]/test3/include/w
+// CHECK-NEXT: [[PWD]]/test3/include/x
+// CHECK-NEXT: [[PWD]]/test3/include/y
+// CHECK-NEXT: [[PWD]]/test3/include/z
+// CHECK-NEXT: End of search list.
+
+#--- test3/include/w/a.h
+
+#--- test3/include/x/a.h
+#error 'test3/include/x/a.h' should not have been included!
+
+#--- test3/include/x/b.h
+
+#--- test3/include/y/a.h
+#error 'test3/include/y/a.h' should not have been included!
+
+#--- test3/include/y/b.h
+#error 'test3/include/y/b.h' should not have been included!
+
+#--- test3/include/y/c.h
+
+#--- test3/include/z/a.h
+#error 'test3/include/z/a.h' should not have been included!
+
+#--- test3/include/z/b.h
+#error 'test3/include/z/b.h' should not have been included!
+
+#--- test3/include/z/c.h
+#error 'test3/include/z/c.h' should not have been included!
+
+#--- test3/include/z/d.h
+
+
+// Test 4: Validate ordering and duplicate elimination for /external:env.
+//
+// RUN: env EXTRA_INCLUDE1="%t/test4/include/y" \
+// RUN: env EXTRA_INCLUDE2="%t/test4/include/z;%t/test4/include/y;%t/test4/include/x;%t/test4/include/w" \
+// RUN: %clang \
+// RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -nostdinc \
+// RUN:     -I%t/test4/include/w \
+// RUN:     -iexternal %t/test4/include/x \
+// RUN:     -iexternal-env=EXTRA_INCLUDE1 \
+// RUN:     -iexternal-env=EXTRA_INCLUDE2 \
+// RUN:     %t/test4/t.c 2>&1 | FileCheck -DPWD=%t %t/test4/t.c
+// RUN: env EXTRA_INCLUDE1="%t/test4/include/y" \
+// RUN: env EXTRA_INCLUDE2="%t/test4/include/z;%t/test4/include/y;%t/test4/include/x;%t/test4/include/w" \
+// RUN: %clang_cl \
+// RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -nobuiltininc /X \
+// RUN:     /I%t/test4/include/w \
+// RUN:     /external:I %t/test4/include/x \
+// RUN:     /external:env:EXTRA_INCLUDE1 \
+// RUN:     /external:env:EXTRA_INCLUDE2 \
+// RUN:     %t/test4/t.c 2>&1 | FileCheck -DPWD=%t %t/test4/t.c
+
+#--- test4/t.c
+#include <a.h>
+#include <b.h>
+#include <c.h>
+#include <d.h>
+
+// CHECK:      ignoring duplicate directory "[[PWD]]/test4/include/y"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test4/include/x"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test4/include/w"
+// CHECK-NEXT: #include "..." search starts here:
+// CHECK-NEXT: #include <...> search starts here:
+// CHECK-NEXT: [[PWD]]/test4/include/w
+// CHECK-NEXT: [[PWD]]/test4/include/x
+// CHECK-NEXT: [[PWD]]/test4/include/y
+// CHECK-NEXT: [[PWD]]/test4/include/z
+// CHECK-NEXT: End of search list.
+
+#--- test4/include/w/a.h
+
+#--- test4/include/x/a.h
+#error 'test4/include/x/a.h' should not have been included!
+
+#--- test4/include/x/b.h
+
+#--- test4/include/y/a.h
+#error 'test4/include/y/a.h' should not have been included!
+
+#--- test4/include/y/b.h
+#error 'test4/include/y/b.h' should not have been included!
+
+#--- test4/include/y/c.h
+
+#--- test4/include/z/a.h
+#error 'test4/include/z/a.h' should not have been included!
+
+#--- test4/include/z/b.h
+#error 'test4/include/z/b.h' should not have been included!
+
+#--- test4/include/z/c.h
+#error 'test4/include/z/c.h' should not have been included!
+
+#--- test4/include/z/d.h
+
+
+// Test 5: Validate ordering and duplicate elimination for the INCLUDE and
+// EXTERNAL_INCLUDE environment variables.
+//
+// RUN: env EXTRA_INCLUDE="%t/test5/include/w" \
+// RUN: env INCLUDE="%t/test5/include/x;%t/test5/include/y;%t/test5/include/w;%t/test5/include/v;%t/test5/include/u" \
+// RUN: env EXTERNAL_INCLUDE="%t/test5/include/z;%t/test5/include/y;%t/test5/include/w;%t/test5/include/v;%t/test5/include/u" \
+// RUN: %clang \
+// RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -nostdinc \
+// RUN:     -I%t/test5/include/u \
+// RUN:     -iexternal %t/test5/include/v \
+// RUN:     -iexternal-env=EXTRA_INCLUDE \
+// RUN:     -iexternal-env=INCLUDE \
+// RUN:     -iexternal-env=EXTERNAL_INCLUDE \
+// RUN:     %t/test5/t.c 2>&1 | FileCheck -DPWD=%t %t/test5/t.c
+// RUN: env EXTRA_INCLUDE="%t/test5/include/w" \
+// RUN: env INCLUDE="%t/test5/include/x;%t/test5/include/y;%t/test5/include/w;%t/test5/include/v;%t/test5/include/u" \
+// RUN: env EXTERNAL_INCLUDE="%t/test5/include/z;%t/test5/include/y;%t/test5/include/w;%t/test5/include/v;%t/test5/include/u" \
+// RUN: %clang_cl \
+// RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -nobuiltininc \
+// RUN:     /I%t/test5/include/u \
+// RUN:     /external:I %t/test5/include/v \
+// RUN:     /external:env:EXTRA_INCLUDE \
+// RUN:     %t/test5/t.c 2>&1 | FileCheck -DPWD=%t %t/test5/t.c
+
+#--- test5/t.c
+#include <a.h>
+#include <b.h>
+#include <c.h>
+#include <d.h>
+#include <e.h>
+#include <f.h>
+
+// CHECK:      ignoring duplicate directory "[[PWD]]/test5/include/w"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test5/include/v"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test5/include/u"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test5/include/y"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test5/include/w"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test5/include/v"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test5/include/u"
+// CHECK-NEXT: #include "..." search starts here:
+// CHECK-NEXT: #include <...> search starts here:
+// CHECK-NEXT: [[PWD]]/test5/include/u
+// CHECK-NEXT: [[PWD]]/test5/include/v
+// CHECK-NEXT: [[PWD]]/test5/include/w
+// CHECK-NEXT: [[PWD]]/test5/include/x
+// CHECK-NEXT: [[PWD]]/test5/include/y
+// CHECK-NEXT: [[PWD]]/test5/include/z
+// CHECK-NEXT: End of search list.
+
+#--- test5/include/u/a.h
+
+#--- test5/include/v/a.h
+#error 'test5/include/v/a.h' should not have been included!
+
+#--- test5/include/v/b.h
+
+#--- test5/include/w/a.h
+#error 'test5/include/w/a.h' should not have been included!
+
+#--- test5/include/w/b.h
+#error 'test5/include/w/b.h' should not have been included!
+
+#--- test5/include/w/c.h
+
+#--- test5/include/x/a.h
+#error 'test5/include/x/a.h' should not have been included!
+
+#--- test5/include/x/b.h
+#error 'test5/include/x/b.h' should not have been included!
+
+#--- test5/include/x/c.h
+#error 'test5/include/x/c.h' should not have been included!
+
+#--- test5/include/x/d.h
+
+#--- test5/include/y/a.h
+#error 'test5/include/y/a.h' should not have been included!
+
+#--- test5/include/y/b.h
+#error 'test5/include/y/b.h' should not have been included!
+
+#--- test5/include/y/c.h
+#error 'test5/include/y/c.h' should not have been included!
+
+#--- test5/include/y/d.h
+#error 'test5/include/y/d.h' should not have been included!
+
+#--- test5/include/y/e.h
+
+#--- test5/include/z/a.h
+#error 'test5/include/z/a.h' should not have been included!
+
+#--- test5/include/z/b.h
+#error 'test5/include/z/b.h' should not have been included!
+
+#--- test5/include/z/c.h
+#error 'test5/include/z/c.h' should not have been included!
+
+#--- test5/include/z/d.h
+#error 'test5/include/z/d.h' should not have been included!
+
+#--- test5/include/z/e.h
+#error 'test5/include/z/e.h' should not have been included!
+
+#--- test5/include/z/f.h
diff --git a/llvm/include/llvm/Option/ArgList.h b/llvm/include/llvm/Option/ArgList.h
index 09812f976d0166..77703910b3d26b 100644
--- a/llvm/include/llvm/Option/ArgList.h
+++ b/llvm/include/llvm/Option/ArgList.h
@@ -288,7 +288,12 @@ class ArgList {
 
   /// getAllArgValues - Get the values of all instances of the given argument
   /// as strings.
-  std::vector<std::string> getAllArgValues(OptSpecifier Id) const;
+  template <typename... OptSpecifiers>
+  std::vector<std::string> 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 6e164150d2e5e9..3bbff6c75e78aa 100644
--- a/llvm/lib/Option/ArgList.cpp
+++ b/llvm/lib/Option/ArgList.cpp
@@ -95,12 +95,6 @@ StringRef ArgList::getLastArgValue(OptSpecifier Id, StringRef Default) const {
   return Default;
 }
 
-std::vector<std::string> ArgList::getAllArgValues(OptSpecifier Id) const {
-  SmallVector<const char *, 16> Values;
-  AddAllArgValues(Values, Id);
-  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))

>From cd82a68083feb53a17b709635b4e66eea80b62d3 Mon Sep 17 00:00:00 2001
From: Tom Honermann <tom.honermann at intel.com>
Date: Tue, 10 Dec 2024 14:20:08 -0800
Subject: [PATCH 2/4] Make header search path behavior determined by driver
 used rather than `-fms-compatibility`.

Use of `-fms-compatibility` to opt-in to Microsoft header search behavior was
found to cause incompatibility problems with some projects that use the GCC
compatible driver on Windows. This change introduces a new `-fheader-search`
option to control the behavior. Valid values for the option include `gcc` and
`microsoft`. The `clang-cl` driver uses `-fheader-search=microsoft` by default;
all other drivers continue to use `-fheader-search=gcc` by default.

This change affects backward compatibility. The `-fheader-search=` option can
be used to opt in to previous behavior with some caveats. Clang 19 and earlier
followed the GCC header search behavior except when searching for header files
included by quoted inclusion (`#include "file.h"`) when `-fms-compatibility`
is specified; in that case, Microsoft's behavior of searching for the header
file in the directories of each file in the include stack was used. An option
to control this specific behavior is not provided, but could be added if found
to be necessary or useful.
---
 clang/docs/ReleaseNotes.rst                   | 13 +++--
 clang/docs/UsersManual.rst                    | 49 ++++++++++++++-----
 clang/include/clang/Driver/Options.td         | 11 +++++
 clang/include/clang/Lex/HeaderSearchOptions.h |  8 +++
 clang/lib/Lex/InitHeaderSearch.cpp            | 19 +++----
 clang/lib/Lex/PPDirectives.cpp                |  5 +-
 clang/lib/Serialization/ASTReader.cpp         |  2 +-
 clang/lib/Serialization/ASTWriter.cpp         |  1 +
 clang/test/Driver/header-search-duplicates.c  |  5 +-
 .../microsoft-header-search-duplicates.c      | 29 +++--------
 .../microsoft-header-search-fail.c            |  2 +-
 .../Preprocessor/microsoft-header-search.c    |  2 +-
 clang/tools/driver/driver.cpp                 |  3 ++
 13 files changed, 96 insertions(+), 53 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ec3edc8c940301..2d1074e3226ce3 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -945,16 +945,17 @@ Windows Support
   allowed as VS2013 and prior allow it.
 
 - Clang now matches MSVC behavior regarding the 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
+  search paths when run via the ``clang-cl`` driver (by default) or with the
+  ``-fheader-search=microsoft`` option otherwise. 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. This ordering is not compatible with
   the ordering that MSVC uses when paths are duplicated across ``/I`` options
   and the ``INCLUDE`` environment variable.
 
   The order that MSVC uses and that Clang now replicates when the
-  ``-fms-compatibility`` option is enabled follows.
+  ``-fheader-search=microsoft`` option is enabled follows.
 
   - Paths specified by the ``/I`` and ``/external:I`` options are processed in
     the order that they appear. Paths specified by ``/I`` that duplicate a path
@@ -982,6 +983,10 @@ Windows Support
     Paths that duplicate an earlier path in the ``EXTERNAL_INCLUDE``
     environment variable are ignored.
 
+  The ``-fheader-search=gcc`` option can be used to opt in to GCC duplicate
+  header search path handling (which remains the default behavior for the GCC
+  compatible drivers).
+
 LoongArch Support
 ^^^^^^^^^^^^^^^^^
 
diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 8af9f5be644a02..609a3fa82fa482 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -5166,12 +5166,26 @@ follows:
 
 2. Consult the environment.
 
-    TODO: This is not yet implemented.
+    - `/external:env:[VARIABLE]`
 
-    This will consult the environment variables:
+      This command line option specifies a user identified environment variable
+      which is treated as a path delimited (`;`) list of system header paths.
 
-    - `WindowsSdkDir`
-    - `UCRTVersion`
+    - `INCLUDE`
+
+      This environment variable is treated as a path delimited (`;`) list of
+      system header paths.
+
+    - `EXTERNAL_INCLUDE`
+
+      This environment variable is treated as a path delimited (`;`) list of
+      system header paths.
+
+    The following environment variables will be consulted and used to form paths
+    to validate and load content from as appropriate:
+
+      - `WindowsSdkDir`
+      - `UCRTVersion`
 
 3. Fallback to the registry.
 
@@ -5183,6 +5197,8 @@ The Visual C++ Toolset has a slightly more elaborate mechanism for detection.
 
 1. Consult the command line.
 
+    Anything the user specifies is always given precedence.
+
     - `/winsysroot:`
 
     The `/winsysroot:` is used as an equivalent to `-sysroot` on Unix
@@ -5200,21 +5216,30 @@ The Visual C++ Toolset has a slightly more elaborate mechanism for detection.
 
 2. Consult the environment.
 
-    - `/external:[VARIABLE]`
+    - `/external:env:[VARIABLE]`
+
+      This command line option specifies a user identified environment variable
+      which is treated as a path delimited (`;`) list of external header search
+      paths. Additionally, any header search path provided by other means that
+      has a prefix that matches one of these paths is treated as an external
+      header search path.
+
+    - `INCLUDE`
 
-      This specifies a user identified environment variable which is treated as
-      a path delimiter (`;`) separated list of paths to map into `-imsvc`
-      arguments which are treated as `-isystem`.
+      This environment variable is treated as a path delimited (`;`) list of
+      header search paths.
 
-    - `INCLUDE` and `EXTERNAL_INCLUDE`
+    - `EXTERNAL_INCLUDE`
 
-      The path delimiter (`;`) separated list of paths will be mapped to
-      `-imsvc` arguments which are treated as `-isystem`.
+      This environment variable is treated as a path delimited (`;`) list of
+      external header search paths. Additionally, any header search path
+      provided by other means that has a prefix that matches one of these paths
+      is treated as an external header search path.
 
     - `LIB` (indirectly)
 
       The linker `link.exe` or `lld-link.exe` will honour the environment
-      variable `LIB` which is a path delimiter (`;`) set of paths to consult for
+      variable `LIB` which is a path delimited (`;`) set of paths to consult for
       the import libraries to use when linking the final target.
 
     The following environment variables will be consulted and used to form paths
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index bf84c7fc71b764..0d4654e4f14dfb 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -8240,6 +8240,17 @@ def fexperimental_max_bitint_width_EQ:
 // Header Search Options
 //===----------------------------------------------------------------------===//
 
+let Visibility = [ClangOption, CC1Option, CLOption] in {
+
+def fheader_search : Joined<["-"], "fheader-search=">, Group<clang_i_Group>,
+  HelpText<"Specify the method used to resolve included header files">,
+  Values<"gcc,microsoft">,
+  NormalizedValuesScope<"clang::HeaderSearchMode">,
+  NormalizedValues<["GCC", "Microsoft"]>,
+  MarshallingInfoEnum<HeaderSearchOpts<"Mode">, "GCC">;
+
+} // let Visibility = [ClangOption, CC1Option, CLOption]
+
 let Visibility = [CC1Option] in {
 
 def nostdsysteminc : Flag<["-"], "nostdsysteminc">,
diff --git a/clang/include/clang/Lex/HeaderSearchOptions.h b/clang/include/clang/Lex/HeaderSearchOptions.h
index 0aa0e533b1ded8..5c18c6c8a397e2 100644
--- a/clang/include/clang/Lex/HeaderSearchOptions.h
+++ b/clang/include/clang/Lex/HeaderSearchOptions.h
@@ -69,6 +69,11 @@ enum IncludeDirGroup {
 
 } // namespace frontend
 
+/// HeaderSearchMode - The method used to resolve included headers to files.
+/// This controls the order in which include paths are searched and how
+/// duplicate search paths are handled.
+enum class HeaderSearchMode { GCC, Microsoft };
+
 /// HeaderSearchOptions - Helper class for storing options related to the
 /// initialization of the HeaderSearch object.
 class HeaderSearchOptions {
@@ -103,6 +108,9 @@ class HeaderSearchOptions {
         : Prefix(Prefix), IsSystemHeader(IsSystemHeader) {}
   };
 
+  /// The header search mode to use.
+  HeaderSearchMode Mode = HeaderSearchMode::GCC;
+
   /// If non-empty, the directory to use as a "virtual system root" for include
   /// paths.
   std::string Sysroot;
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp b/clang/lib/Lex/InitHeaderSearch.cpp
index 54c7195f6ee0a9..5bb5c6e3addb09 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -98,7 +98,7 @@ class InitHeaderSearch {
                               const HeaderSearchOptions &HSOpts);
 
   /// Merges all search path lists into one list and send it to HeaderSearch.
-  void Realize(const LangOptions &Lang);
+  void Realize(const HeaderSearchOptions &HSOpts, const LangOptions &Lang);
 };
 
 }  // end anonymous namespace.
@@ -369,7 +369,7 @@ void InitHeaderSearch::AddDefaultIncludePaths(
 /// and system search paths. If partitioning is not needed, then call with
 /// Part1Begin equal to Part2Begin. The return value is the number of items
 /// removed from the first partition.
-static unsigned RemoveDuplicates(const LangOptions &Lang,
+static unsigned RemoveDuplicates(const HeaderSearchOptions &HSOpts,
                                  std::vector<DirectoryLookupInfo> &SearchList,
                                  unsigned Part1Begin, unsigned Part2Begin,
                                  bool Verbose) {
@@ -430,8 +430,8 @@ static unsigned RemoveDuplicates(const LangOptions &Lang,
       // A path that matches a later path specified by -iexternal is always
       // suppressed.
       DirToRemove = PrevIndex;
-    } else if (!Lang.MSVCCompat && PrevSrcKind == SrcMgr::C_User &&
-               CurSrcKind != SrcMgr::C_User) {
+    } else if (HSOpts.Mode != HeaderSearchMode::Microsoft &&
+               PrevSrcKind == SrcMgr::C_User && CurSrcKind != SrcMgr::C_User) {
       // When not in Microsoft compatibility mode, a user path that matches
       // a later system path is suppressed.
       DirToRemove = PrevIndex;
@@ -489,7 +489,8 @@ mapToUserEntries(const std::vector<DirectoryLookupInfo> &Infos) {
   return LookupsToUserEntries;
 }
 
-void InitHeaderSearch::Realize(const LangOptions &Lang) {
+void InitHeaderSearch::Realize(const HeaderSearchOptions &HSOpts,
+                               const LangOptions &Lang) {
   // Concatenate ANGLE+SYSTEM+AFTER chains together into SearchList.
   std::vector<DirectoryLookupInfo> SearchList;
   SearchList.reserve(IncludePath.size());
@@ -499,7 +500,7 @@ void InitHeaderSearch::Realize(const LangOptions &Lang) {
     if (Include.Group == Quoted)
       SearchList.push_back(Include);
   // Remove duplicate search paths within the quoted inclusion list.
-  RemoveDuplicates(Lang, SearchList, 0, 0, Verbose);
+  RemoveDuplicates(HSOpts, SearchList, 0, 0, Verbose);
   unsigned EndQuoted = SearchList.size();
 
   // Add search paths for angled inclusion next. Note that user paths and
@@ -516,7 +517,7 @@ void InitHeaderSearch::Realize(const LangOptions &Lang) {
   // Remove duplicate search paths within the angled inclusion list.
   // This may leave paths duplicated across the quoted and angled inclusion
   // sections.
-  RemoveDuplicates(Lang, SearchList, EndQuoted, EndQuoted, Verbose);
+  RemoveDuplicates(HSOpts, SearchList, EndQuoted, EndQuoted, Verbose);
   unsigned EndAngled = SearchList.size();
 
   // Add search paths for language dependent system paths next.
@@ -539,7 +540,7 @@ void InitHeaderSearch::Realize(const LangOptions &Lang) {
   // same file. This may result in earlier user paths being removed, and thus
   // requires updating the EndAngled index.
   unsigned NonSystemRemoved =
-      RemoveDuplicates(Lang, SearchList, EndQuoted, EndAngled, Verbose);
+      RemoveDuplicates(HSOpts, SearchList, EndQuoted, EndAngled, Verbose);
   EndAngled -= NonSystemRemoved;
 
   Headers.SetSearchPaths(extractLookups(SearchList), EndQuoted, EndAngled,
@@ -599,5 +600,5 @@ void clang::ApplyHeaderSearchOptions(HeaderSearch &HS,
       HS.getModuleMap().setBuiltinIncludeDir(*Dir);
   }
 
-  Init.Realize(Lang);
+  Init.Realize(HSOpts, Lang);
 }
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index a23ad40884f249..bd4b2f6464a3e2 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -23,6 +23,7 @@
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/CodeCompletionHandler.h"
 #include "clang/Lex/HeaderSearch.h"
+#include "clang/Lex/HeaderSearchOptions.h"
 #include "clang/Lex/LexDiagnostic.h"
 #include "clang/Lex/LiteralSupport.h"
 #include "clang/Lex/MacroInfo.h"
@@ -999,7 +1000,9 @@ OptionalFileEntryRef Preprocessor::LookupFile(
     // MSVC searches the current include stack from top to bottom for
     // headers included by quoted include directives.
     // See: http://msdn.microsoft.com/en-us/library/36k2cdd4.aspx
-    if (LangOpts.MSVCCompat && !isAngled) {
+    if (getHeaderSearchInfo().getHeaderSearchOpts().Mode ==
+            HeaderSearchMode::Microsoft &&
+        !isAngled) {
       for (IncludeStackInfo &ISEntry : llvm::reverse(IncludeMacroStack)) {
         if (IsFileLexer(ISEntry))
           if ((FileEnt = ISEntry.ThePPLexer->getFileEntry()))
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index ec85fad3389a1c..cfa236879982bf 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -6166,8 +6166,8 @@ bool ASTReader::ParseHeaderSearchOptions(const RecordData &Record,
                                          ASTReaderListener &Listener) {
   HeaderSearchOptions HSOpts;
   unsigned Idx = 0;
+  HSOpts.Mode = static_cast<HeaderSearchMode>(Record[Idx++]);
   HSOpts.Sysroot = ReadString(Record, Idx);
-
   HSOpts.ResourceDir = ReadString(Record, Idx);
   HSOpts.ModuleCachePath = ReadString(Record, Idx);
   HSOpts.ModuleUserBuildPath = ReadString(Record, Idx);
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 368e8f23675433..d551cb136a69c8 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1673,6 +1673,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) {
   const HeaderSearchOptions &HSOpts =
       PP.getHeaderSearchInfo().getHeaderSearchOpts();
 
+  Record.push_back(static_cast<unsigned>(HSOpts.Mode));
   AddString(HSOpts.Sysroot, Record);
   AddString(HSOpts.ResourceDir, Record);
   AddString(HSOpts.ModuleCachePath, Record);
diff --git a/clang/test/Driver/header-search-duplicates.c b/clang/test/Driver/header-search-duplicates.c
index b539bcdf18e2e7..9a96566e331ca8 100644
--- a/clang/test/Driver/header-search-duplicates.c
+++ b/clang/test/Driver/header-search-duplicates.c
@@ -5,9 +5,8 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
 
-// This test exercises the clang driver using a target that does not implicitly
-// enable the -fms-compatibility option. The -nostdinc option is used to
-// suppress default search paths to ease testing.
+// This test uses the -nostdinc option to suppress default search paths to
+// ease testing.
 
 // Header search paths are categorized into the following general groups.
 // - Quoted: Search paths that are only used to resolve inclusion of header
diff --git a/clang/test/Driver/microsoft-header-search-duplicates.c b/clang/test/Driver/microsoft-header-search-duplicates.c
index 5a30e3be21ef36..16f3eb3f0f9864 100644
--- a/clang/test/Driver/microsoft-header-search-duplicates.c
+++ b/clang/test/Driver/microsoft-header-search-duplicates.c
@@ -2,26 +2,8 @@
 // Microsoft compatibility mode. See header-search-duplicates.c for GCC
 // compatible behavior.
 
-// This test is intended to be usable to validate MSVC behavior using a .bat
-// test driver similar to the following. A failure to compile successfully
-// would indicate a problem with the test or, perhaps, a behavioral difference
-// across MSVC versions.
-//   @echo on
-//   setlocal
-//   rd /s/q test-msvc
-//   split-file microsoft-header-search-duplicates.c test-msvc
-//   pushd test-msvc
-//   REM Validate test 1:
-//   set INCLUDE=...
-//   set EXTERNAL_INCLUDE=...
-//   set EXTRA_INCLUDE=...
-//   cl.exe /c /showIncludes ... test1.c
-//   popd
-
-// This test exercises both the clang and clang-cl drivers using a target that
-// implicitly enables the '-fms-compatibility' option. The '-nobuiltininc',
-// '-nostdinc', and '/X ('-nostdlibinc') options are used to suppress implicit
-// header search paths to ease testing.
+// This test uses the '-nobuiltininc', '-nostdinc', and '/X ('-nostdlibinc')
+// options to suppress implicit header search paths to ease testing.
 
 // Header search paths are processed as follows:
 // 1) Paths specified by the '/I' and '/external:I' options are processed in
@@ -38,7 +20,7 @@
 // 3) Paths specified by the 'INCLUDE' environment variable are processed in
 //    order. Paths that duplicate a path from step 1, step 2, or an earlier
 //    path in the 'INCLUDE' environment variable are ignored.
-// 4) Paths specified by the 'EXTERNALINCLUDE' environment variable are
+// 4) Paths specified by the 'EXTERNAL_INCLUDE' environment variable are
 //    processed in order. Paths that duplicate a path from step 1, step 2,
 //    step 3, or an earlier path in the 'EXTERNAL_INCLUDE' environment
 //    variable are ignored.
@@ -51,6 +33,7 @@
 //
 // RUN: %clang \
 // RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -fheader-search=microsoft \
 // RUN:     -nostdinc \
 // RUN:     -I%t/test1/include/y \
 // RUN:     -I%t/test1/include/z \
@@ -87,6 +70,7 @@
 //
 // RUN: %clang \
 // RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -fheader-search=microsoft \
 // RUN:     -nostdinc \
 // RUN:     -iexternal %t/test2/include/z \
 // RUN:     -iexternal %t/test2/include/y \
@@ -123,6 +107,7 @@
 //
 // RUN: %clang \
 // RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -fheader-search=microsoft \
 // RUN:     -nostdinc \
 // RUN:     -iexternal %t/test3/include/w \
 // RUN:     -I%t/test3/include/z \
@@ -193,6 +178,7 @@
 // RUN: env EXTRA_INCLUDE2="%t/test4/include/z;%t/test4/include/y;%t/test4/include/x;%t/test4/include/w" \
 // RUN: %clang \
 // RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -fheader-search=microsoft \
 // RUN:     -nostdinc \
 // RUN:     -I%t/test4/include/w \
 // RUN:     -iexternal %t/test4/include/x \
@@ -262,6 +248,7 @@
 // RUN: env EXTERNAL_INCLUDE="%t/test5/include/z;%t/test5/include/y;%t/test5/include/w;%t/test5/include/v;%t/test5/include/u" \
 // RUN: %clang \
 // RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -fheader-search=microsoft \
 // RUN:     -nostdinc \
 // RUN:     -I%t/test5/include/u \
 // RUN:     -iexternal %t/test5/include/v \
diff --git a/clang/test/Preprocessor/microsoft-header-search-fail.c b/clang/test/Preprocessor/microsoft-header-search-fail.c
index c377cb11d658aa..9f8927974de939 100644
--- a/clang/test/Preprocessor/microsoft-header-search-fail.c
+++ b/clang/test/Preprocessor/microsoft-header-search-fail.c
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
 
-// RUN: %clang_cc1 -Eonly -fms-compatibility %t/test.c -I %t/include -verify
+// RUN: %clang_cc1 -Eonly -fheader-search=microsoft %t/test.c -I %t/include -verify
 
 //--- test.c
 #include "x/header.h"
diff --git a/clang/test/Preprocessor/microsoft-header-search.c b/clang/test/Preprocessor/microsoft-header-search.c
index 875bffe8793b8b..15fef1b0d184b4 100644
--- a/clang/test/Preprocessor/microsoft-header-search.c
+++ b/clang/test/Preprocessor/microsoft-header-search.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -I%S/Inputs/microsoft-header-search %s -fms-compatibility -verify
+// RUN: %clang_cc1 -I%S/Inputs/microsoft-header-search %s -fheader-search=microsoft -verify
 
 // expected-warning at Inputs/microsoft-header-search/a/findme.h:3 {{findme.h successfully included using Microsoft header search rules}}
 // expected-warning at Inputs/microsoft-header-search/a/b/include3.h:3 {{#include resolved using non-portable Microsoft search rules as}}
diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp
index 12038de476ace1..80a67e46195712 100644
--- a/clang/tools/driver/driver.cpp
+++ b/clang/tools/driver/driver.cpp
@@ -292,6 +292,9 @@ int clang_main(int Argc, char **Argv, const llvm::ToolContext &ToolContext) {
       // Insert at the end of the argument list to append.
       Args.append(AppendedOpts.begin(), AppendedOpts.end());
     }
+
+    // Header search path mode defaults to Microsoft.
+    Args.insert(Args.begin() + 1, "-fheader-search=microsoft");
   }
 
   llvm::StringSet<> SavedStrings;

>From 536a35492935638d278fcafdc71e4ac614b34046 Mon Sep 17 00:00:00 2001
From: Tom Honermann <tom.honermann at intel.com>
Date: Mon, 6 Jan 2025 18:05:46 -0800
Subject: [PATCH 3/4] Enable warning suppression for header files that match
 external include directories.

Internal Intel testing revealed that warning suppression for header files with
paths that match a directory specified by the `/external:I` or `/external:env`
options or a directory specified by the `EXTERNAL_INCLUDE` environment variable
is important to avoid failures in build systems that are sensitive to unexpected
warnings. This change implements such warning suppression.

There are two behaviors that MSVC exhibits that are not replicated with these
changes.
1) Clang ignores an external directory if the path does not match an existing
   directory. Since external directories may specify a prefix for path matching
   purposes, it is not necessary that a matching directory entry exist in the
   filesystem; it is only necessary that the path is a prefix match for some
   directory. Test 7 in clang/test/Driver/microsoft-header-search-duplicates.c
   demonstrates that Clang fails to perform warning suppression for such
   external directory prefixes.
2) Clang does not preserve trailing path separators for include paths. MSVC
   does preserve them and uses them when performing external directory prefix
   matches against header file paths; when present, the final path component
   must match a complete path component in the header file path, not just a
   prefix. Since Clang does not preserve trailing path separators, prefix
   matches can succeed when they should not. This behavior is exercised by
   test 8 in clang/test/Driver/microsoft-header-search-duplicates.c.

Previously, it was unclear/unspecified how external directory include paths
were ordered relative to system include paths in some cases. This change
clarifies behavior by ordering paths specified with the `/external:env` option
or by the `EXTERNAL_INCLUDE` environment variable with those specified by
`-isystem` and `-internal-isystem` respectively. As part of this change, some
options were renamed and others were introduced.
1) The `-iexternal-after` option was renamed to `-iexternal-system`.
2) A `-isystem-env=` option was added to allow the clang-cl behavior with
   respect to the `INCLUDE` environment variable to be modeled using the gcc
   compatible drivers. This option is equivalent to passing each directory
   present in `INCLUDE` via the `-isystem` option.
3) A `-internal-iexternal-system` CC1 option was added to enable behavior to
   be differentiated for the `/external:env` option and the very similar
   behavior implicitly enabled for support of the `EXTERNAL_INCLUDE` environment
   variable. For `/external:env`, each directory in the named environment
   variable is handled as though it was passed via `-iexternal-system`. For
   `EXTERNAL_INCLUDE`, each directory is handled as though it was passed via
   `-internal-iexternal-system`. This differentiated behavior is needed to
   maintain proper ordering of include paths with other implicitly added
   system include directories passed to CC1 via `-internal-isystem`.
---
 clang/include/clang/Driver/Options.td         |  12 +-
 clang/include/clang/Driver/ToolChain.h        |  21 +-
 clang/include/clang/Lex/HeaderSearch.h        |  12 +
 clang/include/clang/Lex/HeaderSearchOptions.h |  16 +-
 clang/lib/Driver/Driver.cpp                   |   3 +-
 clang/lib/Driver/ToolChain.cpp                |  41 ++-
 clang/lib/Driver/ToolChains/Clang.cpp         |   8 +-
 clang/lib/Driver/ToolChains/MSVC.cpp          |   3 +-
 clang/lib/Frontend/CompilerInvocation.cpp     |  63 +++--
 clang/lib/Lex/HeaderSearch.cpp                |   6 +
 clang/lib/Lex/InitHeaderSearch.cpp            |  20 +-
 clang/test/Driver/cl-include.c                |  30 +-
 clang/test/Driver/cl-options.c                |   4 +-
 .../microsoft-header-search-duplicates.c      | 263 +++++++++++++++++-
 14 files changed, 412 insertions(+), 90 deletions(-)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 0d4654e4f14dfb..07d8a8d6fff65b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4590,7 +4590,7 @@ def idirafter : JoinedOrSeparate<["-"], "idirafter">, Group<clang_i_Group>,
 def iexternal : Separate<["-"], "iexternal">, Group<clang_i_Group>,
   Visibility<[ClangOption, CC1Option]>,
   HelpText<"Add directory to include search path with warnings suppressed">, MetaVarName<"<dir>">;
-def iexternal_after : Separate<["-"], "iexternal-after">, Group<clang_i_Group>,
+def iexternal_system : Separate<["-"], "iexternal-system">, 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>,
@@ -4638,6 +4638,10 @@ def isysroot : JoinedOrSeparate<["-"], "isysroot">, Group<clang_i_Group>,
 def isystem : JoinedOrSeparate<["-"], "isystem">, Group<clang_i_Group>,
   Visibility<[ClangOption, CC1Option]>,
   HelpText<"Add directory to SYSTEM include search path">, MetaVarName<"<directory>">;
+def isystem_env_EQ : Joined<["-"], "isystem-env=">, Group<clang_i_Group>,
+  MetaVarName<"<var>">,
+  Visibility<[ClangOption]>,
+  HelpText<"Add directoires in env var <var> to SYSTEM include search path">;
 def isystem_after : JoinedOrSeparate<["-"], "isystem-after">,
   Group<clang_i_Group>, Flags<[NoXarchOption]>, MetaVarName<"<directory>">,
   HelpText<"Add directory to end of the SYSTEM include search path">;
@@ -8279,6 +8283,12 @@ def internal_isystem : Separate<["-"], "internal-isystem">,
   HelpText<"Add directory to the internal system include search path; these "
            "are assumed to not be user-provided and are used to model system "
            "and standard headers' paths.">;
+def internal_iexternal_system : Separate<["-"], "internal-iexternal-system">,
+  MetaVarName<"<directory>">,
+  HelpText<"Add directory to the internal system include search path with "
+           "external directory prefix semantics; these are assumed to not be "
+           "user-provided and are used to model system and standard headers' "
+           "paths.">;
 def internal_externc_isystem : Separate<["-"], "internal-externc-isystem">,
   MetaVarName<"<directory>">,
   HelpText<"Add directory to the internal system include search path with "
diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h
index 6687c3b3833fde..ae62e7248050b6 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -229,7 +229,7 @@ class ToolChain {
   ///@{
   static void addSystemInclude(const llvm::opt::ArgList &DriverArgs,
                                llvm::opt::ArgStringList &CC1Args,
-                               const Twine &Path);
+                               const Twine &Path, bool Internal = true);
   static void addExternCSystemInclude(const llvm::opt::ArgList &DriverArgs,
                                       llvm::opt::ArgStringList &CC1Args,
                                       const Twine &Path);
@@ -239,16 +239,19 @@ class ToolChain {
                                       const Twine &Path);
   static void addSystemIncludes(const llvm::opt::ArgList &DriverArgs,
                                 llvm::opt::ArgStringList &CC1Args,
-                                ArrayRef<StringRef> Paths);
+                                ArrayRef<StringRef> Paths,
+                                bool Internal = true);
   static bool addSystemIncludesFromEnv(const llvm::opt::ArgList &DriverArgs,
                                        llvm::opt::ArgStringList &CC1Args,
-                                       StringRef Var);
-  static void addExternalAfterIncludes(const llvm::opt::ArgList &DriverArgs,
-                                       llvm::opt::ArgStringList &CC1Args,
-                                       ArrayRef<StringRef> Paths);
-  static bool addExternalIncludesFromEnv(const llvm::opt::ArgList &DriverArgs,
-                                         llvm::opt::ArgStringList &CC1Args,
-                                         StringRef Var);
+                                       StringRef Var, bool Internal = true);
+  static void addExternalSystemIncludes(const llvm::opt::ArgList &DriverArgs,
+                                        llvm::opt::ArgStringList &CC1Args,
+                                        ArrayRef<StringRef> Paths,
+                                        bool Internal = true);
+  static bool
+  addExternalSystemIncludesFromEnv(const llvm::opt::ArgList &DriverArgs,
+                                   llvm::opt::ArgStringList &CC1Args,
+                                   StringRef Var, bool Internal = true);
 
   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/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h
index a10adae17998b5..f5beb51ba6a72b 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -276,6 +276,13 @@ class HeaderSearch {
   /// a system header.
   std::vector<std::pair<std::string, bool>> SystemHeaderPrefixes;
 
+  /// External directories are user specified directories that are to be treated
+  /// like system directories for the purposes of warning suppression. A header
+  /// file that has a path that matches one of these prefixes is promoted to a
+  /// system header regardless of which header search path was used to resolve
+  /// the \#include directive.
+  std::vector<std::string> ExternalDirectoryPrefixes;
+
   /// The hash used for module cache paths.
   std::string ModuleHash;
 
@@ -392,6 +399,11 @@ class HeaderSearch {
     SystemHeaderPrefixes.assign(P.begin(), P.end());
   }
 
+  /// Set the list of external directory prefixes.
+  void SetExternalDirectoryPrefixes(ArrayRef<std::string> P) {
+    ExternalDirectoryPrefixes.assign(P.begin(), P.end());
+  }
+
   /// Checks whether the map exists or not.
   bool HasIncludeAliasMap() const { return (bool)IncludeAliases; }
 
diff --git a/clang/include/clang/Lex/HeaderSearchOptions.h b/clang/include/clang/Lex/HeaderSearchOptions.h
index 5c18c6c8a397e2..a65106394cfd7c 100644
--- a/clang/include/clang/Lex/HeaderSearchOptions.h
+++ b/clang/include/clang/Lex/HeaderSearchOptions.h
@@ -35,19 +35,19 @@ enum IncludeDirGroup {
   /// Paths for '\#include <>' added by '-I'.
   Angled,
 
-  /// Like Angled, but marks system directories while retaining relative order
-  /// with user directories. This group is intended to match the semantics of
-  /// the MSVC /external:I option.
+  /// Like Angled, but marks the directory as an external directory prefix.
+  /// This group is intended to match the semantics of the MSVC /external:I
+  /// option.
   External,
 
-  /// Like External, but searched after other external directories but before
-  /// system directories. This group is intended to match the semantics of the
-  /// MSVC /external:env option.
-  ExternalAfter,
-
   /// Like Angled, but marks system directories.
   System,
 
+  /// Like System, but marks the directory as an external directory prefix.
+  /// This group is intended to match the semantics of the MSVC
+  /// /external:env option.
+  ExternalSystem,
+
   /// Like System, but headers are implicitly wrapped in extern "C".
   ExternCSystem,
 
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index ed0eee61c280b9..e5294c73938976 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1308,7 +1308,8 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
 
   // Check for missing include directories. Diagnostics should not be issued
   // for directories specified with -iexternal, -iexternal-env=, or
-  // -iexternal-after since those options may be used to specify partial paths.
+  // -iexternal-system since those options may be used to specify external
+  // directory prefixes that don't necessarily match an existing path.
   if (!Diags.isIgnored(diag::warn_missing_include_dirs, SourceLocation())) {
     for (auto IncludeDir : Args.getAllArgValues(options::OPT_I_Group)) {
       if (!VFS->exists(IncludeDir))
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 1b49ef20b8020f..678b93b242a263 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1264,8 +1264,11 @@ ToolChain::CXXStdlibType ToolChain::GetCXXStdlibType(const ArgList &Args) const{
 /// Utility function to add a system include directory to CC1 arguments.
 /*static*/ void ToolChain::addSystemInclude(const ArgList &DriverArgs,
                                             ArgStringList &CC1Args,
-                                            const Twine &Path) {
-  CC1Args.push_back("-internal-isystem");
+                                            const Twine &Path, bool Internal) {
+  if (Internal)
+    CC1Args.push_back("-internal-isystem");
+  else
+    CC1Args.push_back("-isystem");
   CC1Args.push_back(DriverArgs.MakeArgString(Path));
 }
 
@@ -1294,9 +1297,13 @@ void ToolChain::addExternCSystemIncludeIfExists(const ArgList &DriverArgs,
 /// Utility function to add a list of system include directories to CC1.
 /*static*/ void ToolChain::addSystemIncludes(const ArgList &DriverArgs,
                                              ArgStringList &CC1Args,
-                                             ArrayRef<StringRef> Paths) {
+                                             ArrayRef<StringRef> Paths,
+                                             bool Internal) {
   for (const auto &Path : Paths) {
-    CC1Args.push_back("-internal-isystem");
+    if (Internal)
+      CC1Args.push_back("-internal-isystem");
+    else
+      CC1Args.push_back("-isystem");
     CC1Args.push_back(DriverArgs.MakeArgString(Path));
   }
 }
@@ -1306,12 +1313,13 @@ void ToolChain::addExternCSystemIncludeIfExists(const ArgList &DriverArgs,
 /// true if the variable is set and not empty.
 /*static*/ bool ToolChain::addSystemIncludesFromEnv(const ArgList &DriverArgs,
                                                     ArgStringList &CC1Args,
-                                                    StringRef Var) {
+                                                    StringRef Var,
+                                                    bool Internal) {
   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);
+      addSystemIncludes(DriverArgs, CC1Args, Dirs, Internal);
       return true;
     }
   }
@@ -1320,11 +1328,15 @@ 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) {
+/*static*/ void ToolChain::addExternalSystemIncludes(const ArgList &DriverArgs,
+                                                     ArgStringList &CC1Args,
+                                                     ArrayRef<StringRef> Paths,
+                                                     bool Internal) {
   for (const auto &Path : Paths) {
-    CC1Args.push_back("-iexternal-after");
+    if (Internal)
+      CC1Args.push_back("-internal-iexternal-system");
+    else
+      CC1Args.push_back("-iexternal-system");
     CC1Args.push_back(DriverArgs.MakeArgString(Path));
   }
 }
@@ -1332,14 +1344,15 @@ void ToolChain::addExternCSystemIncludeIfExists(const ArgList &DriverArgs,
 /// Utility function to add a list of ';' delimited directories specified in
 /// an environment variable to the external include path list for CC1. Returns
 /// true if the variable is set and not empty.
-/*static*/ bool ToolChain::addExternalIncludesFromEnv(const ArgList &DriverArgs,
-                                                      ArgStringList &CC1Args,
-                                                      StringRef Var) {
+/*static*/ bool
+ToolChain::addExternalSystemIncludesFromEnv(const ArgList &DriverArgs,
+                                            ArgStringList &CC1Args,
+                                            StringRef Var, bool Internal) {
   if (auto Val = llvm::sys::Process::GetEnv(Var)) {
     SmallVector<StringRef, 8> Dirs;
     StringRef(*Val).split(Dirs, ";", /*MaxSplit=*/-1, /*KeepEmpty=*/false);
     if (!Dirs.empty()) {
-      addExternalAfterIncludes(DriverArgs, CC1Args, Dirs);
+      addExternalSystemIncludes(DriverArgs, CC1Args, Dirs, Internal);
       return true;
     }
   }
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 2b7aa738b33b28..f4ec56192c7063 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1201,9 +1201,15 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
     } else if (A->getOption().matches(options::OPT_iexternal)) {
       // This option has to retain relative order with other -I options.
       continue;
+    } else if (A->getOption().matches(options::OPT_isystem_env_EQ)) {
+      A->claim();
+      ToolChain::addSystemIncludesFromEnv(Args, CmdArgs, A->getValue(),
+                                          /*Internal*/ false);
+      continue;
     } else if (A->getOption().matches(options::OPT_iexternal_env_EQ)) {
       A->claim();
-      ToolChain::addExternalIncludesFromEnv(Args, CmdArgs, A->getValue());
+      ToolChain::addExternalSystemIncludesFromEnv(Args, CmdArgs, A->getValue(),
+                                                  /*Internal*/ false);
       continue;
     }
 
diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp
index 1590f572da97cb..df90c84cece184 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -671,7 +671,8 @@ void MSVCToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
   if (!DriverArgs.getLastArg(options::OPT__SLASH_vctoolsdir,
                              options::OPT__SLASH_winsysroot)) {
     bool Found = addSystemIncludesFromEnv(DriverArgs, CC1Args, "INCLUDE");
-    Found |= addSystemIncludesFromEnv(DriverArgs, CC1Args, "EXTERNAL_INCLUDE");
+    Found |= addExternalSystemIncludesFromEnv(DriverArgs, CC1Args,
+                                              "EXTERNAL_INCLUDE");
     if (Found)
       return;
   }
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 30e715e92f47b0..c3c689025a5609 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3217,11 +3217,6 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
     GenerateArg(Consumer, Opt, It->Path);
   }
 
-  // Add the paths for the -iexternal-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
@@ -3241,10 +3236,16 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
     GenerateArg(Consumer, OPT_idirafter, It->Path);
   for (; It < End && Matches(*It, {frontend::Quoted}, false, true); ++It)
     GenerateArg(Consumer, OPT_iquote, It->Path);
-  for (; It < End && Matches(*It, {frontend::System}, false, std::nullopt);
-       ++It)
-    GenerateArg(Consumer, It->IgnoreSysRoot ? OPT_isystem : OPT_iwithsysroot,
-                It->Path);
+  for (; It < End && Matches(*It, {frontend::System, frontend::ExternalSystem},
+                             false, std::nullopt);
+       ++It) {
+    OptSpecifier Opt = OPT_isystem;
+    if (It->Group == frontend::ExternalSystem)
+      Opt = OPT_iexternal_system;
+    else if (!It->IgnoreSysRoot)
+      Opt = OPT_iwithsysroot;
+    GenerateArg(Consumer, Opt, It->Path);
+  }
   for (; It < End && Matches(*It, {frontend::System}, true, true); ++It)
     GenerateArg(Consumer, OPT_iframework, It->Path);
   for (; It < End && Matches(*It, {frontend::System}, true, false); ++It)
@@ -3264,12 +3265,16 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
   // Note: Some paths that came from "-internal-isystem" arguments may have
   // already been generated as "-isystem". If that's the case, their position on
   // command line was such that this has no semantic impact on include paths.
-  for (; It < End &&
-         Matches(*It, {frontend::System, frontend::ExternCSystem}, false, true);
+  for (; It < End && Matches(*It,
+                             {frontend::System, frontend::ExternalSystem,
+                              frontend::ExternCSystem},
+                             false, true);
        ++It) {
-    OptSpecifier Opt = It->Group == frontend::System
-                           ? OPT_internal_isystem
-                           : OPT_internal_externc_isystem;
+    OptSpecifier Opt = OPT_internal_isystem;
+    if (It->Group == frontend::ExternalSystem)
+      Opt = OPT_internal_iexternal_system;
+    else if (It->Group == frontend::ExternCSystem)
+      Opt = OPT_internal_externc_isystem;
     GenerateArg(Consumer, Opt, It->Path);
   }
 
@@ -3358,11 +3363,6 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
                  /*IgnoreSysroot=*/true);
   }
 
-  // Add the -iexternal-env= and -iexternal-after options in order.
-  for (const auto *A : Args.filtered(OPT_iexternal_after))
-    Opts.AddPath(A->getValue(), 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 :
@@ -3380,13 +3380,17 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
   for (const auto *A : Args.filtered(OPT_iquote))
     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_isystem, OPT_iwithsysroot, OPT_iexternal_system)) {
+    if (A->getOption().matches(OPT_iexternal_system))
+      Opts.AddPath(A->getValue(), frontend::ExternalSystem,
+                   /*IsFramework=*/false, /*IgnoreSysRoot=*/true);
+    else if (A->getOption().matches(OPT_iwithsysroot))
+      Opts.AddPath(A->getValue(), frontend::System,
+                   /*IsFramework=*/false, /*IgnoreSysRoot=*/false);
+    else
+      Opts.AddPath(PrefixHeaderPath(A), frontend::System,
+                   /*IsFramework=*/false, /*IgnoreSysRoot=*/true);
   }
   for (const auto *A : Args.filtered(OPT_iframework))
     Opts.AddPath(A->getValue(), frontend::System, true, true);
@@ -3406,9 +3410,12 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
 
   // Add the internal paths from a driver that detects standard include paths.
   for (const auto *A :
-       Args.filtered(OPT_internal_isystem, OPT_internal_externc_isystem)) {
+       Args.filtered(OPT_internal_isystem, OPT_internal_iexternal_system,
+                     OPT_internal_externc_isystem)) {
     frontend::IncludeDirGroup Group = frontend::System;
-    if (A->getOption().matches(OPT_internal_externc_isystem))
+    if (A->getOption().matches(OPT_internal_iexternal_system))
+      Group = frontend::ExternalSystem;
+    else if (A->getOption().matches(OPT_internal_externc_isystem))
       Group = frontend::ExternCSystem;
     Opts.AddPath(A->getValue(), Group, false, true);
   }
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index bf8fe44e4ca9ca..062f569984ecda 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1117,6 +1117,12 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
       }
     }
 
+    // If an external directory prefix matches the file, override the file
+    // characteristic.
+    for (const auto &ExtDir : ExternalDirectoryPrefixes)
+      if (File->getName().starts_with(ExtDir))
+        HFI.DirInfo = SrcMgr::C_System;
+
     if (checkMSVCHeaderSearch(Diags, MSFE, &File->getFileEntry(), IncludeLoc)) {
       if (SuggestedModule)
         *SuggestedModule = MSSuggestedModule;
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp b/clang/lib/Lex/InitHeaderSearch.cpp
index 5bb5c6e3addb09..4563abaeed3e06 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -151,7 +151,7 @@ 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.
+    // Group in External, ExternalSystem, System, (Obj)C(XX)System, After.
     Type = SrcMgr::C_System;
   }
 
@@ -424,7 +424,7 @@ static unsigned RemoveDuplicates(const HeaderSearchOptions &HSOpts,
 
     // By default, a search path that follows a previous matching search path
     // is removed. Exceptions exist for paths from the External include group
-    // and for uesr paths that match a later system path.
+    // and for user paths that match a later system path.
     unsigned DirToRemove = i;
     if (CurGroup == frontend::External) {
       // A path that matches a later path specified by -iexternal is always
@@ -509,20 +509,16 @@ void InitHeaderSearch::Realize(const HeaderSearchOptions &HSOpts,
   for (auto &Include : IncludePath)
     if (Include.Group == Angled || Include.Group == External)
       SearchList.push_back(Include);
-  // Add external search paths that must come at the end of the angled
-  // inclusion list.
-  for (auto &Include : IncludePath)
-    if (Include.Group == ExternalAfter)
-      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(HSOpts, SearchList, EndQuoted, EndQuoted, Verbose);
   unsigned EndAngled = SearchList.size();
 
-  // Add search paths for language dependent system paths next.
+  // Add search paths for system paths next.
   for (auto &Include : IncludePath)
-    if (Include.Group == System || Include.Group == ExternCSystem ||
+    if (Include.Group == System || Include.Group == ExternalSystem ||
+        Include.Group == ExternCSystem ||
         (!Lang.ObjC && !Lang.CPlusPlus && Include.Group == CSystem) ||
         (/*FIXME !Lang.ObjC && */ Lang.CPlusPlus &&
          Include.Group == CXXSystem) ||
@@ -548,6 +544,12 @@ void InitHeaderSearch::Realize(const HeaderSearchOptions &HSOpts,
 
   Headers.SetSystemHeaderPrefixes(SystemHeaderPrefixes);
 
+  std::vector<std::string> ExternalDirectoryPrefixes;
+  for (auto &Include : IncludePath)
+    if (Include.Group == External || Include.Group == ExternalSystem)
+      ExternalDirectoryPrefixes.push_back(Include.Lookup.getName().str());
+  Headers.SetExternalDirectoryPrefixes(ExternalDirectoryPrefixes);
+
   // If verbose, print the list of directories that will be searched.
   if (Verbose) {
     llvm::errs() << "#include \"...\" search starts here:\n";
diff --git a/clang/test/Driver/cl-include.c b/clang/test/Driver/cl-include.c
index df5e2aeb4d4a1a..a272c66fcd4564 100644
--- a/clang/test/Driver/cl-include.c
+++ b/clang/test/Driver/cl-include.c
@@ -10,36 +10,36 @@
 // 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" "{{.*lib.*clang.*include}}"
 // STDINC: "-internal-isystem" "/my/system/inc"
-// STDINC: "-internal-isystem" "/my/system/inc2"
+// STDINC: "-internal-iexternal-system" "/my/system/inc2"
 
 // -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/inc1 env EXTERNAL_INCLUDE=/my/system/inc2 %clang_cl -nostdinc -imsvc /my/other/inc -### -- %s 2>&1 | FileCheck %s --check-prefix=NOSTDINC
 // NOSTDINC: argument unused{{.*}}-imsvc
 // NOSTDINC-NOT: "-internal-isystem" "{{.*lib.*clang.*include}}"
-// NOSTDINC-NOT: "-internal-isystem" "/my/other/inc"
-// NOSTDINC-NOT: "-internal-isystem" "/my/system/inc"
-// NOSTDINC-NOT: "-internal-isystem" "/my/system/inc2"
+// NOSTDINC-NOT: "/my/other/inc"
+// NOSTDINC-NOT: "/my/system/inc1"
+// NOSTDINC-NOT: "/my/system/inc2"
 
 // /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
+// RUN: env INCLUDE=/my/system/inc1 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
 // SLASHX-NOT: "argument unused{{.*}}-imsvc"
-// SLASHX-NOT: "-internal-isystem" "/my/system/inc"
-// SLASHX-NOT: "-internal-isystem" "/my/system/inc2"
-// SLASHX: "-iexternal-after" "/my/other/inc2"
+// SLASHX-NOT: "/my/system/inc1"
+// SLASHX-NOT: "/my/system/inc2"
+// SLASHX: "-iexternal-system" "/my/other/inc2"
 // SLASHX: "-internal-isystem" "{{.*lib.*clang.*include}}"
 // SLASHX: "-internal-isystem" "/my/other/inc"
 
 // /winsysroot suppresses %INCLUDE% and %EXTERNAL_INCLUDE% but not -imsvc dirs or /external: flags.
-// RUN: env INCLUDE=/my/system/inc env EXTERNAL_INCLUDE=/my/system/inc2 env FOO=/my/other/inc2 %clang_cl /winsysroot /foo -imsvc /my/other/inc /external:env:FOO -### -- %s 2>&1 | FileCheck %s --check-prefix=SYSROOT
+// RUN: env INCLUDE=/my/system/inc1 env EXTERNAL_INCLUDE=/my/system/inc2 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-NOT: "/my/system/inc1"
 // SYSROOT-NOT: "/my/system/inc2"
-// SYSROOT: "-iexternal-after" "/my/other/inc2"
+// SYSROOT: "-iexternal-system" "/my/other/inc2"
 // SYSROOT: "-internal-isystem" "/my/other/inc"
 // 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: "-iexternal-after" "/dir1"
-// EXTERNAL_ENV: "-iexternal-after" "/dir2"
-// EXTERNAL_ENV: "-iexternal-after" "/dir3"
+// EXTERNAL_ENV: "-iexternal-system" "/dir1"
+// EXTERNAL_ENV: "-iexternal-system" "/dir2"
+// EXTERNAL_ENV: "-iexternal-system" "/dir3"
diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 434acf09ae6447..5bb9cfd1f04592 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -43,8 +43,8 @@
 // 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-after" "path1"
-// EXTERNAL_ENV: "-iexternal-after" "path2"
+// EXTERNAL_ENV: "-iexternal-system" "path1"
+// EXTERNAL_ENV: "-iexternal-system" "path2"
 
 // RUN: %clang_cl /fp:fast /fp:except -### -- %s 2>&1 | FileCheck -check-prefix=fpexcept %s
 // fpexcept-NOT: -funsafe-math-optimizations
diff --git a/clang/test/Driver/microsoft-header-search-duplicates.c b/clang/test/Driver/microsoft-header-search-duplicates.c
index 16f3eb3f0f9864..099443b0a2106c 100644
--- a/clang/test/Driver/microsoft-header-search-duplicates.c
+++ b/clang/test/Driver/microsoft-header-search-duplicates.c
@@ -253,7 +253,7 @@
 // RUN:     -I%t/test5/include/u \
 // RUN:     -iexternal %t/test5/include/v \
 // RUN:     -iexternal-env=EXTRA_INCLUDE \
-// RUN:     -iexternal-env=INCLUDE \
+// RUN:     -isystem-env=INCLUDE \
 // RUN:     -iexternal-env=EXTERNAL_INCLUDE \
 // RUN:     %t/test5/t.c 2>&1 | FileCheck -DPWD=%t %t/test5/t.c
 // RUN: env EXTRA_INCLUDE="%t/test5/include/w" \
@@ -348,3 +348,264 @@
 #error 'test5/include/z/e.h' should not have been included!
 
 #--- test5/include/z/f.h
+
+
+// Test 6: Validate that warning suppression is goverened by external include
+// path matching regardless of include path order.
+//
+// RUN: env EXTRA_INCLUDE="%t/test6/include/x" \
+// RUN: env INCLUDE="%t/test6/include/y" \
+// RUN: env EXTERNAL_INCLUDE="%t/test6/include/z" \
+// RUN: %clang \
+// RUN:     -Xclang -verify \
+// RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -fheader-search=microsoft \
+// RUN:     -nostdinc \
+// RUN:     -Wall \
+// RUN:     -Wno-system-headers \
+// RUN:     -I%t/test6/include/v \
+// RUN:     -I%t/test6/include/w \
+// RUN:     -iexternal %t/test6/include/w \
+// RUN:     -I%t/test6/include/x \
+// RUN:     -I%t/test6/include/y \
+// RUN:     -I%t/test6/include/z \
+// RUN:     -iexternal-env=EXTRA_INCLUDE \
+// RUN:     -isystem-env=INCLUDE \
+// RUN:     -iexternal-env=EXTERNAL_INCLUDE \
+// RUN:     %t/test6/t.c 2>&1 | FileCheck -DPWD=%t %t/test6/t.c
+// RUN: env EXTRA_INCLUDE="%t/test6/include/x" \
+// RUN: env INCLUDE="%t/test6/include/y" \
+// RUN: env EXTERNAL_INCLUDE="%t/test6/include/z" \
+// RUN: %clang_cl \
+// RUN:     -Xclang -verify \
+// RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -nobuiltininc \
+// RUN:     /W4 \
+// RUN:     /external:W0 \
+// RUN:     /I%t/test6/include/v \
+// RUN:     /I%t/test6/include/w \
+// RUN:     /external:I %t/test6/include/w \
+// RUN:     /I%t/test6/include/x \
+// RUN:     /I%t/test6/include/y \
+// RUN:     /I%t/test6/include/z \
+// RUN:     /external:env:EXTRA_INCLUDE \
+// RUN:     %t/test6/t.c 2>&1 | FileCheck -DPWD=%t %t/test6/t.c
+
+#--- test6/t.c
+#include <a.h>
+#include <b.h>
+#include <c.h>
+#include <d.h>
+#include <e.h>
+
+// CHECK:      ignoring duplicate directory "[[PWD]]/test6/include/w"
+// CHECK-NEXT:  as it is a non-system directory that duplicates a system directory
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test6/include/x"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test6/include/y"
+// CHECK-NEXT: ignoring duplicate directory "[[PWD]]/test6/include/z"
+// CHECK-NEXT: #include "..." search starts here:
+// CHECK-NEXT: #include <...> search starts here:
+// CHECK-NEXT: [[PWD]]/test6/include/v
+// CHECK-NEXT: [[PWD]]/test6/include/w
+// CHECK-NEXT: [[PWD]]/test6/include/x
+// CHECK-NEXT: [[PWD]]/test6/include/y
+// CHECK-NEXT: [[PWD]]/test6/include/z
+// CHECK-NEXT: End of search list.
+// CHECK-NOT:  diagnostics seen but not expected
+
+#--- test6/include/v/a.h
+// expected-warning at +1 {{shift count >= width of type}}
+int va = 1 << 1024;
+
+#--- test6/include/w/a.h
+#error 'test6/include/w/a.h' should not have been included!
+
+#--- test6/include/w/b.h
+int wb = 1 << 1024; // Warning should be suppressed.
+
+#--- test6/include/x/a.h
+#error 'test6/include/x/a.h' should not have been included!
+
+#--- test6/include/x/b.h
+#error 'test6/include/x/b.h' should not have been included!
+
+#--- test6/include/x/c.h
+int xc = 1 << 1024; // Warning should be suppressed.
+
+#--- test6/include/y/a.h
+#error 'test6/include/y/a.h' should not have been included!
+
+#--- test6/include/y/b.h
+#error 'test6/include/y/b.h' should not have been included!
+
+#--- test6/include/y/c.h
+#error 'test6/include/y/c.h' should not have been included!
+
+#--- test6/include/y/d.h
+// expected-warning at +1 {{shift count >= width of type}}
+int yd = 1 << 1024; // Warning should NOT be suppressed.
+
+#--- test6/include/z/a.h
+#error 'test6/include/z/a.h' should not have been included!
+
+#--- test6/include/z/b.h
+#error 'test6/include/z/b.h' should not have been included!
+
+#--- test6/include/z/c.h
+#error 'test6/include/z/c.h' should not have been included!
+
+#--- test6/include/z/d.h
+#error 'test6/include/z/d.h' should not have been included!
+
+#--- test6/include/z/e.h
+int ze = 1 << 1024; // Warning should be suppressed.
+
+
+// Test 7: Validate that warning suppression for a header file included via a
+// -I specified path is goverened by an external include path that is a partial
+// match for the resolved header file path (even if the #include directive would
+// not have matched relative to the external path). Note that partial matching
+// includes matching portions of the final path component even if the paths
+// would otherwise select distinct files or directories.
+//
+// RUN: %clang \
+// RUN:     -Xclang -verify \
+// RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -fheader-search=microsoft \
+// RUN:     -nostdinc \
+// RUN:     -Wall \
+// RUN:     -Wno-system-headers \
+// RUN:     -I%t/test7/include/w \
+// RUN:     -I%t/test7/include/x \
+// RUN:     -I%t/test7/include/y \
+// RUN:     -I%t/test7/include/z \
+// RUN:     -iexternal %t/test7/include/x/foo \
+// RUN:     -iexternal %t/test7/include/y/fo \
+// RUN:     -iexternal %t/test7/include/z/f \
+// RUN:     %t/test7/t.c 2>&1 | FileCheck -DPWD=%t %t/test7/t.c
+// RUN: %clang_cl \
+// RUN:     -Xclang -verify \
+// RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -nobuiltininc \
+// RUN:     /W4 \
+// RUN:     /external:W0 \
+// RUN:     /I%t/test7/include/w \
+// RUN:     /I%t/test7/include/x \
+// RUN:     /I%t/test7/include/y \
+// RUN:     /I%t/test7/include/z \
+// RUN:     /external:I %t/test7/include/x/foo \
+// RUN:     /external:I %t/test7/include/y/fo \
+// RUN:     /external:I %t/test7/include/z/f \
+// RUN:     %t/test7/t.c 2>&1 | FileCheck -DPWD=%t %t/test7/t.c
+
+#--- test7/t.c
+#include <foo/a.h>
+#include <foo/b.h>
+#include <foo/c.h>
+#include <foo/d.h>
+
+// CHECK:      #include "..." search starts here:
+// CHECK-NEXT: #include <...> search starts here:
+// CHECK-NEXT: [[PWD]]/test7/include/w
+// CHECK-NEXT: [[PWD]]/test7/include/x
+// CHECK-NEXT: [[PWD]]/test7/include/y
+// CHECK-NEXT: [[PWD]]/test7/include/z
+// CHECK-NEXT: [[PWD]]/test7/include/x/foo
+// CHECK-NEXT: [[PWD]]/test7/include/y/fo
+// CHECK-NEXT: End of search list.
+// CHECK-NOT:  diagnostics seen but not expected
+// CHECK-NOT:  diagnostics expected but not seen
+
+#--- test7/include/w/foo/a.h
+// expected-warning at +1 {{shift count >= width of type}}
+int wa = 1 << 1024;
+
+#--- test7/include/x/foo/a.h
+#error 'test7/include/x/foo/a.h' should not have been included!
+
+#--- test7/include/x/foo/b.h
+int xb = 1 << 1024; // Warning should be suppressed.
+
+#--- test7/include/y/foo/a.h
+#error 'test7/include/y/foo/a.h' should not have been included!
+
+#--- test7/include/y/foo/b.h
+#error 'test7/include/y/foo/b.h' should not have been included!
+
+#--- test7/include/y/fo/unused
+
+#--- test7/include/y/foo/c.h
+int yc = 1 << 1024; // Warning should be suppressed.
+
+#--- test7/include/z/foo/a.h
+#error 'test7/include/z/foo/a.h' should not have been included!
+
+#--- test7/include/z/foo/b.h
+#error 'test7/include/z/foo/b.h' should not have been included!
+
+#--- test7/include/z/foo/c.h
+#error 'test7/include/z/foo/c.h' should not have been included!
+
+#--- test7/include/z/foo/d.h
+// FIXME: MSVC retains external directory prefixes that don't match an actual
+// FIXME: file or directory in the filesystem. Clang discards such paths.
+// expected-warning at +1 {{shift count >= width of type}}
+int zd = 1 << 1024; // Warning should be suppressed.
+
+
+// Test 8: Validate that an external directory path with a trailing path
+// separator is not considered a partial match for an include path where
+// the path component before the trailing path separator is a prefix match
+// for a longer name.
+//
+// RUN: %clang \
+// RUN:     -Xclang -verify \
+// RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -fheader-search=microsoft \
+// RUN:     -nostdinc \
+// RUN:     -Wall \
+// RUN:     -Wno-system-headers \
+// RUN:     -I%t/test8/include/y \
+// RUN:     -I%t/test8/include/z \
+// RUN:     -iexternal %t/test8/include/z/fo \
+// RUN:     %t/test8/t.c 2>&1 | FileCheck -DPWD=%t %t/test8/t.c
+// RUN: %clang_cl \
+// RUN:     -Xclang -verify \
+// RUN:     -target x86_64-pc-windows -v -fsyntax-only \
+// RUN:     -nobuiltininc \
+// RUN:     /W4 \
+// RUN:     /external:W0 \
+// RUN:     /I%t/test8/include/y \
+// RUN:     /I%t/test8/include/z \
+// RUN:     /external:I %t/test8/include/z/fo/ \
+// RUN:     %t/test8/t.c 2>&1 | FileCheck -DPWD=%t %t/test8/t.c
+
+#--- test8/t.c
+#include <foo/a.h>
+#include <foo/b.h>
+
+// CHECK:      #include "..." search starts here:
+// CHECK-NEXT: #include <...> search starts here:
+// CHECK-NEXT: [[PWD]]/test8/include/y
+// CHECK-NEXT: [[PWD]]/test8/include/z
+// CHECK-NEXT: [[PWD]]/test8/include/z/fo
+// CHECK-NEXT: End of search list.
+// CHECK-NOT:  diagnostics seen but not expected
+// CHECK-NOT:  diagnostics expected but not seen
+
+#--- test8/include/y/foo/a.h
+// expected-warning at +1 {{shift count >= width of type}}
+int wa = 1 << 1024;
+
+#--- test8/include/z/foo/a.h
+#error 'test8/include/z/foo/a.h' should not have been included!
+
+#--- test8/include/z/fo/unused
+
+#--- test8/include/z/foo/b.h
+// FIXME: MSVC retains trailing path separators on external directory prefixes
+// FIXME: and will only match the final path component against a complete name
+// FIXME: Clang discards trailing path separators thereby allowing a prefix
+// FIXME: match for the final path component.
+// FIXME-expected-warning at +1 {{shift count >= width of type}}
+int zd = 1 << 1024; // Warning should NOT be suppressed.

>From f2b15afda9dcdd6815201ad710989306c058db5a Mon Sep 17 00:00:00 2001
From: Tom Honermann <tom.honermann at intel.com>
Date: Tue, 7 Jan 2025 13:00:55 -0800
Subject: [PATCH 4/4] Correct external directory prefix matching.

Previously, comparison between a header file path and an external directory
prefix could fail spuriously due to insignficant differences such as repeated
or unconventional path separators. This change canonicalizes paths before
comparison and uses an LLVM path utility to perform the comparison in a more
robust manner. This resolves test failures on Windows.

Additionally, this change corrects a test to remove sensitivity to environment
variables that might be set in the environment in which the test is run.
---
 clang/lib/Lex/HeaderSearch.cpp                | 25 +++++++++++++++----
 .../microsoft-header-search-duplicates.c      |  4 +--
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 062f569984ecda..c2204564d95128 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1117,11 +1117,26 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
       }
     }
 
-    // If an external directory prefix matches the file, override the file
-    // characteristic.
-    for (const auto &ExtDir : ExternalDirectoryPrefixes)
-      if (File->getName().starts_with(ExtDir))
-        HFI.DirInfo = SrcMgr::C_System;
+    // If the file is not already recognized as a system header, check if it
+    // matches an external directory prefix and override the file characteristic
+    // accordingly.
+    if (HFI.DirInfo == SrcMgr::C_User) {
+      // Canonicalize the file path.
+      SmallString<256> FilePath = File->getName();
+      llvm::sys::path::remove_dots(FilePath, /*remove_dot_dot*/ true);
+      for (const auto &ExtDir : ExternalDirectoryPrefixes) {
+        // Canonicalize the external directory prefix.
+        SmallString<256> ExtDirPath = StringRef(ExtDir);
+        llvm::sys::path::remove_dots(ExtDirPath, /*remove_dot_dot*/ true);
+        // If the external directory prefix is a match, override the file
+        // characteristic and break out of the loop. Note that this operation
+        // is destructive to FilePath if the prefix matches.
+        if (llvm::sys::path::replace_path_prefix(FilePath, ExtDirPath, "")) {
+          HFI.DirInfo = SrcMgr::C_System;
+          break;
+        }
+      }
+    }
 
     if (checkMSVCHeaderSearch(Diags, MSFE, &File->getFileEntry(), IncludeLoc)) {
       if (SuggestedModule)
diff --git a/clang/test/Driver/microsoft-header-search-duplicates.c b/clang/test/Driver/microsoft-header-search-duplicates.c
index 099443b0a2106c..37a80435d813a8 100644
--- a/clang/test/Driver/microsoft-header-search-duplicates.c
+++ b/clang/test/Driver/microsoft-header-search-duplicates.c
@@ -486,7 +486,7 @@ int ze = 1 << 1024; // Warning should be suppressed.
 // RUN: %clang_cl \
 // RUN:     -Xclang -verify \
 // RUN:     -target x86_64-pc-windows -v -fsyntax-only \
-// RUN:     -nobuiltininc \
+// RUN:     -nobuiltininc /X \
 // RUN:     /W4 \
 // RUN:     /external:W0 \
 // RUN:     /I%t/test7/include/w \
@@ -572,7 +572,7 @@ int zd = 1 << 1024; // Warning should be suppressed.
 // RUN: %clang_cl \
 // RUN:     -Xclang -verify \
 // RUN:     -target x86_64-pc-windows -v -fsyntax-only \
-// RUN:     -nobuiltininc \
+// RUN:     -nobuiltininc /X \
 // RUN:     /W4 \
 // RUN:     /external:W0 \
 // RUN:     /I%t/test8/include/y \



More information about the llvm-commits mailing list