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

[iains]

Summary

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

[iains]

many thanks to Daniela Engert from bringing this to my attention (and providing a reproducer test case).

[ChuanqiXu]

LGTM. Thanks.

[iains]

rebased and fixed some formatting.

clang/include/clang/Basic/Module.h

using ConflictCallback =
                          StringRef Message)>;
 
   /// Make a specific module visible.
   void setVisible(Module *M, SourceLocation Loc,
                   VisibleCallback Vis = [](Module *) {},
                   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.
   std::vector<SourceLocation> ImportLocs;
 
   /// Visibility generation, bumped every time the visibility state changes.
   unsigned Generation = 0;
 };

clang/lib/Basic/Module.cpp

for (auto &C : V.M->Conflicts) {
           Path.push_back(I->M);
         Cb(Path, C.Other, C.Message);
       }
     }
   };
   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)
     Path = M.Directory->getName();
   if (auto File = M.getASTFile())
     ASTFile = File->getName();
 }
 
 std::string ASTSourceDescriptor::getModuleName() const {
   if (ClangModule)
     return ClangModule->Name;
   else
     return std::string(PCHModuleName);
 }

clang/lib/Sema/SemaModule.cpp

Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
   getASTContext().setCurrentNamedModule(Mod);
 
   // We already potentially made an implicit import (in the case of a module
   // implementation unit importing its interface).  Make this module visible
   // and return the import decl to be added to the current TU.
   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,
                                             Interface, Path[0].second);
     CurContext->addDecl(Import);
 
     // Sequence initialization of the imported module before that of the current
     // module, if any.

clang/test/CXX/module/basic/basic.def.odr/p6.cppm

 // Some of the following tests intentionally have no -verify in their RUN
 // lines; we are testing that those cases do not produce errors.
 //
 // RUN: %clang_cc1 -std=c++20 %t/module-vs-module.cpp -fmodule-file=M=%t/M.pcm -DMODULE_INTERFACE -verify
 // RUN: %clang_cc1 -std=c++20 %t/module-vs-module.cpp -fmodule-file=M=%t/M.pcm -DMODULE_INTERFACE -DNO_IMPORT
 //
 // 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
 // RUN: %clang_cc1 -std=c++20 %t/module-vs-module.cpp -fmodule-file=N=%t/N-no-M.pcm -DNO_IMPORT
 
 //--- global-vs-module.cppm
 #ifndef NO_GLOBAL
 module;

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

+// 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();
+}
+