[clang][modules] Delay creating `IdentifierInfo` for names of explicit modules

When using explicit Clang modules, some declarations might unexpectedly become invisible.

This is caused by the mechanism that loads PCM files passed via `-fmodule-file=<path>` and creates an `IdentifierInfo` for the module name. The `IdentifierInfo` creation takes place when the `ASTReader` is in a weird state, with modules that are loaded but not yet set up properly. This patch delays the creation of `IdentifierInfo` until the `ASTReader` is done with reading the PCM.

Note that the `-fmodule-file=<name>=<path>` form of the argument doesn't suffer from this issue, since it doesn't create `IdentifierInfo` for the module name.

Reviewed By: dexonsmith

Differential Revision: https://reviews.llvm.org/D111543

1 files changed, 9 insertions, 6 deletions

diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 20de917..a9b9e65 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -565,25 +565,28 @@ namespace {
 // the files we were handed.
 struct ReadModuleNames : ASTReaderListener {
   Preprocessor &PP;
-  llvm::SmallVector<IdentifierInfo*, 8> LoadedModules;
+  llvm::SmallVector<std::string, 8> LoadedModules;
 
   ReadModuleNames(Preprocessor &PP) : PP(PP) {}
 
   void ReadModuleName(StringRef ModuleName) override {
-    LoadedModules.push_back(PP.getIdentifierInfo(ModuleName));
+    // Keep the module name as a string for now. It's not safe to create a new
+    // IdentifierInfo from an ASTReader callback.
+    LoadedModules.push_back(ModuleName.str());
   }
 
   void registerAll() {
     ModuleMap &MM = PP.getHeaderSearchInfo().getModuleMap();
-    for (auto *II : LoadedModules)
-      MM.cacheModuleLoad(*II, MM.findModule(II->getName()));
+    for (const std::string &LoadedModule : LoadedModules)
+      MM.cacheModuleLoad(*PP.getIdentifierInfo(LoadedModule),
+                         MM.findModule(LoadedModule));
     LoadedModules.clear();
   }
 
   void markAllUnavailable() {
-    for (auto *II : LoadedModules) {
+    for (const std::string &LoadedModule : LoadedModules) {
       if (Module *M = PP.getHeaderSearchInfo().getModuleMap().findModule(
-              II->getName())) {
+              LoadedModule)) {
         M->HasIncompatibleModuleFile = true;
 
         // Mark module as available if the only reason it was unavailable