iains created this revision.
Herald added a subscriber: ChuanqiXu.
Herald added a project: All.
iains edited the summary of this revision.
iains added a reviewer: ChuanqiXu.
iains added a subscriber: clang-modules.
iains published this revision for review.
iains added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
many thanks to Daniela Engert from bringing this to my attention (and providing
a reproducer test case).
The following test fails to compile TU b.cpp because we are not making the
transitively imported modules visible (per [module.import]/p7)
a.cppm:
export module a;
export int foo() {
return 42;
}
b.cppm:
export module b;
import a;
export int bar();
b.cpp:
module b;
int bar() {
return foo();
}
clang++ -c -std=c++2b -fmodule-output a.cppm
clang++ -c -std=c++2b -fmodule-output -fprebuilt-module-path=. b.cppm
clang++ -c -std=c++2b -fprebuilt-module-path=. b.cpp
b.cpp:4:12: error: declaration of 'foo' must be imported from module 'a'
before it is required
return foo();
This is fixed by the following patch (which also addresses a FIXME in
basic.def.odr/p6.cppm).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D152746
Files:
clang/include/clang/Basic/Module.h
clang/lib/Basic/Module.cpp
clang/lib/Sema/SemaModule.cpp
clang/test/CXX/module/basic/basic.def.odr/p6.cppm
clang/test/CXX/module/module.import/p7.cpp
Index: clang/test/CXX/module/module.import/p7.cpp
===================================================================
--- /dev/null
+++ clang/test/CXX/module/module.import/p7.cpp
@@ -0,0 +1,49 @@
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// All of the following should build without diagnostics.
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cpp -emit-module-interface -o %t/a.pcm
+// R U N: %clang_cc1 -std=c++20 %t/a.pcm -emit-obj -o %t/a.o
+//
+// RUN: %clang_cc1 -std=c++20 %t/b.cpp -emit-module-interface -o %t/b.pcm \
+// RUN: -fprebuilt-module-path=%t
+// R U N: %clang_cc1 -std=c++20 %t/b.pcm -emit-obj -o %t/b.o
+//
+// RUN: %clang_cc1 -std=c++20 %t/b-impl.cpp -emit-obj -o %t/b-impl.o \
+// RUN: -fprebuilt-module-path=%t
+//
+// RUN: %clang_cc1 -std=c++20 %t/ab-main.cpp -fsyntax-only \
+// RUN: -fprebuilt-module-path=%t
+
+//--- a.cpp
+
+export module a;
+
+export int foo() {
+ return 42;
+}
+
+//--- b.cpp
+
+export module b;
+import a;
+
+export int bar();
+
+//--- b-impl.cpp
+
+module b;
+
+int bar() {
+ return foo();
+}
+
+//--- ab-main.cpp
+
+import b;
+
+int main() {
+ return bar();
+}
+
Index: clang/test/CXX/module/basic/basic.def.odr/p6.cppm
===================================================================
--- clang/test/CXX/module/basic/basic.def.odr/p6.cppm
+++ clang/test/CXX/module/basic/basic.def.odr/p6.cppm
@@ -17,9 +17,8 @@
//
// RUN: %clang_cc1 -std=c++20 %t/module-vs-module.cpp -fmodule-file=M=%t/M.pcm -emit-module-interface -o %t/N.pcm -DMODULE_INTERFACE -DNO_ERRORS
// RUN: %clang_cc1 -std=c++20 %t/module-vs-module.cpp -fmodule-file=M=%t/M.pcm -fmodule-file=N=%t/N.pcm -verify
-// FIXME: Once we start importing "import" declarations properly, this should
-// be rejected (-verify should be added to the following line).
-// RUN: %clang_cc1 -std=c++20 %t/module-vs-module.cpp -fmodule-file=M=%t/M.pcm -fmodule-file=N=%t/N.pcm -DNO_IMPORT
+//
+// RUN: %clang_cc1 -std=c++20 %t/module-vs-module.cpp -fmodule-file=M=%t/M.pcm -fmodule-file=N=%t/N.pcm -DNO_IMPORT -verify
//
// RUN: %clang_cc1 -std=c++20 %t/module-vs-module.cpp -fmodule-file=M=%t/M.pcm -emit-module-interface -o %t/N-no-M.pcm -DMODULE_INTERFACE -DNO_ERRORS -DNO_IMPORT
// RUN: %clang_cc1 -std=c++20 %t/module-vs-module.cpp -fmodule-file=M=%t/M.pcm -fmodule-file=N=%t/N-no-M.pcm -verify
Index: clang/lib/Sema/SemaModule.cpp
===================================================================
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -397,6 +397,7 @@
if (Interface) {
VisibleModules.setVisible(Interface, ModuleLoc);
+ VisibleModules.makeTransitiveImportsVisible(Interface, ModuleLoc);
// Make the import decl for the interface in the impl module.
ImportDecl *Import = ImportDecl::Create(Context, CurContext, ModuleLoc,
Index: clang/lib/Basic/Module.cpp
===================================================================
--- clang/lib/Basic/Module.cpp
+++ clang/lib/Basic/Module.cpp
@@ -697,6 +697,13 @@
VisitModule({M, nullptr});
}
+void
+VisibleModuleSet::makeTransitiveImportsVisible(Module *M, SourceLocation Loc,
+ VisibleCallback Vis, ConflictCallback Cb) {
+ for (auto *I : M->Imports)
+ setVisible(I, Loc, Vis, Cb);
+}
+
ASTSourceDescriptor::ASTSourceDescriptor(Module &M)
: Signature(M.Signature), ClangModule(&M) {
if (M.Directory)
Index: clang/include/clang/Basic/Module.h
===================================================================
--- clang/include/clang/Basic/Module.h
+++ clang/include/clang/Basic/Module.h
@@ -822,6 +822,14 @@
ConflictCallback Cb = [](ArrayRef<Module *>, Module *,
StringRef) {});
+ /// Make transitive imports visible for [module.import]/7.
+ void makeTransitiveImportsVisible(Module *M, SourceLocation Loc,
+ VisibleCallback Vis = [](Module *) {},
+ ConflictCallback Cb = [](ArrayRef<Module *>,
+ Module *,
+ StringRef) {});
+
+
private:
/// Import locations for each visible module. Indexed by the module's
/// VisibilityID.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
