diff --git a/clang-tools-extra/clangd/unittests/ModulesTests.cpp b/clang-tools-extra/clangd/unittests/ModulesTests.cpp --- a/clang-tools-extra/clangd/unittests/ModulesTests.cpp +++ b/clang-tools-extra/clangd/unittests/ModulesTests.cpp @@ -26,7 +26,7 @@ void foo() {} )cpp"); TU.ExtraArgs.push_back("-fmodule-name=M"); - TU.ExtraArgs.push_back("-fmodule-map-file=m.modulemap"); + TU.ExtraArgs.push_back("-fmodule-map-file=" + testPath("m.modulemap")); TU.AdditionalFiles["Textual.h"] = "void foo();"; TU.AdditionalFiles["m.modulemap"] = R"modulemap( module M { @@ -39,6 +39,31 @@ TU.index(); } +// Verify that visibility of AST nodes belonging to modules, but loaded from +// preamble PCH, is restored. +TEST(Modules, PreambleBuildVisibility) { + TestTU TU = TestTU::withCode(R"cpp( + #include "module.h" + + foo x; +)cpp"); + TU.OverlayRealFileSystemForModules = true; + TU.ExtraArgs.push_back("-fmodules"); + TU.ExtraArgs.push_back("-fmodules-strict-decluse"); + TU.ExtraArgs.push_back("-Xclang"); + TU.ExtraArgs.push_back("-fmodules-local-submodule-visibility"); + TU.ExtraArgs.push_back("-fmodule-map-file=" + testPath("m.modulemap")); + TU.AdditionalFiles["module.h"] = R"cpp( + typedef int foo; +)cpp"; + TU.AdditionalFiles["m.modulemap"] = R"modulemap( + module M { + header "module.h" + } +)modulemap"; + EXPECT_TRUE(TU.build().getDiagnostics().empty()); +} + } // namespace } // namespace clangd } // namespace clang diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -1912,6 +1912,12 @@ bool isModuleVisible(const Module *M, bool ModulePrivate = false); + // When loading a non-modular PCH files, this is used to restore module + // visibility. + void makeModuleVisible(Module *Mod, SourceLocation ImportLoc) { + VisibleModules.setVisible(Mod, ImportLoc); + } + /// Determine whether a declaration is visible to name lookup. bool isVisible(const NamedDecl *D) { return D->isUnconditionallyVisible() || isVisibleSlow(D); diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -4987,10 +4987,10 @@ /*ImportLoc=*/Import.ImportLoc); if (Import.ImportLoc.isValid()) PP.makeModuleVisible(Imported, Import.ImportLoc); - // FIXME: should we tell Sema to make the module visible too? + // This updates visibility for Preprocessor only. For Sema, which can be + // nullptr here, we do the same later, in UpdateSema(). } } - ImportedModules.clear(); } void ASTReader::finalizeForWriting() { @@ -7942,6 +7942,15 @@ SemaObj->FpPragmaStack.CurrentPragmaLocation = FpPragmaCurrentLocation; } } + + // For non-modular AST files, restore visiblity of modules. + for (auto &Import : ImportedModules) { + if (Import.ImportLoc.isInvalid()) + continue; + if (Module *Imported = getSubmodule(Import.ID)) { + SemaObj->makeModuleVisible(Imported, Import.ImportLoc); + } + } } IdentifierInfo *ASTReader::get(StringRef Name) { diff --git a/clang/test/PCH/Inputs/modules/Foo.h b/clang/test/PCH/Inputs/modules/Foo.h --- a/clang/test/PCH/Inputs/modules/Foo.h +++ b/clang/test/PCH/Inputs/modules/Foo.h @@ -1 +1,3 @@ void make_foo(void); + +typedef int MyType; diff --git a/clang/test/PCH/preamble-modules.cpp b/clang/test/PCH/preamble-modules.cpp new file mode 100644 --- /dev/null +++ b/clang/test/PCH/preamble-modules.cpp @@ -0,0 +1,15 @@ +// Check that modules included in the preamble remain visible to the rest of the +// file. + +// RUN: rm -rf %t.mcp +// RUN: %clang_cc1 -emit-pch -o %t.pch %s -fmodules -fmodule-map-file=%S/Inputs/modules/module.modulemap -fmodules-local-submodule-visibility -fmodules-cache-path=%t.mcp +// RUN: %clang_cc1 -include-pch %t.pch %s -fmodules -fmodule-map-file=%S/Inputs/modules/module.modulemap -fmodules-local-submodule-visibility -fmodules-cache-path=%t.mcp + +#ifndef MAIN_FILE +#define MAIN_FILE +// Premable section. +#include "Inputs/modules/Foo.h" +#else +// Main section. +MyType foo; +#endif