[clang][modules] Stop uniquing implicit modules via FileEntry

clang-tools-extra/clangd/ModulesBuilder.cpp

@@ -275,8 +275,8 @@ bool IsModuleFileUpToDate(PathRef ModuleFilePath,
   // listener.
   Reader.setListener(nullptr);
 
-  if (Reader.ReadAST(ModuleFilePath, serialization::MK_MainFile,
-                     SourceLocation(),
+  if (Reader.ReadAST(ModuleFileName::makeExplicit(ModuleFilePath),
+                     serialization::MK_MainFile, SourceLocation(),
                      ASTReader::ARR_None) != ASTReader::Success)
     return false;
 

clang/include/clang/Basic/Module.h

@@ -54,6 +54,113 @@ class TargetInfo;
 /// Describes the name of a module.
 using ModuleId = SmallVector<std::pair<std::string, SourceLocation>, 2>;
 
+namespace serialization {
+class ModuleManager;
+} // namespace serialization
+
+/// Deduplication key for a loaded module file in \c ModuleManager.
+///
+/// For implicitly-built modules, this is the \c DirectoryEntry of the module
+/// cache and the module file name with the (optional) context hash.
+/// This enables using \c FileManager's inode-based canonicalization of the
+/// user-provided module cache path without hitting issues on file systems that
+/// recycle inodes for recompiled module files.
+///
+/// For explicitly-built modules, this is \c FileEntry.
+/// This uses \c FileManager's inode-based canonicalization of the user-provided
+/// module file path. Because input explicitly-built modules do not change
+/// during the lifetime of the compiler, inode recycling is not of concern here.
+class ModuleFileKey {
+  /// The FileManager entity used for deduplication.
+  const void *Ptr;
+  /// The path relative to the module cache path for implicit module file, empty
+  /// for other kinds of module files.
+  std::string ImplicitModulePathSuffix;
+
+  friend class serialization::ModuleManager;
+  friend class ModuleFileName;
+  friend llvm::DenseMapInfo<ModuleFileKey>;
+
+  ModuleFileKey(const void *Ptr) : Ptr(Ptr) {}
+
+  ModuleFileKey(const FileEntry *ModuleFile) : Ptr(ModuleFile) {}
+
+  ModuleFileKey(const DirectoryEntry *ModuleCacheDir, StringRef PathSuffix)
+      : Ptr(ModuleCacheDir), ImplicitModulePathSuffix(PathSuffix) {}
+
+public:
+  bool operator==(const ModuleFileKey &Other) const {
+    return Ptr == Other.Ptr &&
+           ImplicitModulePathSuffix == Other.ImplicitModulePathSuffix;
+  }
+
+  bool operator!=(const ModuleFileKey &Other) const {
+    return !operator==(Other);
+  }
+};
+
+/// Identifies a module file to be loaded.
+///
+/// For implicitly-built module files, the path is split into the module cache
+/// path and the module file name with the (optional) context hash. For all
+/// other types of module files, this is just the file system path.
+class ModuleFileName {
+  std::string Path;
+  unsigned ImplicitModuleSuffixLength = 0;
+
+public:
+  /// Creates an empty module file name.
+  ModuleFileName() = default;
+
+  /// Creates a file name for an explicit module.
+  static ModuleFileName makeExplicit(std::string Name) {
+    ModuleFileName File;
+    File.Path = std::move(Name);
+    return File;
+  }
+
+  /// Creates a file name for an explicit module.
+  static ModuleFileName makeExplicit(StringRef Name) {
+    return makeExplicit(Name.str());
+  }
+
+  /// Creates a file name for an implicit module.
+  static ModuleFileName makeImplicit(std::string Name, unsigned SuffixLength) {
+    assert(SuffixLength != 0 && "Empty suffix for implicit module file name");
+    assert(SuffixLength <= Name.size() &&
+           "Suffix for implicit module file name out-of-bounds");
+    ModuleFileName File;
+    File.Path = std::move(Name);
+    File.ImplicitModuleSuffixLength = SuffixLength;
+    return File;
+  }
+
+  /// Creates a file name for an implicit module.
+  static ModuleFileName makeImplicit(StringRef Name, unsigned SuffixLength) {
+    return makeImplicit(Name.str(), SuffixLength);
+  }
+
+  /// Returns the suffix length for an implicit module name, zero otherwise.
+  unsigned getImplicitModuleSuffixLength() const {
+    return ImplicitModuleSuffixLength;
+  }
+
+  /// Returns the plain module file name.
+  StringRef str() const { return Path; }
+
+  /// Converts to StringRef representing the plain module file name.
+  operator StringRef() const { return Path; }
+
+  /// Checks whether the module file name is empty.
+  bool empty() const { return Path.empty(); }
+
+  /// Creates the deduplication key for use in \c ModuleManager.
+  /// Returns an empty optional if:
+  /// * the module cache does not exist for an implicit module name,
+  /// * the module file does not exist for an explicit module name.
+  std::optional<ModuleFileKey> makeKey(FileManager &FileMgr) const;
+};
+
 /// The signature of a module, which is a hash of the AST content.
 struct ASTFileSignature : std::array<uint8_t, 20> {
   using BaseT = std::array<uint8_t, 20>;
@@ -926,4 +1033,23 @@ class VisibleModuleSet {
 
 } // namespace clang
 
+template <> struct llvm::DenseMapInfo<clang::ModuleFileKey> {
+  static clang::ModuleFileKey getEmptyKey() {
+    return DenseMapInfo<const void *>::getEmptyKey();
+  }
+
+  static clang::ModuleFileKey getTombstoneKey() {
+    return DenseMapInfo<const void *>::getTombstoneKey();
+  }
+
+  static unsigned getHashValue(const clang::ModuleFileKey &Val) {
+    return hash_combine(Val.Ptr, Val.ImplicitModulePathSuffix);
+  }
+
+  static bool isEqual(const clang::ModuleFileKey &LHS,
+                      const clang::ModuleFileKey &RHS) {
+    return LHS == RHS;
+  }
+};
+
 #endif // LLVM_CLANG_BASIC_MODULE_H

clang/include/clang/Frontend/CompilerInstance.h

@@ -17,6 +17,7 @@
 #include "clang/Frontend/PCHContainerOperations.h"
 #include "clang/Frontend/Utils.h"
 #include "clang/Lex/DependencyDirectivesScanner.h"
+#include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/HeaderSearchOptions.h"
 #include "clang/Lex/ModuleLoader.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -867,7 +868,7 @@ class CompilerInstance : public ModuleLoader {
 
   void createASTReader();
 
-  bool loadModuleFile(StringRef FileName,
+  bool loadModuleFile(ModuleFileName FileName,
                       serialization::ModuleFile *&LoadedModuleFile);
 
   /// Configuration object for making the result of \c cloneForModuleCompile()

clang/include/clang/Lex/HeaderSearch.h

@@ -664,7 +664,7 @@ class HeaderSearch {
   ///
   /// \returns The name of the module file that corresponds to this module,
   /// or an empty string if this module does not correspond to any module file.
-  std::string getCachedModuleFileName(Module *Module);
+  ModuleFileName getCachedModuleFileName(Module *Module);
 
   /// Retrieve the name of the prebuilt module file that should be used
   /// to load a module with the given name.
@@ -676,8 +676,8 @@ class HeaderSearch {
   ///
   /// \returns The name of the module file that corresponds to this module,
   /// or an empty string if this module does not correspond to any module file.
-  std::string getPrebuiltModuleFileName(StringRef ModuleName,
-                                        bool FileMapOnly = false);
+  ModuleFileName getPrebuiltModuleFileName(StringRef ModuleName,
+                                           bool FileMapOnly = false);
 
   /// Retrieve the name of the prebuilt module file that should be used
   /// to load the given module.
@@ -686,7 +686,7 @@ class HeaderSearch {
   ///
   /// \returns The name of the module file that corresponds to this module,
   /// or an empty string if this module does not correspond to any module file.
-  std::string getPrebuiltImplicitModuleFileName(Module *Module);
+  ModuleFileName getPrebuiltImplicitModuleFileName(Module *Module);
 
   /// Retrieve the name of the (to-be-)cached module file that should
   /// be used to load a module with the given name.
@@ -698,8 +698,8 @@ class HeaderSearch {
   ///
   /// \returns The name of the module file that corresponds to this module,
   /// or an empty string if this module does not correspond to any module file.
-  std::string getCachedModuleFileName(StringRef ModuleName,
-                                      StringRef ModuleMapPath);
+  ModuleFileName getCachedModuleFileName(StringRef ModuleName,
+                                         StringRef ModuleMapPath);
 
   /// Lookup a module Search for a module with the given name.
   ///
@@ -815,13 +815,13 @@ class HeaderSearch {
   /// \param ModuleMapPath A path that when combined with \c ModuleName
   /// uniquely identifies this module. See Module::ModuleMap.
   ///
-  /// \param CachePath A path to the module cache.
+  /// \param NormalizedCachePath The normalized path to the module cache.
   ///
   /// \returns The name of the module file that corresponds to this module,
   /// or an empty string if this module does not correspond to any module file.
-  std::string getCachedModuleFileNameImpl(StringRef ModuleName,
-                                          StringRef ModuleMapPath,
-                                          StringRef CachePath);
+  ModuleFileName getCachedModuleFileNameImpl(StringRef ModuleName,
+                                             StringRef ModuleMapPath,
+                                             StringRef NormalizedCachePath);
 
   /// Retrieve a module with the given name, which may be part of the
   /// given framework.

clang/include/clang/Serialization/ASTBitCodes.h

@@ -44,7 +44,7 @@ namespace serialization {
 /// Version 4 of AST files also requires that the version control branch and
 /// revision match exactly, since there is no backward compatibility of
 /// AST files at this time.
-const unsigned VERSION_MAJOR = 36;
+const unsigned VERSION_MAJOR = 37;
 
 /// AST file minor version number supported by this version of
 /// Clang.

clang/include/clang/Serialization/ASTReader.h

@@ -1549,7 +1549,7 @@ class ASTReader
         : Mod(Mod), ImportedBy(ImportedBy), ImportLoc(ImportLoc) {}
   };
 
-  ASTReadResult ReadASTCore(StringRef FileName, ModuleKind Type,
+  ASTReadResult ReadASTCore(ModuleFileName FileName, ModuleKind Type,
                             SourceLocation ImportLoc, ModuleFile *ImportedBy,
                             SmallVectorImpl<ImportedModule> &Loaded,
                             off_t ExpectedSize, time_t ExpectedModTime,
@@ -1885,7 +1885,7 @@ class ASTReader
   /// NewLoadedModuleFile would refer to the address of the new loaded top level
   /// module. The state of NewLoadedModuleFile is unspecified if the AST file
   /// isn't loaded successfully.
-  ASTReadResult ReadAST(StringRef FileName, ModuleKind Type,
+  ASTReadResult ReadAST(ModuleFileName FileName, ModuleKind Type,
                         SourceLocation ImportLoc,
                         unsigned ClientLoadCapabilities,
                         ModuleFile **NewLoadedModuleFile = nullptr);

clang/include/clang/Serialization/ModuleFile.h

@@ -144,8 +144,10 @@ enum class InputFilesValidation {
 /// other modules.
 class ModuleFile {
 public:
-  ModuleFile(ModuleKind Kind, FileEntryRef File, unsigned Generation)
-      : Kind(Kind), File(File), Generation(Generation) {}
+  ModuleFile(ModuleKind Kind, ModuleFileKey FileKey, FileEntryRef File,
+             unsigned Generation)
+      : Kind(Kind), FileKey(std::move(FileKey)), File(File),
+        Generation(Generation) {}
   ~ModuleFile();
 
   // === General information ===
@@ -157,7 +159,10 @@ class ModuleFile {
   ModuleKind Kind;
 
   /// The file name of the module file.
-  std::string FileName;
+  ModuleFileName FileName;
+
+  /// The key ModuleManager used for the module file.
+  ModuleFileKey FileKey;
 
   /// The name of the module.
   std::string ModuleName;

clang/include/clang/Serialization/ModuleManager.h

@@ -16,6 +16,7 @@
 
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/HeaderSearch.h"
 #include "clang/Serialization/ModuleFile.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/STLExtras.h"
@@ -56,8 +57,8 @@ class ModuleManager {
   // to implement short-circuiting logic when running DFS over the dependencies.
   SmallVector<ModuleFile *, 2> Roots;
 
-  /// All loaded modules, indexed by name.
-  llvm::DenseMap<const FileEntry *, ModuleFile *> Modules;
+  /// All loaded modules.
+  llvm::DenseMap<ModuleFileKey, ModuleFile *> Modules;
 
   /// FileManager that handles translating between filenames and
   /// FileEntry *.
@@ -172,14 +173,22 @@ class ModuleManager {
   ModuleFile &operator[](unsigned Index) const { return *Chain[Index]; }
 
   /// Returns the module associated with the given file name.
+  /// Soon to be removed.
   ModuleFile *lookupByFileName(StringRef FileName) const;
 
   /// Returns the module associated with the given module name.
   ModuleFile *lookupByModuleName(StringRef ModName) const;
 
   /// Returns the module associated with the given module file.
+  /// Soon to be removed.
   ModuleFile *lookup(const FileEntry *File) const;
 
+  /// Returns the module associated with the given module file name.
+  ModuleFile *lookupByFileName(ModuleFileName FileName) const;
+
+  /// Returns the module associated with the given module file key.
+  ModuleFile *lookup(ModuleFileKey Key) const;
+
   /// Returns the in-memory (virtual file) buffer with the given name
   std::unique_ptr<llvm::MemoryBuffer> lookupBuffer(StringRef Name);
 
@@ -237,14 +246,13 @@ class ModuleManager {
   ///
   /// \return A pointer to the module that corresponds to this file name,
   /// and a value indicating whether the module was loaded.
-  AddModuleResult addModule(StringRef FileName, ModuleKind Type,
-                            SourceLocation ImportLoc,
-                            ModuleFile *ImportedBy, unsigned Generation,
-                            off_t ExpectedSize, time_t ExpectedModTime,
+  AddModuleResult addModule(ModuleFileName FileName, ModuleKind Type,
+                            SourceLocation ImportLoc, ModuleFile *ImportedBy,
+                            unsigned Generation, off_t ExpectedSize,
+                            time_t ExpectedModTime,
                             ASTFileSignature ExpectedSignature,
                             ASTFileSignatureReader ReadSignature,
-                            ModuleFile *&Module,
-                            std::string &ErrorStr);
+                            ModuleFile *&Module, std::string &ErrorStr);
 
   /// Remove the modules starting from First (to the end).
   void removeModules(ModuleIterator First);
@@ -282,26 +290,6 @@ class ModuleManager {
   void visit(llvm::function_ref<bool(ModuleFile &M)> Visitor,
              llvm::SmallPtrSetImpl<ModuleFile *> *ModuleFilesHit = nullptr);
 
-  /// Attempt to resolve the given module file name to a file entry.
-  ///
-  /// \param FileName The name of the module file.
-  ///
-  /// \param ExpectedSize The size that the module file is expected to have.
-  /// If the actual size differs, the resolver should return \c true.
-  ///
-  /// \param ExpectedModTime The modification time that the module file is
-  /// expected to have. If the actual modification time differs, the resolver
-  /// should return \c true.
-  ///
-  /// \param File Will be set to the file if there is one, or null
-  /// otherwise.
-  ///
-  /// \returns True if a file exists but does not meet the size/
-  /// modification time criteria, false if the file is either available and
-  /// suitable, or is missing.
-  bool lookupModuleFile(StringRef FileName, off_t ExpectedSize,
-                        time_t ExpectedModTime, OptionalFileEntryRef &File);
-
   /// View the graphviz representation of the module graph.
   void viewGraph();
 

clang/lib/Basic/Module.cpp

@@ -33,6 +33,25 @@
 
 using namespace clang;
 
+std::optional<ModuleFileKey>
+ModuleFileName::makeKey(FileManager &FileMgr) const {
+  if (ImplicitModuleSuffixLength) {
+    StringRef ModuleCachePath =
+        StringRef(Path).drop_back(ImplicitModuleSuffixLength);
+    StringRef ImplicitModuleSuffix =
+        StringRef(Path).take_back(ImplicitModuleSuffixLength);
+    if (auto ModuleCache = FileMgr.getOptionalDirectoryRef(
+            ModuleCachePath, /*CacheFailure=*/false))
+      return ModuleFileKey(*ModuleCache, ImplicitModuleSuffix);
+  } else {
+    if (auto ModuleFile = FileMgr.getOptionalFileRef(Path, /*OpenFile=*/true,
+                                                     /*CacheFailure=*/false))
+      return ModuleFileKey(*ModuleFile);
+  }
+
+  return std::nullopt;
+}
+
 Module::Module(ModuleConstructorTag, StringRef Name,
                SourceLocation DefinitionLoc, Module *Parent, bool IsFramework,
                bool IsExplicit, unsigned VisibilityID)

clang/lib/DependencyScanning/DependencyScannerImpl.cpp

@@ -614,7 +614,7 @@ struct AsyncModuleCompile : PPCallbacks {
 
     HeaderSearch &HS = CI.getPreprocessor().getHeaderSearchInfo();
     ModuleCache &ModCache = CI.getModuleCache();
-    std::string ModuleFileName = HS.getCachedModuleFileName(M);
+    ModuleFileName ModuleFileName = HS.getCachedModuleFileName(M);
 
     uint64_t Timestamp = ModCache.getModuleTimestamp(ModuleFileName);
     // Someone else already built/validated the PCM.