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

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-10-18T11:50:29+02:00
New Revision: a2d805c020a1658b04ed7e606ee67e234a9d5b56

URL: https://github.com/llvm/llvm-project/commit/a2d805c020a1658b04ed7e606ee67e234a9d5b56
DIFF: https://github.com/llvm/llvm-project/commit/a2d805c020a1658b04ed7e606ee67e234a9d5b56.diff

LOG: [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

Added: 
    clang/test/Modules/Inputs/module-name-used-by-objc-bridge/Interface.h
    clang/test/Modules/Inputs/module-name-used-by-objc-bridge/InterfaceBridge.h
    clang/test/Modules/Inputs/module-name-used-by-objc-bridge/module.modulemap
    clang/test/Modules/module-name-used-by-objc-bridge.m

Modified: 
    clang/lib/Frontend/CompilerInstance.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 20de9174740ac..a9b9e6516d56f 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

diff  --git a/clang/test/Modules/Inputs/module-name-used-by-objc-bridge/Interface.h b/clang/test/Modules/Inputs/module-name-used-by-objc-bridge/Interface.h
new file mode 100644
index 0000000000000..a825c0d25f311
--- /dev/null
+++ b/clang/test/Modules/Inputs/module-name-used-by-objc-bridge/Interface.h
@@ -0,0 +1,2 @@
+ at interface Interface
+ at end

diff  --git a/clang/test/Modules/Inputs/module-name-used-by-objc-bridge/InterfaceBridge.h b/clang/test/Modules/Inputs/module-name-used-by-objc-bridge/InterfaceBridge.h
new file mode 100644
index 0000000000000..63f6399a806ce
--- /dev/null
+++ b/clang/test/Modules/Inputs/module-name-used-by-objc-bridge/InterfaceBridge.h
@@ -0,0 +1 @@
+typedef struct __attribute__((objc_bridge(Interface))) Foo *Bar;

diff  --git a/clang/test/Modules/Inputs/module-name-used-by-objc-bridge/module.modulemap b/clang/test/Modules/Inputs/module-name-used-by-objc-bridge/module.modulemap
new file mode 100644
index 0000000000000..a0bd43ff80a56
--- /dev/null
+++ b/clang/test/Modules/Inputs/module-name-used-by-objc-bridge/module.modulemap
@@ -0,0 +1,7 @@
+module InterfaceBridge {
+  header "InterfaceBridge.h"
+}
+
+module Interface {
+  header "Interface.h"
+}

diff  --git a/clang/test/Modules/module-name-used-by-objc-bridge.m b/clang/test/Modules/module-name-used-by-objc-bridge.m
new file mode 100644
index 0000000000000..afaa0cb0e4a7d
--- /dev/null
+++ b/clang/test/Modules/module-name-used-by-objc-bridge.m
@@ -0,0 +1,25 @@
+// RUN: rm -rf %t && mkdir %t
+
+// RUN: %clang_cc1 -fmodules -x objective-c -emit-module -fmodule-name=InterfaceBridge \
+// RUN:   %S/Inputs/module-name-used-by-objc-bridge/module.modulemap -o %t/InterfaceBridge.pcm
+
+// RUN: %clang_cc1 -fmodules -x objective-c -emit-module -fmodule-name=Interface \
+// RUN:   %S/Inputs/module-name-used-by-objc-bridge/module.modulemap -o %t/Interface.pcm
+
+// Check that the `-fmodule-file=<name>=<path>` form succeeds:
+// RUN: %clang_cc1 -fmodules -fsyntax-only %s -I %S/Inputs/module-name-used-by-objc-bridge \
+// RUN:   -fmodule-file=InterfaceBridge=%t/InterfaceBridge.pcm -fmodule-file=Interface=%t/Interface.pcm \
+// RUN:   -fmodule-map-file=%S/Inputs/module-name-used-by-objc-bridge/module.modulemap -verify
+
+// Check that the `-fmodule-file=<path>` form succeeds:
+// RUN: %clang_cc1 -fmodules -fsyntax-only %s -I %S/Inputs/module-name-used-by-objc-bridge \
+// RUN:   -fmodule-file=%t/InterfaceBridge.pcm -fmodule-file=%t/Interface.pcm \
+// RUN:   -fmodule-map-file=%S/Inputs/module-name-used-by-objc-bridge/module.modulemap -verify
+
+#import "InterfaceBridge.h"
+#import "Interface.h"
+
+ at interface Interface (User)
+ at end
+
+// expected-no-diagnostics