Deduplicate search paths.

Now that we can pick up search paths from frameworks (necessary to debug
them properly), we can end up with exponential explosions leading to the
same search path coming up thousands of times, which destroys compilation
time /and/ debugger responsiveness. This is already hitting people with
frameworks compiled for app extensions (due to a mistaken approximation
of whether or not something is a framework), but we're turning this on for
all frameworks in the immediate future.

rdar://problem/20291720

Swift SVN r27087

include/swift/AST/ASTContext.h

@@ -528,6 +528,11 @@ public:
   Type getTypeVariableMemberType(TypeVariableType *baseTypeVar, 
                                  AssociatedTypeDecl *assocType);
 
+  /// Adds a search path to SearchPathOpts, unless it is already present.
+  ///
+  /// Does any proper bookkeeping to keep all module loaders up to date as well.
+  void addSearchPath(StringRef searchPath, bool isFramework);
+
   /// \brief Adds a module loader to this AST context.
   ///
   /// \param loader The new module loader, which will be added after any

include/swift/AST/ClangModuleLoader.h

@@ -23,6 +23,8 @@ class Preprocessor;
 namespace swift {
 
 class ClangModuleLoader : public ModuleLoader {
+private:
+  virtual void anchor();
 protected:
   using ModuleLoader::ModuleLoader;
 public:
@@ -33,6 +35,12 @@ public:
   /// Returns the module that contains imports and declarations from all loaded
   /// Objective-C header files.
   virtual Module *getImportedHeaderModule() const = 0;
+
+  /// Adds a new search path to the Clang CompilerInstance, as if specified with
+  /// -I or -F.
+  ///
+  /// \returns true if there was an error adding the search path.
+  virtual bool addSearchPath(StringRef newSearchPath, bool isFramework) = 0;
 };
 
 } // namespace swift

include/swift/AST/SearchPathOptions.h

@@ -25,14 +25,20 @@ public:
   std::string SDKPath;
 
   /// Path(s) which should be searched for modules.
+  ///
+  /// Do not add values to this directly. Instead, use
+  /// \c ASTContext::addSearchPath.
   std::vector<std::string> ImportSearchPaths;
 
   /// Path(s) which should be searched for frameworks.
+  ///
+  /// Do not add values to this directly. Instead, use
+  /// \c ASTContext::addSearchPath.
   std::vector<std::string> FrameworkSearchPaths;
 
   /// Path(s) which should be searched for libraries.
   ///
-  /// This is used in immediate modes.
+  /// This is used in immediate modes. It is safe to add paths to this directly.
   std::vector<std::string> LibrarySearchPaths;
 
   /// Path to search for compiler-relative header files.

include/swift/ClangImporter/ClangImporter.h

@@ -135,7 +135,7 @@ public:
   /// -I or -F.
   ///
   /// \returns true if there was an error adding the search path.
-  bool addSearchPath(StringRef newSearchPath, bool isFramework);
+  bool addSearchPath(StringRef newSearchPath, bool isFramework) override;
 
   /// Imports an Objective-C header file into the shared imported header module.
   ///

lib/AST/ASTContext.cpp

@@ -39,6 +39,7 @@ using namespace swift;
 
 LazyResolver::~LazyResolver() = default;
 void ModuleLoader::anchor() {}
+void ClangModuleLoader::anchor() {}
 
 llvm::StringRef swift::getProtocolName(KnownProtocolKind kind) {
   switch (kind) {
@@ -256,6 +257,8 @@ struct ASTContext::Implementation {
   /// checking unintended Objective-C overrides.
   std::vector<AbstractFunctionDecl *> ObjCMethods;
 
+  llvm::StringMap<uint8_t> SearchPathsSet;
+
   /// \brief The permanent arena.
   Arena Permanent;
 
@@ -362,6 +365,17 @@ ASTContext::ASTContext(LangOptions &langOpts, SearchPathOptions &SearchPathOpts,
 #define IDENTIFIER(Id) Id_##Id = getIdentifier(#Id);
 #define IDENTIFIER_WITH_NAME(Name, IdStr) Id_##Name = getIdentifier(IdStr);
 #include "swift/AST/KnownIdentifiers.def"
+
+  enum {
+    ImportSearchPathKind = 1 << 0,
+    FrameworkSearchPathKind = 1 << 1
+  };
+
+  // Record the initial set of search paths.
+  for (StringRef path : SearchPathOpts.ImportSearchPaths)
+    Impl.SearchPathsSet[path] |= ImportSearchPathKind;
+  for (StringRef path : SearchPathOpts.FrameworkSearchPaths)
+    Impl.SearchPathsSet[path] |= FrameworkSearchPathKind;
 }
 
 ASTContext::~ASTContext() {
@@ -1044,6 +1058,22 @@ Type ASTContext::getTypeVariableMemberType(TypeVariableType *baseTypeVar,
   return arena.GetTypeMember(baseTypeVar, assocType);
 }
 
+void ASTContext::addSearchPath(StringRef searchPath, bool isFramework) {
+  auto &loaded = Impl.SearchPathsSet[searchPath];
+  uint8_t loadedFlag = (1 << isFramework);
+  if (loaded & loadedFlag)
+    return;
+  loaded |= loadedFlag;
+
+  if (isFramework)
+    SearchPathOpts.FrameworkSearchPaths.push_back(searchPath);
+  else
+    SearchPathOpts.ImportSearchPaths.push_back(searchPath);
+
+  if (auto *clangLoader = getClangModuleLoader())
+    clangLoader->addSearchPath(searchPath, isFramework);
+}
+
 void ASTContext::addModuleLoader(std::unique_ptr<ModuleLoader> loader,
                                  bool IsClang) {
   if (IsClang) {

lib/Serialization/ModuleFile.cpp

@@ -1013,16 +1013,10 @@ Status ModuleFile::associateWithFileContext(FileUnit *file,
     return error(Status::TargetTooNew);
   }
 
-  auto clangImporter = static_cast<ClangImporter *>(ctx.getClangModuleLoader());
+  for (const auto &searchPathPair : SearchPaths)
+    ctx.addSearchPath(searchPathPair.first, searchPathPair.second);
 
-  for (const auto &searchPathPair : SearchPaths) {
-    if (searchPathPair.second) {
-      ctx.SearchPathOpts.FrameworkSearchPaths.push_back(searchPathPair.first);
-    } else {
-      ctx.SearchPathOpts.ImportSearchPaths.push_back(searchPathPair.first);
-    }
-    clangImporter->addSearchPath(searchPathPair.first, searchPathPair.second);
-  }
+  auto clangImporter = static_cast<ClangImporter *>(ctx.getClangModuleLoader());
 
   bool missingDependency = false;
   for (auto &dependency : Dependencies) {

test/Serialization/search-paths.swift

@@ -16,8 +16,19 @@
 // RUN: %target-swift-frontend -emit-module -o %t -I %t/secret -F %t/Frameworks -parse-as-library %S/Inputs/has_xref.swift -application-extension
 // RUN: %target-swift-frontend %s -parse -I %t
 
+// Make sure we don't end up with duplicate search paths.
+// RUN: %target-swiftc_driver -emit-module -o %t/has_xref.swiftmodule -I %t/secret -F %t/Frameworks -parse-as-library %S/Inputs/has_xref.swift %S/../Inputs/empty.swift -Xfrontend -serialize-debugging-options
+// RUN: llvm-bcanalyzer -dump %t/has_xref.swiftmodule | FileCheck %s
+// RUN: %target-swift-frontend %s -parse -I %t
+
 // XFAIL: linux
 
 import has_xref // expected-error {{missing required modules: 'has_alias', 'struct_with_operators'}}
 
 numeric(42) // expected-error {{use of unresolved identifier 'numeric'}}
+
+// CHECK: <INPUT_BLOCK
+// CHECK-NEXT: <SEARCH_PATH abbrevid={{[0-9]+}} op0=0/> blob data = '{{.+}}/secret'
+// CHECK-NEXT: <SEARCH_PATH abbrevid={{[0-9]+}} op0=1/> blob data = '{{.+}}/Frameworks'
+// CHECK-NOT: SEARCH_PATH
+// CHECK: </INPUT_BLOCK>