diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h index dd384c1d76c5f..1506481fc1ff7 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -881,10 +881,11 @@ class VisibleModuleSet { StringRef Message)>; /// Make a specific module visible. - void setVisible(Module *M, SourceLocation Loc, - VisibleCallback Vis = [](Module *) {}, - ConflictCallback Cb = [](ArrayRef, Module *, - StringRef) {}); + void setVisible( + Module *M, SourceLocation Loc, bool IncludeExports = true, + VisibleCallback Vis = [](Module *) {}, + ConflictCallback Cb = [](ArrayRef, Module *, StringRef) {}); + private: /// Import locations for each visible module. Indexed by the module's /// VisibilityID. diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h index 3d223c345ea15..1f8de1254249b 100644 --- a/clang/include/clang/Lex/Preprocessor.h +++ b/clang/include/clang/Lex/Preprocessor.h @@ -1755,7 +1755,8 @@ class Preprocessor { bool LexAfterModuleImport(Token &Result); void CollectPpImportSuffix(SmallVectorImpl &Toks); - void makeModuleVisible(Module *M, SourceLocation Loc); + void makeModuleVisible(Module *M, SourceLocation Loc, + bool IncludeExports = true); SourceLocation getModuleImportLoc(Module *M) const { return CurSubmoduleState->VisibleModules.getImportLoc(M); diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp index 330108d5b3e47..bb85a4010931f 100644 --- a/clang/lib/Basic/Module.cpp +++ b/clang/lib/Basic/Module.cpp @@ -662,7 +662,8 @@ LLVM_DUMP_METHOD void Module::dump() const { } void VisibleModuleSet::setVisible(Module *M, SourceLocation Loc, - VisibleCallback Vis, ConflictCallback Cb) { + bool IncludeExports, VisibleCallback Vis, + ConflictCallback Cb) { // We can't import a global module fragment so the location can be invalid. assert((M->isGlobalModule() || Loc.isValid()) && "setVisible expects a valid import location"); @@ -688,12 +689,14 @@ void VisibleModuleSet::setVisible(Module *M, SourceLocation Loc, Vis(V.M); // Make any exported modules visible. - SmallVector Exports; - V.M->getExportedModules(Exports); - for (Module *E : Exports) { - // Don't import non-importable modules. - if (!E->isUnimportable()) - VisitModule({E, &V}); + if (IncludeExports) { + SmallVector Exports; + V.M->getExportedModules(Exports); + for (Module *E : Exports) { + // Don't import non-importable modules. + if (!E->isUnimportable()) + VisitModule({E, &V}); + } } for (auto &C : V.M->Conflicts) { diff --git a/clang/lib/Lex/PPLexerChange.cpp b/clang/lib/Lex/PPLexerChange.cpp index 22d7c4e013273..e1dcc5499170e 100644 --- a/clang/lib/Lex/PPLexerChange.cpp +++ b/clang/lib/Lex/PPLexerChange.cpp @@ -754,9 +754,10 @@ void Preprocessor::EnterSubmodule(Module *M, SourceLocation ImportLoc, // Switch to this submodule as the current submodule. CurSubmoduleState = &State; - // This module is visible to itself. + // This module is visible to itself, but exports should not be made visible + // until they are imported. if (FirstTime) - makeModuleVisible(M, ImportLoc); + makeModuleVisible(M, ImportLoc, /*IncludeExports=*/false); } bool Preprocessor::needModuleMacros() const { diff --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp index 5ac5e6feae144..7256473d54ed2 100644 --- a/clang/lib/Lex/Preprocessor.cpp +++ b/clang/lib/Lex/Preprocessor.cpp @@ -1331,9 +1331,10 @@ bool Preprocessor::LexAfterModuleImport(Token &Result) { return true; } -void Preprocessor::makeModuleVisible(Module *M, SourceLocation Loc) { +void Preprocessor::makeModuleVisible(Module *M, SourceLocation Loc, + bool IncludeExports) { CurSubmoduleState->VisibleModules.setVisible( - M, Loc, [](Module *) {}, + M, Loc, IncludeExports, [](Module *) {}, [&](ArrayRef Path, Module *Conflict, StringRef Message) { // FIXME: Include the path in the diagnostic. // FIXME: Include the import location for the conflicting module. diff --git a/clang/test/Modules/local-submodule-visibility-transitive-import.c b/clang/test/Modules/local-submodule-visibility-transitive-import.c new file mode 100644 index 0000000000000..333d4860e9123 --- /dev/null +++ b/clang/test/Modules/local-submodule-visibility-transitive-import.c @@ -0,0 +1,62 @@ +// Checks that macros from transitive imports work with local submodule +// visibility. In the below test, previously a() and d() failed because +// OTHER_MACRO1 and OTHER_MACRO3 were not visible at the use site. + +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t \ +// RUN: -fmodules-local-submodule-visibility -I%t %t/tu.c -verify + +//--- Other1.h +#define OTHER_MACRO1(...) + +//--- Other2.h +#define OTHER_MACRO2(...) + +//--- Other3.h +#define OTHER_MACRO3(...) + +//--- module.modulemap +module Other { + module O1 { header "Other1.h" } + module O2 { header "Other2.h" } + module O3 { header "Other3.h" } +} + +//--- Top/A.h +#include "Other1.h" +#define MACRO_A OTHER_MACRO1(x, y) + +//--- Top/B.h +#include "Other2.h" +#define MACRO_B OTHER_MACRO2(x, y) + +//--- Top/C.h +#include "D.h" + +//--- Top/D.h +#include "Other3.h" +#define MACRO_D OTHER_MACRO3(x, y) + +//--- Top/Top.h +#include "A.h" +#include "B.h" +#include "C.h" + +void a() MACRO_A; +void b() MACRO_B; +void d() MACRO_D; + +//--- Top/module.modulemap +module Top { + umbrella header "Top.h" + module A { header "A.h" export * } + module D { header "D.h" export * } + module * { export * } + export * + export Other.O3 +} + +//--- tu.c +#include "Top/Top.h" +// expected-no-diagnostics