[clang][modules] Remove Module::ASTFile (#185994)

This removes the assumption that a deserialized module is backed by a
`FileEntry`. The `OptionalFileEntryRef` member is replaced with
`ModuleFile{Name,Key}`.

clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp

@@ -164,7 +164,7 @@ void ExpandModularHeadersPPCallbacks::InclusionDirective(
   if (ModuleImported) {
     serialization::ModuleFile *MF =
         Compiler.getASTReader()->getModuleManager().lookup(
-            *SuggestedModule->getASTFile());
+            *SuggestedModule->getASTFileKey());
     handleModuleFile(MF);
   }
   parseToLocation(DirectiveLoc);

clang/docs/ReleaseNotes.rst

@@ -501,6 +501,7 @@ libclang
 - Visit constraints of `auto` type to properly visit concept usages (#GH166580)
 - Visit switch initializer statements (https://bugs.kde.org/show_bug.cgi?id=415537#c2)
 - Fix crash in clang_getBinaryOperatorKindSpelling and clang_getUnaryOperatorKindSpelling
+- The clang_Module_getASTFile API is deprecated and now always returns nullptr
 
 Code Completion
 ---------------

clang/include/clang-c/Index.h

@@ -4651,6 +4651,8 @@ CINDEX_LINKAGE CXModule clang_getModuleForFile(CXTranslationUnit, CXFile);
  * \param Module a module object.
  *
  * \returns the module file where the provided module object came from.
+ *
+ * @deprecated: module files are longer guaranteed to be loaded from a CXFile
  */
 CINDEX_LINKAGE CXFile clang_Module_getASTFile(CXModule Module);
 

clang/include/clang/Basic/Module.h

@@ -365,9 +365,10 @@ private:
   /// \c SubModules vector at which that submodule resides.
   mutable llvm::StringMap<unsigned> SubModuleIndex;
 
-  /// The AST file if this is a top-level module which has a
+  /// The AST file name and key if this is a top-level module which has a
   /// corresponding serialized AST file, or null otherwise.
-  OptionalFileEntryRef ASTFile;
+  std::optional<ModuleFileName> ASTFileName;
+  std::optional<ModuleFileKey> ASTFileKey;
 
   /// The top-level headers associated with this module.
   llvm::SmallSetVector<FileEntryRef, 2> TopHeaders;
@@ -840,15 +841,26 @@ public:
     return getTopLevelModule()->Name;
   }
 
-  /// The serialized AST file for this module, if one was created.
-  OptionalFileEntryRef getASTFile() const {
-    return getTopLevelModule()->ASTFile;
+  /// The serialized AST file name for this module, if one was created.
+  const ModuleFileName *getASTFileName() const {
+    const Module *TopLevel = getTopLevelModule();
+    return TopLevel->ASTFileName ? &*TopLevel->ASTFileName : nullptr;
   }
 
-  /// Set the serialized AST file for the top-level module of this module.
-  void setASTFile(OptionalFileEntryRef File) {
-    assert((!getASTFile() || getASTFile() == File) && "file path changed");
-    getTopLevelModule()->ASTFile = File;
+  /// The serialized AST file key for this module, if one was created.
+  const ModuleFileKey *getASTFileKey() const {
+    const Module *TopLevel = getTopLevelModule();
+    return TopLevel->ASTFileKey ? &*TopLevel->ASTFileKey : nullptr;
+  }
+
+  /// Set the serialized module file for the top-level module of this module.
+  void setASTFileNameAndKey(ModuleFileName NewName, ModuleFileKey NewKey) {
+    assert(((!getASTFileName() && !getASTFileKey()) ||
+            *getASTFileKey() == NewKey) &&
+           "file path changed");
+    Module *TopLevel = getTopLevelModule();
+    TopLevel->ASTFileName = NewName;
+    TopLevel->ASTFileKey = NewKey;
   }
 
   /// Retrieve the umbrella directory as written.

clang/include/clang/DependencyScanning/ModuleDepCollector.h

@@ -42,7 +42,7 @@ struct PrebuiltModuleDep {
 
   explicit PrebuiltModuleDep(const Module *M)
       : ModuleName(M->getTopLevelModuleName()),
-        PCMFile(M->getASTFile()->getName()),
+        PCMFile(M->getASTFileName()->str()),
         ModuleMapFile(M->PresumedModuleMapFile) {}
 };
 

clang/lib/Basic/ASTSourceDescriptor.cpp

@@ -19,8 +19,8 @@ ASTSourceDescriptor::ASTSourceDescriptor(Module &M)
     : Signature(M.Signature), ClangModule(&M) {
   if (M.Directory)
     Path = M.Directory->getName();
-  if (auto File = M.getASTFile())
-    ASTFile = File->getName();
+  if (auto FileKey = M.getASTFileName())
+    ASTFile = FileKey->str();
 }
 
 std::string ASTSourceDescriptor::getModuleName() const {

clang/lib/DependencyScanning/ModuleDepCollector.cpp

@@ -682,7 +682,7 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
   // -fmodule-name is used to compile a translation unit that imports this
   // module. In that case it can be skipped. The appropriate header
   // dependencies will still be reported as expected.
-  if (!M->getASTFile())
+  if (!M->getASTFileKey())
     return {};
 
   // If this module has been handled already, just return its ID.
@@ -716,7 +716,7 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
 
   serialization::ModuleFile *MF =
       MDC.ScanInstance.getASTReader()->getModuleManager().lookup(
-          *M->getASTFile());
+          *M->getASTFileKey());
 
   llvm::SmallString<256> Storage;
   MD.FileDepsBaseDir =
@@ -938,7 +938,7 @@ bool ModuleDepCollector::isPrebuiltModule(const Module *M) {
   if (PrebuiltModuleFileIt == PrebuiltModuleFiles.end())
     return false;
   assert("Prebuilt module came from the expected AST file" &&
-         PrebuiltModuleFileIt->second == M->getASTFile()->getName());
+         PrebuiltModuleFileIt->second == M->getASTFileName()->str());
   return true;
 }
 

clang/lib/Frontend/CompilerInstance.cpp

@@ -1883,10 +1883,9 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST(
     M = HS.lookupModule(ModuleName, ImportLoc, true, !IsInclusionDirective);
 
     // Check whether M refers to the file in the prebuilt module path.
-    if (M && M->getASTFile())
-      if (auto ModuleFile = FileMgr->getOptionalFileRef(ModuleFilename))
-        if (*ModuleFile == M->getASTFile())
-          return M;
+    if (M && M->getASTFileKey() &&
+        *M->getASTFileKey() == ModuleFilename.makeKey(*FileMgr))
+      return M;
 
     getDiagnostics().Report(ModuleNameLoc, diag::err_module_prebuilt)
         << ModuleName;
@@ -2295,8 +2294,7 @@ GlobalModuleIndex *CompilerInstance::loadGlobalModuleIndex(
     for (ModuleMap::module_iterator I = MMap.module_begin(),
         E = MMap.module_end(); I != E; ++I) {
       Module *TheModule = I->second;
-      OptionalFileEntryRef Entry = TheModule->getASTFile();
-      if (!Entry) {
+      if (!TheModule->getASTFileKey()) {
         SmallVector<IdentifierLoc, 2> Path;
         Path.emplace_back(TriggerLoc,
                           getPreprocessor().getIdentifierInfo(TheModule->Name));

clang/lib/Index/IndexingAction.cpp

@@ -254,7 +254,8 @@ static void indexPreprocessorModuleMacros(Preprocessor &PP,
     if (M.second.getLatest() == nullptr) {
       for (auto *MM : PP.getLeafModuleMacros(M.first)) {
         auto *OwningMod = MM->getOwningModule();
-        if (OwningMod && OwningMod->getASTFile() == Mod.File) {
+        if (OwningMod && OwningMod->getASTFileKey() &&
+            *OwningMod->getASTFileKey() == Mod.FileKey) {
           if (auto *MI = MM->getMacroInfo()) {
             indexPreprocessorMacro(M.first, MI, MacroDirective::MD_Define,
                                    MI->getDefinitionLoc(), DataConsumer);

clang/lib/Sema/SemaModule.cpp

@@ -386,9 +386,9 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
       Diag(Path[0].getLoc(), diag::err_module_redefinition) << ModuleName;
       if (M->DefinitionLoc.isValid())
         Diag(M->DefinitionLoc, diag::note_prev_module_definition);
-      else if (OptionalFileEntryRef FE = M->getASTFile())
+      else if (const ModuleFileName *FileName = M->getASTFileName())
         Diag(M->DefinitionLoc, diag::note_prev_module_definition_from_ast_file)
-            << FE->getName();
+            << *FileName;
       Mod = M;
       break;
     }

clang/lib/Serialization/ASTReader.cpp

@@ -4652,10 +4652,10 @@ ASTReader::ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F,
             << F.ModuleName << F.BaseDirectory << M->Directory->getName();
 
       if (!canRecoverFromOutOfDate(F.FileName, ClientLoadCapabilities)) {
-        if (auto ASTFE = M ? M->getASTFile() : std::nullopt) {
+        if (auto ASTFileName = M ? M->getASTFileName() : nullptr) {
           // This module was defined by an imported (explicit) module.
-          Diag(diag::err_module_file_conflict) << F.ModuleName << F.FileName
-                                               << ASTFE->getName();
+          Diag(diag::err_module_file_conflict)
+              << F.ModuleName << F.FileName << *ASTFileName;
           // TODO: Add a note with the module map paths if they differ.
         } else {
           // This module was built with a different module map.
@@ -6339,15 +6339,16 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
                                        "too many submodules");
 
       if (!ParentModule) {
-        if (OptionalFileEntryRef CurFile = CurrentModule->getASTFile()) {
+        if (const ModuleFileKey *CurFileKey = CurrentModule->getASTFileKey()) {
           // Don't emit module relocation error if we have -fno-validate-pch
           if (!bool(PP.getPreprocessorOpts().DisablePCHOrModuleValidation &
                     DisableValidationForModuleKind::Module)) {
-            assert(CurFile != F.File && "ModuleManager did not de-duplicate");
+            assert(*CurFileKey != F.FileKey &&
+                   "ModuleManager did not de-duplicate");
 
             Diag(diag::err_module_file_conflict)
-                << CurrentModule->getTopLevelModuleName() << CurFile->getName()
-                << F.File.getName();
+                << CurrentModule->getTopLevelModuleName()
+                << *CurrentModule->getASTFileName() << F.FileName;
 
             auto CurModMapFile =
                 ModMap.getContainingModuleMapFile(CurrentModule);
@@ -6361,7 +6362,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
         }
 
         F.DidReadTopLevelSubmodule = true;
-        CurrentModule->setASTFile(F.File);
+        CurrentModule->setASTFileNameAndKey(F.FileName, F.FileKey);
         CurrentModule->PresumedModuleMapFile = F.ModuleMapPath;
       }
 

clang/lib/Serialization/ModuleManager.cpp

@@ -51,8 +51,8 @@ ModuleFile *ModuleManager::lookupByFileName(StringRef Name) const {
 
 ModuleFile *ModuleManager::lookupByModuleName(StringRef Name) const {
   if (const Module *Mod = HeaderSearchInfo.getModuleMap().findModule(Name))
-    if (OptionalFileEntryRef File = Mod->getASTFile())
-      return lookup(*File);
+    if (const ModuleFileName *FileName = Mod->getASTFileName())
+      return lookupByFileName(*FileName);
 
   return nullptr;
 }
@@ -299,10 +299,8 @@ void ModuleManager::removeModules(ModuleIterator First) {
   }
 
   // Delete the modules.
-  for (ModuleIterator victim = First; victim != Last; ++victim) {
-    Modules.erase(ModuleFileKey(victim->File));
+  for (ModuleIterator victim = First; victim != Last; ++victim)
     Modules.erase(victim->FileKey);
-  }
 
   Chain.erase(Chain.begin() + (First - begin()), Chain.end());
 }

clang/test/Index/annotate-module.m

@@ -46,6 +46,6 @@ int glob;
 // RUN: c-index-test -cursor-at=%s:3:11 %s -fmodules-cache-path=%t.cache -fmodules -F %S/../Modules/Inputs \
 // RUN:     | FileCheck %s -check-prefix=CHECK-CURSOR
 
-// CHECK-CURSOR:      3:1 ModuleImport=DependsOnModule:3:1 (Definition) Extent=[3:1 - 3:24] Spelling=DependsOnModule ([3:9 - 3:24]) ModuleName=DependsOnModule ({{.*}}DependsOnModule-{{[^.]*}}.pcm) system=0 Headers(2):
+// CHECK-CURSOR:      3:1 ModuleImport=DependsOnModule:3:1 (Definition) Extent=[3:1 - 3:24] Spelling=DependsOnModule ([3:9 - 3:24]) ModuleName=DependsOnModule system=0 Headers(2):
 // CHECK-CURSOR-NEXT: {{.*}}other.h
 // CHECK-CURSOR-NEXT: {{.*}}DependsOnModule.h

clang/test/Modules/DebugInfoSubmoduleImport.c

@@ -5,7 +5,7 @@
 // RUN:     %s -emit-llvm -debugger-tuning=lldb -o - | FileCheck %s
 //
 // RUN: %clang_cc1 -fmodules -fmodule-format=obj -debug-info-kind=limited -dwarf-ext-refs \
-// RUN:     -fimplicit-module-maps -x c -fmodules-cache-path=%t -I %S/Inputs \
+// RUN:     -fimplicit-module-maps -x c -fmodules-cache-path=%t/cache -I %S/Inputs \
 // RUN:     -fmodules-local-submodule-visibility \
 // RUN:     %s -emit-llvm -debugger-tuning=lldb -o - | FileCheck %s
 #include "DebugSubmoduleA.h"

clang/test/Modules/DebugInfoTransitiveImport.m

@@ -1,7 +1,7 @@
 // UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fmodule-format=obj -debug-info-kind=limited -dwarf-ext-refs \
-// RUN:     -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs \
+// RUN:     -fimplicit-module-maps -fmodules-cache-path=%t/cache -I %S/Inputs \
 // RUN:     %s -mllvm -debug-only=pchcontainer -debugger-tuning=lldb 2>&1 | FileCheck %s
 // REQUIRES: asserts
 

clang/tools/c-index-test/c-index-test.c

@@ -3099,19 +3099,14 @@ static void inspect_print_cursor(CXCursor Cursor) {
 
   {
     CXModule mod = clang_Cursor_getModule(Cursor);
-    CXFile astFile;
-    CXString name, astFilename;
+    CXString name;
     unsigned i, numHeaders;
     if (mod) {
-      astFile = clang_Module_getASTFile(mod);
-      astFilename = clang_getFileName(astFile);
       name = clang_Module_getFullName(mod);
       numHeaders = clang_Module_getNumTopLevelHeaders(TU, mod);
-      printf(" ModuleName=%s (%s) system=%d Headers(%d):",
-             clang_getCString(name), clang_getCString(astFilename),
+      printf(" ModuleName=%s system=%d Headers(%d):", clang_getCString(name),
              clang_Module_isSystem(mod), numHeaders);
       clang_disposeString(name);
-      clang_disposeString(astFilename);
       for (i = 0; i < numHeaders; ++i) {
         CXFile file = clang_Module_getTopLevelHeader(TU, mod, i);
         CXString filename = clang_getFileName(file);

clang/tools/libclang/CIndex.cpp

@@ -9396,12 +9396,7 @@ CXModule clang_getModuleForFile(CXTranslationUnit TU, CXFile File) {
   return Header.getModule();
 }
 
-CXFile clang_Module_getASTFile(CXModule CXMod) {
-  if (!CXMod)
-    return nullptr;
-  Module *Mod = static_cast<Module *>(CXMod);
-  return cxfile::makeCXFile(Mod->getASTFile());
-}
+CXFile clang_Module_getASTFile(CXModule CXMod) { return nullptr; }
 
 CXModule clang_Module_getParent(CXModule CXMod) {
   if (!CXMod)

clang/tools/libclang/CXIndexDataConsumer.cpp

@@ -505,7 +505,11 @@ void CXIndexDataConsumer::importedModule(const ImportDecl *ImportD) {
     if (SrcMod->getTopLevelModule() == Mod->getTopLevelModule())
       return;
 
-  OptionalFileEntryRef FE = Mod->getASTFile();
+  OptionalFileEntryRef FE;
+  if (const ModuleFileName *ASTFileName = Mod->getASTFileName()) {
+    FileManager &FileMgr = cxtu::getASTUnit(CXTU)->getFileManager();
+    FE = FileMgr.getOptionalFileRef(*ASTFileName);
+  }
   CXIdxImportedASTFileInfo Info = {cxfile::makeCXFile(FE), Mod,
                                    getIndexLoc(ImportD->getLocation()),
                                    ImportD->isImplicit()};