[clang] b37233a - [C++20][Modules] Complete implementation of module.import p7.

[Iain Sandoe via cfe-commits]

Author: Iain Sandoe
Date: 2023-06-25T08:33:39+01:00
New Revision: b37233a253f30e4bd5f040d598826df443293bee

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

LOG: [C++20][Modules] Complete implementation of module.import p7.

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).

Differential Revision: https://reviews.llvm.org/D152746

Added: 
    clang/test/CXX/module/module.import/p7.cpp

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h
index 399ed92a325e5..b3b5376160107 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -822,6 +822,11 @@ class VisibleModuleSet {
                   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.

diff  --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 3f9b8d0c775b9..2bdbe8d2b110d 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -695,6 +695,14 @@ void VisibleModuleSet::setVisible(Module *M, SourceLocation Loc,
   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)

diff  --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index 9b2982e7fa4df..2ffb0dbddd537 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -397,6 +397,7 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
   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,

diff  --git a/clang/test/CXX/module/basic/basic.def.odr/p6.cppm b/clang/test/CXX/module/basic/basic.def.odr/p6.cppm
index d84c05ce32cda..8e7917dc63ea5 100644
--- a/clang/test/CXX/module/basic/basic.def.odr/p6.cppm
+++ b/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

diff  --git a/clang/test/CXX/module/module.import/p7.cpp b/clang/test/CXX/module/module.import/p7.cpp
new file mode 100644
index 0000000000000..2de6f0bd65c27
--- /dev/null
+++ b/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();
+}
+