WIP

Author: tahonermann

clang/include/clang/Driver/Options.td

@@ -4563,6 +4563,9 @@ def iapinotes_modules : JoinedOrSeparate<["-"], "iapinotes-modules">, Group<clan
 def idirafter : JoinedOrSeparate<["-"], "idirafter">, Group<clang_i_Group>,
   Visibility<[ClangOption, CC1Option]>,
   HelpText<"Add directory to AFTER include search path">;
+def iexternal : JoinedOrSeparate<["-"], "iexternal">, Group<clang_i_Group>,
+  Visibility<[ClangOption, CC1Option]>,
+  HelpText<"Add directory to external include search path">, MetaVarName<"<directory>">;
 def iframework : JoinedOrSeparate<["-"], "iframework">, Group<clang_i_Group>,
   Visibility<[ClangOption, CC1Option]>,
   HelpText<"Add directory to SYSTEM framework search path">;
@@ -8426,7 +8429,7 @@ def _SLASH_diagnostics_classic : CLFlag<"diagnostics:classic">,
 def _SLASH_D : CLJoinedOrSeparate<"D", [CLOption, DXCOption]>,
   HelpText<"Define macro">, MetaVarName<"<macro[=value]>">, Alias<D>;
 def _SLASH_E : CLFlag<"E">, HelpText<"Preprocess to stdout">, Alias<E>;
-def _SLASH_external_COLON_I : CLJoinedOrSeparate<"external:I">, Alias<isystem>,
+def _SLASH_external_COLON_I : CLJoinedOrSeparate<"external:I">, Alias<iexternal>,
   HelpText<"Add directory to include search path with warnings suppressed">,
   MetaVarName<"<dir>">;
 def _SLASH_fp_contract : CLFlag<"fp:contract">, HelpText<"">, Alias<ffp_contract>, AliasArgs<["on"]>;

clang/include/clang/Lex/HeaderSearchOptions.h

@@ -38,6 +38,10 @@ enum IncludeDirGroup {
   /// Like Angled, but marks header maps used when building frameworks.
   IndexHeaderMap,
 
+  /// Like Angled, but marks system directories while retaining relative order
+  /// with user directories.
+  External,
+
   /// Like Angled, but marks system directories.
   System,
 

clang/lib/Driver/Driver.cpp

@@ -1298,7 +1298,8 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
 
   // Check for missing include directories.
   if (!Diags.isIgnored(diag::warn_missing_include_dirs, SourceLocation())) {
-    for (auto IncludeDir : Args.getAllArgValues(options::OPT_I_Group)) {
+    for (auto IncludeDir :
+        Args.getAllArgValues(options::OPT_I_Group, options::OPT_iexternal)) {
       if (!VFS->exists(IncludeDir))
         Diag(diag::warn_missing_include_dirs) << IncludeDir;
     }

clang/lib/Driver/ToolChains/Clang.cpp

@@ -1185,7 +1185,8 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
   Args.addAllArgs(CmdArgs,
                   {options::OPT_D, options::OPT_U, options::OPT_I_Group,
                    options::OPT_F, options::OPT_index_header_map,
-                   options::OPT_embed_dir_EQ});
+                   options::OPT_embed_dir_EQ,
+                   options::OPT_iexternal});
 
   // Add -Wp, and -Xpreprocessor if using the preprocessor.
 

clang/lib/Driver/ToolChains/MSVC.cpp

@@ -660,6 +660,8 @@ void MSVCToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
     return false;
   };
 
+  // FIXME: /external:env won't cause user paths already present in the
+  // FIXME: search path to be dropped like /external:I does.
   // Add %INCLUDE%-like dirs via /external:env: flags.
   for (const auto &Var :
        DriverArgs.getAllArgValues(options::OPT__SLASH_external_env)) {

clang/lib/Frontend/CompilerInvocation.cpp

@@ -3194,8 +3194,10 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
   auto It = Opts.UserEntries.begin();
   auto End = Opts.UserEntries.end();
 
-  // Add -I..., -F..., and -index-header-map options in order.
-  for (; It < End && Matches(*It, {frontend::IndexHeaderMap, frontend::Angled},
+  // Add the -I..., -F..., -index-header-map, and MSVC /external:I options
+  // in order.
+  for (; It < End && Matches(*It, {frontend::IndexHeaderMap, frontend::Angled,
+                                   frontend::External},
                              std::nullopt, true);
        ++It) {
     OptSpecifier Opt = [It, Matches]() {
@@ -3207,13 +3209,15 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
         return OPT_F;
       if (Matches(*It, frontend::Angled, false, true))
         return OPT_I;
+      if (Matches(*It, frontend::External, std::nullopt, true))
+        return OPT_iexternal;
       llvm_unreachable("Unexpected HeaderSearchOptions::Entry.");
     }();
 
     if (It->Group == frontend::IndexHeaderMap)
       GenerateArg(Consumer, OPT_index_header_map);
     GenerateArg(Consumer, Opt, It->Path);
-  };
+  }
 
   // Note: some paths that came from "[-iprefix=xx] -iwithprefixbefore=yy" may
   // have already been generated as "-I[xx]yy". If that's the case, their
@@ -3323,8 +3327,8 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
         llvm::CachedHashString(MacroDef.split('=').first));
   }
 
-  // Add -I..., -F..., and -index-header-map options in order.
-  bool IsIndexHeaderMap = false;
+  // Add the -I..., -F..., -index-header-map, and MSVC /external:I options
+  // options in order.
   bool IsSysrootSpecified =
       Args.hasArg(OPT__sysroot_EQ) || Args.hasArg(OPT_isysroot);
 
@@ -3343,15 +3347,19 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
     return A->getValue();
   };
 
-  for (const auto *A : Args.filtered(OPT_I, OPT_F, OPT_index_header_map)) {
+  bool IsIndexHeaderMap = false;
+  for (const auto *A : Args.filtered(OPT_I, OPT_F, OPT_index_header_map,
+                                     OPT_iexternal)) {
+    frontend::IncludeDirGroup Group =
+        IsIndexHeaderMap ? frontend::IndexHeaderMap : frontend::Angled;
+
     if (A->getOption().matches(OPT_index_header_map)) {
       // -index-header-map applies to the next -I or -F.
       IsIndexHeaderMap = true;
       continue;
     }
-
-    frontend::IncludeDirGroup Group =
-        IsIndexHeaderMap ? frontend::IndexHeaderMap : frontend::Angled;
+    if (A->getOption().matches(OPT_iexternal))
+      Group = frontend::External;
 
     bool IsFramework = A->getOption().matches(OPT_F);
     Opts.AddPath(PrefixHeaderPath(A, IsFramework), Group, IsFramework,

clang/lib/Lex/InitHeaderSearch.cpp

@@ -366,8 +366,8 @@ void InitHeaderSearch::AddDefaultIncludePaths(
 }
 
 /// If there are duplicate directory entries in the specified search list,
-/// remove the later (dead) ones.  Returns the number of non-system headers
-/// removed, which is used to update NumAngled.
+/// identify and remove the ones to be ignored and issue a diagnostic.
+/// Returns the number of non-system search paths emoved.
 static unsigned RemoveDuplicates(const LangOptions &Lang,
                                  std::vector<DirectoryLookupInfo> &SearchList,
                                  unsigned First, bool Verbose) {
@@ -377,41 +377,43 @@ static unsigned RemoveDuplicates(const LangOptions &Lang,
   unsigned NonSystemRemoved = 0;
   for (unsigned i = First; i != SearchList.size(); ++i) {
     unsigned DirToRemove = i;
+    bool NonSystemDirRemoved = false;
 
     const DirectoryLookup &CurEntry = SearchList[i].Lookup;
 
+    // If the current entry is for a previously unseen location, cache it and
+    // continue with the next entry.
     if (CurEntry.isNormalDir()) {
-      // If this isn't the first time we've seen this dir, remove it.
       if (SeenDirs.insert(CurEntry.getDir()).second)
         continue;
     } else if (CurEntry.isFramework()) {
-      // If this isn't the first time we've seen this framework dir, remove it.
       if (SeenFrameworkDirs.insert(CurEntry.getFrameworkDir()).second)
         continue;
     } else {
       assert(CurEntry.isHeaderMap() && "Not a headermap or normal dir?");
-      // If this isn't the first time we've seen this headermap, remove it.
       if (SeenHeaderMaps.insert(CurEntry.getHeaderMap()).second)
         continue;
     }
 
-    // When not in MSVC compatibility mode, if we have a normal
-    // #include dir/framework/headermap that is shadowed later in the chain by
-    // a system include location, we actually want to ignore the user's request
-    // and drop the user dir... keeping the system dir.  This is weird, but
-    // required to emulate GCC's search path correctly.
+    // Pruning of duplicate search locations is intended to emulate the behavior
+    // exhibited by GCC (by default) or MSVC (in Microsoft compatibility mode)
+    // to ensure that #include and #include_next directives produce the same
+    // results as these other compilers.
     //
-    // Since dupes of system dirs are rare, just rescan to find the original
-    // that we're nuking instead of using a DenseMap.
-    if (!Lang.MSVCCompat && CurEntry.getDirCharacteristic() != SrcMgr::C_User) {
-      // Find the dir that this is the same of.
+    // GCC and MSVC both prune duplicate user search locations that follow a
+    // previous matching user search location. Both compilers also prune user
+    // search locations that are also present as system search locations
+    // regardless of the order in which they appear. The compilers differ with
+    // respect to pruning system search locations that duplicate a previous
+    // system search location; GCC preserves the first such occurences while
+    // MSVC preserves the last one.
+    if (CurEntry.getDirCharacteristic() != SrcMgr::C_User) {
+      // Find the matching search entry.
       unsigned FirstDir;
-      for (FirstDir = First;; ++FirstDir) {
-        assert(FirstDir != i && "Didn't find dupe?");
-
+      for (FirstDir = First; FirstDir < i; ++FirstDir) {
         const DirectoryLookup &SearchEntry = SearchList[FirstDir].Lookup;
 
-        // If these are different lookup types, then they can't be the dupe.
+        // Different lookup types are not considered duplicate entries.
         if (SearchEntry.getLookupType() != CurEntry.getLookupType())
           continue;
 
@@ -428,21 +430,34 @@ static unsigned RemoveDuplicates(const LangOptions &Lang,
         if (isSame)
           break;
       }
+      assert(FirstDir < i && "Expected duplicate search location not found");
 
-      // If the first dir in the search path is a non-system dir, zap it
-      // instead of the system one.
-      if (SearchList[FirstDir].Lookup.getDirCharacteristic() == SrcMgr::C_User)
+      if (Lang.MSVCCompat) {
+        // In Microsoft compatibility mode, a later system search location entry
+        // suppresses a previous user or system search location.
         DirToRemove = FirstDir;
+        if (SearchList[FirstDir].Lookup.getDirCharacteristic() ==
+            SrcMgr::C_User)
+          NonSystemDirRemoved = true;
+      } else {
+        // In GCC compatibility mode, a later system search location entry
+        // suppresses a previous user search location.
+        if (SearchList[FirstDir].Lookup.getDirCharacteristic() ==
+            SrcMgr::C_User) {
+          DirToRemove = FirstDir;
+          NonSystemDirRemoved = true;
+        }
+      }
     }
 
     if (Verbose) {
       llvm::errs() << "ignoring duplicate directory \""
                    << CurEntry.getName() << "\"\n";
-      if (DirToRemove != i)
+      if (NonSystemDirRemoved)
         llvm::errs() << "  as it is a non-system directory that duplicates "
                      << "a system directory\n";
     }
-    if (DirToRemove != i)
+    if (NonSystemDirRemoved)
       ++NonSystemRemoved;
 
     // This is reached if the current entry is a duplicate.  Remove the
@@ -490,7 +505,8 @@ void InitHeaderSearch::Realize(const LangOptions &Lang) {
   unsigned NumQuoted = SearchList.size();
 
   for (auto &Include : IncludePath)
-    if (Include.Group == Angled || Include.Group == IndexHeaderMap)
+    if (Include.Group == Angled || Include.Group == IndexHeaderMap ||
+        Include.Group == External)
       SearchList.push_back(Include);
 
   RemoveDuplicates(Lang, SearchList, NumQuoted, Verbose);