jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, rsmith.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
When using explicit Clang modules, some declarations might unexpectedly become
invisible.
This was noticed in the following scenario:
- module named `Interface` contains an ObjC interface declaration of the same
name, and its PCM is passed to Clang via the `-fmodule-file=<path>` argument,
- another module referring to `Interface` within an attribute (`objc_bridge`)
is passed to Clang (also via the `-fmodule-file=<path>` argument),
- the Clang invocation then can't find the `Interface` interface declaration.
This seems to be caused by the mechanism that loads PCM files passed via
`-fmodule-file=<path>` which creates an `IdentifierInfo` for the module name.
I'm not sure why exactly that trips up name resolution, but using `StringRef`
for the module name resolves the error. Creating an `IdentifierInfo` with some
prefix (e.g. `CI.getPreprocessor().getIdentifierInfo("LoadModuleCacheKey_" +
ModuleName)`) also resolves the issue.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D111543
Files:
clang/include/clang/Lex/ModuleMap.h
clang/lib/Frontend/CompilerInstance.cpp
clang/test/Modules/Inputs/interface-visibility-2/Interface.h
clang/test/Modules/Inputs/interface-visibility-2/InterfaceBridge.h
clang/test/Modules/Inputs/interface-visibility-2/module.modulemap
clang/test/Modules/interface-visibility-2.m
Index: clang/test/Modules/interface-visibility-2.m
===================================================================
--- /dev/null
+++ clang/test/Modules/interface-visibility-2.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/interface-visibility-2/module.modulemap -o %t/InterfaceBridge.pcm
+
+// RUN: %clang_cc1 -fmodules -x objective-c -emit-module -fmodule-name=Interface \
+// RUN: %S/Inputs/interface-visibility-2/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/Interface-visibility-2 \
+// RUN: -fmodule-file=InterfaceBridge=%t/InterfaceBridge.pcm -fmodule-file=Interface=%t/Interface.pcm \
+// RUN: -fmodule-map-file=%S/Inputs/interface-visibility-2/module.modulemap -verify
+
+// Check that the `-fmodule-file=<path>` form succeeds:
+// RUN: %clang_cc1 -fmodules -fsyntax-only %s -I %S/Inputs/Interface-visibility-2 \
+// RUN: -fmodule-file=%t/InterfaceBridge.pcm -fmodule-file=%t/Interface.pcm \
+// RUN: -fmodule-map-file=%S/Inputs/interface-visibility-2/module.modulemap -verify
+
+#import "InterfaceBridge.h"
+#import "Interface.h"
+
+@interface Interface (User)
+@end
+
+// expected-no-diagnostics
Index: clang/test/Modules/Inputs/interface-visibility-2/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/interface-visibility-2/module.modulemap
@@ -0,0 +1,7 @@
+module InterfaceBridge {
+ header "InterfaceBridge.h"
+}
+
+module Interface {
+ header "Interface.h"
+}
Index: clang/test/Modules/Inputs/interface-visibility-2/InterfaceBridge.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/interface-visibility-2/InterfaceBridge.h
@@ -0,0 +1 @@
+typedef struct __attribute__((objc_bridge(Interface))) Foo *Bar;
Index: clang/test/Modules/Inputs/interface-visibility-2/Interface.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/interface-visibility-2/Interface.h
@@ -0,0 +1,2 @@
+@interface Interface
+@end
Index: clang/lib/Frontend/CompilerInstance.cpp
===================================================================
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -1617,28 +1617,27 @@
// the files we were handed.
struct ReadModuleNames : ASTReaderListener {
CompilerInstance &CI;
- llvm::SmallVector<IdentifierInfo*, 8> LoadedModules;
+ llvm::SmallVector<StringRef, 8> LoadedModules;
ReadModuleNames(CompilerInstance &CI) : CI(CI) {}
void ReadModuleName(StringRef ModuleName) override {
- LoadedModules.push_back(
- CI.getPreprocessor().getIdentifierInfo(ModuleName));
+ LoadedModules.push_back(ModuleName);
}
void registerAll() {
ModuleMap &MM = CI.getPreprocessor().getHeaderSearchInfo().getModuleMap();
- for (auto *II : LoadedModules)
- MM.cacheModuleLoad(*II, MM.findModule(II->getName()));
+ for (StringRef LoadedModule : LoadedModules)
+ MM.cacheModuleLoad(LoadedModule, MM.findModule(LoadedModule));
LoadedModules.clear();
}
void markAllUnavailable() {
- for (auto *II : LoadedModules) {
+ for (StringRef LoadedModule : LoadedModules) {
if (Module *M = CI.getPreprocessor()
.getHeaderSearchInfo()
.getModuleMap()
- .findModule(II->getName())) {
+ .findModule(LoadedModule)) {
M->HasIncompatibleModuleFile = true;
// Mark module as available if the only reason it was unavailable
@@ -1924,7 +1923,7 @@
// If we don't already have information on this module, load the module now.
Module *Module = nullptr;
ModuleMap &MM = getPreprocessor().getHeaderSearchInfo().getModuleMap();
- if (auto MaybeModule = MM.getCachedModuleLoad(*Path[0].first)) {
+ if (auto MaybeModule = MM.getCachedModuleLoad(ModuleName)) {
// Use the cached result, which may be nullptr.
Module = *MaybeModule;
} else if (ModuleName == getLangOpts().CurrentModule) {
@@ -1940,7 +1939,7 @@
// DisableGeneratingGlobalModuleIndex = true;
// return ModuleLoadResult();
//}
- MM.cacheModuleLoad(*Path[0].first, Module);
+ MM.cacheModuleLoad(ModuleName, Module);
} else {
ModuleLoadResult Result = findOrCompileModuleAndReadAST(
ModuleName, ImportLoc, ModuleNameLoc, IsInclusionDirective);
@@ -1949,7 +1948,7 @@
if (!Result)
DisableGeneratingGlobalModuleIndex = true;
Module = Result;
- MM.cacheModuleLoad(*Path[0].first, Module);
+ MM.cacheModuleLoad(ModuleName, Module);
}
// If we never found the module, fail. Otherwise, verify the module and link
Index: clang/include/clang/Lex/ModuleMap.h
===================================================================
--- clang/include/clang/Lex/ModuleMap.h
+++ clang/include/clang/Lex/ModuleMap.h
@@ -101,9 +101,9 @@
/// The top-level modules that are known.
llvm::StringMap<Module *> Modules;
- /// Module loading cache that includes submodules, indexed by IdentifierInfo.
+ /// Module loading cache that includes submodules, indexed by module names.
/// nullptr is stored for modules that are known to fail to load.
- llvm::DenseMap<const IdentifierInfo *, Module *> CachedModuleLoads;
+ llvm::StringMap<Module *> CachedModuleLoads;
/// Shadow modules created while building this module map.
llvm::SmallVector<Module*, 2> ShadowModules;
@@ -703,13 +703,13 @@
}
/// Cache a module load. M might be nullptr.
- void cacheModuleLoad(const IdentifierInfo &II, Module *M) {
- CachedModuleLoads[&II] = M;
+ void cacheModuleLoad(StringRef ModuleName, Module *M) {
+ CachedModuleLoads[ModuleName] = M;
}
/// Return a cached module load.
- llvm::Optional<Module *> getCachedModuleLoad(const IdentifierInfo &II) {
- auto I = CachedModuleLoads.find(&II);
+ llvm::Optional<Module *> getCachedModuleLoad(StringRef ModuleName) {
+ auto I = CachedModuleLoads.find(ModuleName);
if (I == CachedModuleLoads.end())
return None;
return I->second;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
