Merge pull request #41476 from ahoppen/pr/store-path-in-modulesearchpath

[Serialization] Store the path inside ModuleSearchPath as a string, not a StringRef

Author: ahoppen

include/swift/AST/SearchPathOptions.h

@@ -36,10 +36,9 @@ enum class ModuleSearchPathKind {
 
 /// A single module search path that can come from different sources, e.g.
 /// framework search paths, import search path etc.
-struct ModuleSearchPath {
-  /// The actual path of the module search path. References a search path string
-  /// stored inside \c SearchPathOptions, which must outlive this reference.
-  StringRef Path;
+class ModuleSearchPath : public llvm::RefCountedBase<ModuleSearchPath> {
+  /// The actual path of the module search path.
+  std::string Path;
 
   /// The kind of the search path.
   ModuleSearchPathKind Kind;
@@ -52,6 +51,18 @@ struct ModuleSearchPath {
   /// different file names in \c searchPathsContainingFile.
   unsigned Index;
 
+public:
+  ModuleSearchPath(StringRef Path, ModuleSearchPathKind Kind, bool IsSystem,
+                   unsigned Index)
+      : Path(Path), Kind(Kind), IsSystem(IsSystem), Index(Index) {}
+
+  StringRef getPath() const { return Path; }
+  ModuleSearchPathKind getKind() const { return Kind; }
+
+  bool isSystem() const { return IsSystem; }
+
+  unsigned getIndex() const { return Index; }
+
   bool operator<(const ModuleSearchPath &Other) const {
     if (this->Kind == Other.Kind) {
       return this->Index < Other.Index;
@@ -61,6 +72,8 @@ struct ModuleSearchPath {
   }
 };
 
+using ModuleSearchPathPtr = llvm::IntrusiveRefCntPtr<ModuleSearchPath>;
+
 class SearchPathOptions;
 
 /// Maintains a mapping of filenames to search paths that contain a file with
@@ -97,7 +110,7 @@ class ModuleSearchPathLookup {
     const SearchPathOptions *Opts;
   } State;
 
-  llvm::StringMap<SmallVector<ModuleSearchPath, 4>> LookupTable;
+  llvm::StringMap<SmallVector<ModuleSearchPathPtr, 4>> LookupTable;
 
   /// Scan the directory at \p SearchPath for files and add those files to the
   /// lookup table. \p Kind specifies the search path kind and \p Index the

lib/AST/SearchPathOptions.cpp

@@ -19,16 +19,24 @@ void ModuleSearchPathLookup::addFilesInPathToLookupTable(
     llvm::vfs::FileSystem *FS, StringRef SearchPath, ModuleSearchPathKind Kind,
     bool IsSystem, unsigned SearchPathIndex) {
   std::error_code Error;
-  assert(llvm::all_of(LookupTable, [&](const auto &LookupTableEntry) {
-    return llvm::none_of(LookupTableEntry.second, [&](const ModuleSearchPath &ExistingSearchPath) -> bool {
-      return ExistingSearchPath.Kind == Kind && ExistingSearchPath.Index == SearchPathIndex;
+  auto entryAlreadyExists = [this](ModuleSearchPathKind Kind,
+                                   unsigned SearchPathIndex) -> bool {
+    return llvm::any_of(LookupTable, [&](const auto &LookupTableEntry) {
+      return llvm::any_of(
+          LookupTableEntry.second, [&](ModuleSearchPathPtr ExistingSearchPath) {
+            return ExistingSearchPath->getKind() == Kind &&
+                   ExistingSearchPath->getIndex() == SearchPathIndex;
+          });
     });
-  }) && "Search path with this kind and index already exists");
+  };
+  assert(!entryAlreadyExists(Kind, SearchPathIndex) &&
+         "Search path with this kind and index already exists");
+  ModuleSearchPathPtr TableEntry =
+      new ModuleSearchPath(SearchPath, Kind, IsSystem, SearchPathIndex);
   for (auto Dir = FS->dir_begin(SearchPath, Error);
        !Error && Dir != llvm::vfs::directory_iterator(); Dir.increment(Error)) {
     StringRef Filename = llvm::sys::path::filename(Dir->path());
-    LookupTable[Filename].push_back(
-        {SearchPath, Kind, IsSystem, SearchPathIndex});
+    LookupTable[Filename].push_back(TableEntry);
   }
 }
 
@@ -92,8 +100,9 @@ ModuleSearchPathLookup::searchPathsContainingFile(
 
   for (auto &Filename : Filenames) {
     for (auto &Entry : LookupTable[Filename]) {
-      if (ResultIds.insert(std::make_pair(Entry.Kind, Entry.Index)).second) {
-        Result.push_back(&Entry);
+      if (ResultIds.insert(std::make_pair(Entry->getKind(), Entry->getIndex()))
+              .second) {
+        Result.push_back(Entry.get());
       }
     }
   }

lib/Serialization/SerializedModuleLoader.cpp

@@ -608,10 +608,10 @@ SerializedModuleLoaderBase::findModule(ImportPath::Element moduleID,
       InterestingFilenames, Ctx.SourceMgr.getFileSystem().get(),
       Ctx.LangOpts.Target.isOSDarwin());
   for (const auto &searchPath : searchPaths) {
-    currPath = searchPath->Path;
-    isSystemModule = searchPath->IsSystem;
+    currPath = searchPath->getPath();
+    isSystemModule = searchPath->isSystem();
 
-    switch (searchPath->Kind) {
+    switch (searchPath->getKind()) {
     case ModuleSearchPathKind::Import:
     case ModuleSearchPathKind::RuntimeLibrary: {
       isFramework = false;
@@ -621,7 +621,7 @@ SerializedModuleLoaderBase::findModule(ImportPath::Element moduleID,
       // This was not always true on non-Apple platforms, and in order to
       // ease the transition, check both layouts.
       bool checkTargetSpecificModule = true;
-      if (searchPath->Kind != ModuleSearchPathKind::RuntimeLibrary ||
+      if (searchPath->getKind() != ModuleSearchPathKind::RuntimeLibrary ||
           !Ctx.LangOpts.Target.isOSDarwin()) {
         auto modulePath = currPath;
         llvm::sys::path::append(modulePath, genericModuleFileName);