[ClangImporter] clang's -iframework comes before builtin usr/local/include, but Swift's -Fsystem comes after

include/swift/AST/SearchPathOptions.h

@@ -311,18 +311,16 @@ class SearchPathOptions {
   friend class ASTContext;
 
 public:
-  struct FrameworkSearchPath {
+  struct SearchPath {
     std::string Path;
     bool IsSystem = false;
-    FrameworkSearchPath(StringRef path, bool isSystem)
-    : Path(path), IsSystem(isSystem) {}
+    SearchPath(StringRef path, bool isSystem)
+        : Path(path), IsSystem(isSystem) {}
 
-    friend bool operator ==(const FrameworkSearchPath &LHS,
-                            const FrameworkSearchPath &RHS) {
+    friend bool operator==(const SearchPath &LHS, const SearchPath &RHS) {
       return LHS.Path == RHS.Path && LHS.IsSystem == RHS.IsSystem;
     }
-    friend bool operator !=(const FrameworkSearchPath &LHS,
-                            const FrameworkSearchPath &RHS) {
+    friend bool operator!=(const SearchPath &LHS, const SearchPath &RHS) {
       return !(LHS == RHS);
     }
   };
@@ -332,23 +330,23 @@ class SearchPathOptions {
 
   /// Path to the SDK which is being built against.
   ///
-  /// Must me modified through setter to keep \c SearchPathLookup in sync.
+  /// Must be modified through setter to keep \c Lookup in sync.
   std::string SDKPath;
 
   /// Path(s) which should be searched for modules.
   ///
-  /// Must me modified through setter to keep \c SearchPathLookup in sync.
-  std::vector<std::string> ImportSearchPaths;
+  /// Must be modified through setter to keep \c Lookup in sync.
+  std::vector<SearchPath> ImportSearchPaths;
 
   /// Path(s) which should be searched for frameworks.
   ///
-  /// Must me modified through setter to keep \c SearchPathLookup in sync.
-  std::vector<FrameworkSearchPath> FrameworkSearchPaths;
+  /// Must be modified through setter to keep \c Lookup in sync.
+  std::vector<SearchPath> FrameworkSearchPaths;
 
   /// Paths to search for stdlib modules. One of these will be
   /// compiler-relative.
   ///
-  /// Must me modified through setter to keep \c SearchPathLookup in sync.
+  /// Must be modified through setter to keep \c Lookup in sync.
   std::vector<std::string> RuntimeLibraryImportPaths;
 
   /// When on Darwin the framework paths that are implicitly imported.
@@ -369,17 +367,16 @@ class SearchPathOptions {
 
   /// Add a single import search path. Must only be called from
   /// \c ASTContext::addSearchPath.
-  void addImportSearchPath(StringRef Path, llvm::vfs::FileSystem *FS) {
-    ImportSearchPaths.push_back(Path.str());
-    Lookup.searchPathAdded(FS, ImportSearchPaths.back(),
-                           ModuleSearchPathKind::Import, /*isSystem=*/false,
+  void addImportSearchPath(SearchPath Path, llvm::vfs::FileSystem *FS) {
+    ImportSearchPaths.push_back(Path);
+    Lookup.searchPathAdded(FS, ImportSearchPaths.back().Path,
+                           ModuleSearchPathKind::Import, Path.IsSystem,
                            ImportSearchPaths.size() - 1);
   }
 
   /// Add a single framework search path. Must only be called from
   /// \c ASTContext::addSearchPath.
-  void addFrameworkSearchPath(FrameworkSearchPath NewPath,
-                              llvm::vfs::FileSystem *FS) {
+  void addFrameworkSearchPath(SearchPath NewPath, llvm::vfs::FileSystem *FS) {
     FrameworkSearchPaths.push_back(NewPath);
     Lookup.searchPathAdded(FS, FrameworkSearchPaths.back().Path,
                            ModuleSearchPathKind::Framework, NewPath.IsSystem,
@@ -448,21 +445,21 @@ class SearchPathOptions {
     SysRoot = sysroot;
   }
 
-  ArrayRef<std::string> getImportSearchPaths() const {
+  ArrayRef<SearchPath> getImportSearchPaths() const {
     return ImportSearchPaths;
   }
 
-  void setImportSearchPaths(std::vector<std::string> NewImportSearchPaths) {
+  void setImportSearchPaths(std::vector<SearchPath> NewImportSearchPaths) {
     ImportSearchPaths = NewImportSearchPaths;
     Lookup.searchPathsDidChange();
   }
 
-  ArrayRef<FrameworkSearchPath> getFrameworkSearchPaths() const {
+  ArrayRef<SearchPath> getFrameworkSearchPaths() const {
     return FrameworkSearchPaths;
   }
 
-  void setFrameworkSearchPaths(
-      std::vector<FrameworkSearchPath> NewFrameworkSearchPaths) {
+  void
+  setFrameworkSearchPaths(std::vector<SearchPath> NewFrameworkSearchPaths) {
     FrameworkSearchPaths = NewFrameworkSearchPaths;
     Lookup.searchPathsDidChange();
   }
@@ -597,8 +594,7 @@ class SearchPathOptions {
       llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS) const;
 
 private:
-  static StringRef
-  pathStringFromFrameworkSearchPath(const FrameworkSearchPath &next) {
+  static StringRef pathStringFromSearchPath(const SearchPath &next) {
     return next.Path;
   };
 
@@ -609,17 +605,16 @@ class SearchPathOptions {
     using llvm::hash_combine;
     using llvm::hash_combine_range;
 
-    using FrameworkPathView = ArrayRefView<FrameworkSearchPath, StringRef,
-                                           pathStringFromFrameworkSearchPath>;
-    FrameworkPathView frameworkPathsOnly{FrameworkSearchPaths};
+    using SearchPathView =
+        ArrayRefView<SearchPath, StringRef, pathStringFromSearchPath>;
+    SearchPathView importPathsOnly{ImportSearchPaths};
+    SearchPathView frameworkPathsOnly{FrameworkSearchPaths};
 
     return hash_combine(SDKPath,
-                        hash_combine_range(ImportSearchPaths.begin(),
-                                           ImportSearchPaths.end()),
-                        hash_combine_range(VFSOverlayFiles.begin(),
-                                           VFSOverlayFiles.end()),
-                        // FIXME: Should we include the system-ness of framework
+                        // FIXME: Should we include the system-ness of
                         // search paths too?
+                        hash_combine_range(importPathsOnly.begin(), importPathsOnly.end()),
+                        hash_combine_range(VFSOverlayFiles.begin(), VFSOverlayFiles.end()),
                         hash_combine_range(frameworkPathsOnly.begin(),
                                            frameworkPathsOnly.end()),
                         hash_combine_range(LibrarySearchPaths.begin(),

include/swift/Frontend/Frontend.h

@@ -187,7 +187,8 @@ class CompilerInvocation {
     return ClangImporterOpts.ClangScannerModuleCachePath;
   }
 
-  void setImportSearchPaths(const std::vector<std::string> &Paths) {
+  void setImportSearchPaths(
+      const std::vector<SearchPathOptions::SearchPath> &Paths) {
     SearchPathOpts.setImportSearchPaths(Paths);
   }
 
@@ -196,16 +197,16 @@ class CompilerInvocation {
     SearchPathOpts.DeserializedPathRecoverer = obfuscator;
   }
 
-  ArrayRef<std::string> getImportSearchPaths() const {
+  ArrayRef<SearchPathOptions::SearchPath> getImportSearchPaths() const {
     return SearchPathOpts.getImportSearchPaths();
   }
 
   void setFrameworkSearchPaths(
-             const std::vector<SearchPathOptions::FrameworkSearchPath> &Paths) {
+      const std::vector<SearchPathOptions::SearchPath> &Paths) {
     SearchPathOpts.setFrameworkSearchPaths(Paths);
   }
 
-  ArrayRef<SearchPathOptions::FrameworkSearchPath> getFrameworkSearchPaths() const {
+  ArrayRef<SearchPathOptions::SearchPath> getFrameworkSearchPaths() const {
     return SearchPathOpts.getFrameworkSearchPaths();
   }
 

include/swift/Option/Options.td

@@ -338,6 +338,11 @@ def I : JoinedOrSeparate<["-"], "I">,
 def I_EQ : Joined<["-"], "I=">, Flags<[FrontendOption, ArgumentIsPath]>,
   Alias<I>;
 
+def Isystem : Separate<["-"], "Isystem">,
+  Flags<[FrontendOption, ArgumentIsPath, SwiftSymbolGraphExtractOption,
+         SwiftSynthesizeInterfaceOption]>,
+  HelpText<"Add directory to the system import search path">;
+
 def import_underlying_module : Flag<["-"], "import-underlying-module">,
   Flags<[FrontendOption, NoInteractiveOption]>,
   HelpText<"Implicitly imports the Objective-C half of a module">;

lib/AST/ASTContext.cpp

@@ -824,8 +824,8 @@ ASTContext::ASTContext(
 #include "swift/AST/KnownIdentifiers.def"
 
   // Record the initial set of search paths.
-  for (StringRef path : SearchPathOpts.getImportSearchPaths())
-    getImpl().SearchPathsSet[path] |= SearchPathKind::Import;
+  for (const auto &path : SearchPathOpts.getImportSearchPaths())
+    getImpl().SearchPathsSet[path.Path] |= SearchPathKind::Import;
   for (const auto &framepath : SearchPathOpts.getFrameworkSearchPaths())
     getImpl().SearchPathsSet[framepath.Path] |= SearchPathKind::Framework;
 
@@ -2098,7 +2098,7 @@ void ASTContext::addSearchPath(StringRef searchPath, bool isFramework,
     SearchPathOpts.addFrameworkSearchPath({searchPath, isSystem},
                                           SourceMgr.getFileSystem().get());
   } else {
-    SearchPathOpts.addImportSearchPath(searchPath,
+    SearchPathOpts.addImportSearchPath({searchPath, isSystem},
                                        SourceMgr.getFileSystem().get());
   }
 
@@ -2168,8 +2168,8 @@ Identifier ASTContext::getRealModuleName(Identifier key, ModuleAliasLookupOption
 }
 
 namespace {
-  static StringRef
-  pathStringFromFrameworkSearchPath(const SearchPathOptions::FrameworkSearchPath &next) {
+  static StringRef pathStringFromSearchPath(
+      const SearchPathOptions::SearchPath &next) {
     return next.Path;
   }
 }
@@ -2183,16 +2183,16 @@ const {
 llvm::StringSet<> ASTContext::getAllModuleSearchPathsSet()
 const {
   llvm::StringSet<> result;
-  auto ImportSearchPaths = SearchPathOpts.getImportSearchPaths();
-  result.insert(ImportSearchPaths.begin(), ImportSearchPaths.end());
-
-  // Framework paths are "special", they contain more than path strings,
-  // but path strings are all we care about here.
-  using FrameworkPathView = ArrayRefView<SearchPathOptions::FrameworkSearchPath,
-                                         StringRef,
-                                         pathStringFromFrameworkSearchPath>;
-  FrameworkPathView frameworkPathsOnly{
-      SearchPathOpts.getFrameworkSearchPaths()};
+
+  // Import and framework paths are "special", they contain more than path
+  // strings, but path strings are all we care about here.
+  using SearchPathView = ArrayRefView<SearchPathOptions::SearchPath, StringRef,
+                                      pathStringFromSearchPath>;
+
+  SearchPathView importPathsOnly{SearchPathOpts.getImportSearchPaths()};
+  result.insert(importPathsOnly.begin(), importPathsOnly.end());
+
+  SearchPathView frameworkPathsOnly{SearchPathOpts.getFrameworkSearchPaths()};
   result.insert(frameworkPathsOnly.begin(), frameworkPathsOnly.end());
 
   if (LangOpts.Target.isOSDarwin()) {

lib/AST/SearchPathOptions.cpp

@@ -48,9 +48,9 @@ void ModuleSearchPathLookup::rebuildLookupTable(const SearchPathOptions *Opts,
   clearLookupTable();
 
   for (auto Entry : llvm::enumerate(Opts->getImportSearchPaths())) {
-    addFilesInPathToLookupTable(FS, Entry.value(),
+    addFilesInPathToLookupTable(FS, Entry.value().Path,
                                 ModuleSearchPathKind::Import,
-                                /*isSystem=*/false, Entry.index());
+                                Entry.value().IsSystem, Entry.index());
   }
 
   for (auto Entry : llvm::enumerate(Opts->getFrameworkSearchPaths())) {
@@ -109,9 +109,11 @@ SearchPathOptions::getSDKPlatformPath(llvm::vfs::FileSystem *FS) const {
 }
 
 void SearchPathOptions::dump(bool isDarwin) const {
-  llvm::errs() << "Module import search paths (non system):\n";
+  llvm::errs() << "Module import search paths:\n";
   for (auto Entry : llvm::enumerate(getImportSearchPaths())) {
-    llvm::errs() << "  [" << Entry.index() << "] " << Entry.value() << "\n";
+    llvm::errs() << "  [" << Entry.index() << "] "
+                 << (Entry.value().IsSystem ? "(system) " : "(non-system) ")
+                 << Entry.value().Path << "\n";
   }
 
   llvm::errs() << "Framework search paths:\n";

lib/ClangImporter/ClangImporter.cpp

@@ -206,20 +206,6 @@ namespace {
       return std::make_unique<HeaderParsingASTConsumer>(Impl);
     }
     bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
-      // Prefer frameworks over plain headers.
-      // We add search paths here instead of when building the initial invocation
-      // so that (a) we use the same code as search paths for imported modules,
-      // and (b) search paths are always added after -Xcc options.
-      SearchPathOptions &searchPathOpts = Ctx.SearchPathOpts;
-      for (const auto &framepath : searchPathOpts.getFrameworkSearchPaths()) {
-        Importer.addSearchPath(framepath.Path, /*isFramework*/true,
-                               framepath.IsSystem);
-      }
-
-      for (const auto &path : searchPathOpts.getImportSearchPaths()) {
-        Importer.addSearchPath(path, /*isFramework*/false, /*isSystem=*/false);
-      }
-
       auto PCH =
           Importer.getOrCreatePCH(ImporterOpts, SwiftPCHHash, /*Cached=*/true);
       if (PCH.has_value()) {
@@ -890,19 +876,19 @@ importer::addCommonInvocationArguments(
     }
   }
 
-  if (std::optional<StringRef> R = ctx.SearchPathOpts.getWinSDKRoot()) {
+  if (std::optional<StringRef> R = searchPathOpts.getWinSDKRoot()) {
     invocationArgStrs.emplace_back("-Xmicrosoft-windows-sdk-root");
     invocationArgStrs.emplace_back(*R);
   }
-  if (std::optional<StringRef> V = ctx.SearchPathOpts.getWinSDKVersion()) {
+  if (std::optional<StringRef> V = searchPathOpts.getWinSDKVersion()) {
     invocationArgStrs.emplace_back("-Xmicrosoft-windows-sdk-version");
     invocationArgStrs.emplace_back(*V);
   }
-  if (std::optional<StringRef> R = ctx.SearchPathOpts.getVCToolsRoot()) {
+  if (std::optional<StringRef> R = searchPathOpts.getVCToolsRoot()) {
     invocationArgStrs.emplace_back("-Xmicrosoft-visualc-tools-root");
     invocationArgStrs.emplace_back(*R);
   }
-  if (std::optional<StringRef> V = ctx.SearchPathOpts.getVCToolsVersion()) {
+  if (std::optional<StringRef> V = searchPathOpts.getVCToolsVersion()) {
     invocationArgStrs.emplace_back("-Xmicrosoft-visualc-tools-version");
     invocationArgStrs.emplace_back(*V);
   }
@@ -948,6 +934,24 @@ importer::addCommonInvocationArguments(
   for (auto extraArg : importerOpts.ExtraArgs) {
     invocationArgStrs.push_back(extraArg);
   }
+
+  for (const auto &framepath : searchPathOpts.getFrameworkSearchPaths()) {
+    if (framepath.IsSystem) {
+      invocationArgStrs.push_back("-iframework");
+      invocationArgStrs.push_back(framepath.Path);
+    } else {
+      invocationArgStrs.push_back("-F" + framepath.Path);
+    }
+  }
+
+  for (const auto &path : searchPathOpts.getImportSearchPaths()) {
+    if (path.IsSystem) {
+      invocationArgStrs.push_back("-isystem");
+      invocationArgStrs.push_back(path.Path);
+    } else {
+      invocationArgStrs.push_back("-I" + path.Path);
+    }
+  }
 }
 
 bool ClangImporter::canReadPCH(StringRef PCHFilename) {

lib/ClangImporter/ClangModuleDependencyScanner.cpp

@@ -61,24 +61,9 @@ moduleCacheRelativeLookupModuleOutput(const ModuleID &MID, ModuleOutputKind MOK,
   return outputPath.str().str();
 }
 
-// Add search paths.
-// Note: This is handled differently for the Clang importer itself, which
-// adds search paths to Clang's data structures rather than to its
-// command line.
-static void addSearchPathInvocationArguments(
+static void addScannerPrefixMapperInvocationArguments(
     std::vector<std::string> &invocationArgStrs, ASTContext &ctx) {
-  SearchPathOptions &searchPathOpts = ctx.SearchPathOpts;
-  for (const auto &framepath : searchPathOpts.getFrameworkSearchPaths()) {
-    invocationArgStrs.push_back(framepath.IsSystem ? "-iframework" : "-F");
-    invocationArgStrs.push_back(framepath.Path);
-  }
-
-  for (const auto &path : searchPathOpts.getImportSearchPaths()) {
-    invocationArgStrs.push_back("-I");
-    invocationArgStrs.push_back(path);
-  }
-
-  for (const auto &arg : searchPathOpts.ScannerPrefixMapper) {
+  for (const auto &arg : ctx.SearchPathOpts.ScannerPrefixMapper) {
     std::string prefixMapArg = "-fdepscan-prefix-map=" + arg;
     invocationArgStrs.push_back(prefixMapArg);
   }
@@ -89,7 +74,7 @@ static std::vector<std::string> getClangDepScanningInvocationArguments(
     ASTContext &ctx, std::optional<StringRef> sourceFileName = std::nullopt) {
   std::vector<std::string> commandLineArgs =
       ClangImporter::getClangDriverArguments(ctx);
-  addSearchPathInvocationArguments(commandLineArgs, ctx);
+  addScannerPrefixMapperInvocationArguments(commandLineArgs, ctx);
 
   auto sourceFilePos = std::find(
       commandLineArgs.begin(), commandLineArgs.end(),

lib/DriverTool/sil_func_extractor_main.cpp

@@ -242,7 +242,11 @@ int sil_func_extractor_main(ArrayRef<const char *> argv, void *MainAddr) {
   Invocation.setMainExecutablePath(llvm::sys::fs::getMainExecutable(argv[0], MainAddr));
 
   // Give the context the list of search paths to use for modules.
-  Invocation.setImportSearchPaths(options.ImportPaths);
+  std::vector<SearchPathOptions::SearchPath> ImportPaths;
+  for (const auto &path : options.ImportPaths) {
+    ImportPaths.push_back({path, /*isSystem=*/false});
+  }
+  Invocation.setImportSearchPaths(ImportPaths);
   // Set the SDK path and target if given.
   if (options.SDKPath.getNumOccurrences() == 0) {
     const char *SDKROOT = getenv("SDKROOT");

lib/DriverTool/sil_llvm_gen_main.cpp

@@ -129,8 +129,12 @@ int sil_llvm_gen_main(ArrayRef<const char *> argv, void *MainAddr) {
   Invocation.setMainExecutablePath(llvm::sys::fs::getMainExecutable(argv[0], MainAddr));
 
   // Give the context the list of search paths to use for modules.
-  Invocation.setImportSearchPaths(options.ImportPaths);
-  std::vector<SearchPathOptions::FrameworkSearchPath> FramePaths;
+  std::vector<SearchPathOptions::SearchPath> ImportPaths;
+  for (const auto &path : options.ImportPaths) {
+    ImportPaths.push_back({path, /*isSystem=*/false});
+  }
+  Invocation.setImportSearchPaths(ImportPaths);
+  std::vector<SearchPathOptions::SearchPath> FramePaths;
   for (const auto &path : options.FrameworkPaths) {
     FramePaths.push_back({path, /*isSystem=*/false});
   }