[C++20][Modules] Introduce an implementation module.
authorIain Sandoe <iain@sandoe.co.uk>
Fri, 3 Jun 2022 09:43:38 +0000 (10:43 +0100)
committerIain Sandoe <iain@sandoe.co.uk>
Thu, 23 Mar 2023 12:47:44 +0000 (12:47 +0000)
We need to be able to distinguish individual TUs from the same module in cases
where TU-local entities either need to be hidden (or, for some cases of ADL in
template instantiation, need to be detected as exposures).

This creates a module type for the implementation which implicitly imports its
primary module interface per C++20:
[module.unit/8] 'A module-declaration that contains neither an export-keyword
nor a module-partition implicitly imports the primary module interface unit of
the module as if by a module-import-declaration.

Implementation modules are never serialized (-emit-module-interface for an
implementation unit is diagnosed and rejected).

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

14 files changed:
clang/include/clang/Basic/Module.h
clang/include/clang/Lex/ModuleMap.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/Decl.cpp
clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Frontend/FrontendActions.cpp
clang/lib/Lex/ModuleMap.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaModule.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/CXX/module/basic/basic.def.odr/p4.cppm
clang/test/CXX/module/basic/basic.link/p2.cppm
clang/test/CodeGenCXX/module-intializer.cpp

index 387ce4d..c0c99eb 100644 (file)
@@ -103,16 +103,22 @@ public:
   /// The location of the module definition.
   SourceLocation DefinitionLoc;
 
+  // FIXME: Consider if reducing the size of this enum (having Partition and
+  // Named modules only) then representing interface/implementation separately
+  // is more efficient.
   enum ModuleKind {
     /// This is a module that was defined by a module map and built out
     /// of header files.
     ModuleMapModule,
 
+    /// This is a C++ 20 header unit.
+    ModuleHeaderUnit,
+
     /// This is a C++20 module interface unit.
     ModuleInterfaceUnit,
 
-    /// This is a C++ 20 header unit.
-    ModuleHeaderUnit,
+    /// This is a C++20 module implementation unit.
+    ModuleImplementationUnit,
 
     /// This is a C++ 20 module partition interface.
     ModulePartitionInterface,
@@ -169,9 +175,16 @@ public:
   /// Does this Module scope describe part of the purview of a standard named
   /// C++ module?
   bool isModulePurview() const {
-    return Kind == ModuleInterfaceUnit || Kind == ModulePartitionInterface ||
-           Kind == ModulePartitionImplementation ||
-           Kind == PrivateModuleFragment;
+    switch (Kind) {
+    case ModuleInterfaceUnit:
+    case ModuleImplementationUnit:
+    case ModulePartitionInterface:
+    case ModulePartitionImplementation:
+    case PrivateModuleFragment:
+      return true;
+    default:
+      return false;
+    }
   }
 
   /// Does this Module scope describe a fragment of the global module within
@@ -561,6 +574,11 @@ public:
            Kind == ModulePartitionImplementation;
   }
 
+  /// Is this a module implementation.
+  bool isModuleImplementation() const {
+    return Kind == ModuleImplementationUnit;
+  }
+
   /// Is this module a header unit.
   bool isHeaderUnit() const { return Kind == ModuleHeaderUnit; }
   // Is this a C++20 module interface or a partition.
index a0ddd13..f155c60 100644 (file)
@@ -560,6 +560,11 @@ public:
   Module *createPrivateModuleFragmentForInterfaceUnit(Module *Parent,
                                                       SourceLocation Loc);
 
+  /// Create a new C++ module with the specified kind, and reparent any pending
+  /// global module fragment(s) to it.
+  Module *createModuleUnitWithKind(SourceLocation Loc, StringRef Name,
+                                   Module::ModuleKind Kind);
+
   /// Create a new module for a C++ module interface unit.
   /// The module must not already exist, and will be configured for the current
   /// compilation.
@@ -569,6 +574,13 @@ public:
   /// \returns The newly-created module.
   Module *createModuleForInterfaceUnit(SourceLocation Loc, StringRef Name);
 
+  /// Create a new module for a C++ module implementation unit.
+  /// The interface module for this implementation (implicitly imported) must
+  /// exist and be loaded and present in the modules map.
+  ///
+  /// \returns The newly-created module.
+  Module *createModuleForImplementationUnit(SourceLocation Loc, StringRef Name);
+
   /// Create a C++20 header unit.
   Module *createHeaderUnit(SourceLocation Loc, StringRef Name,
                            Module::Header H);
index 63ee0f0..277c02e 100644 (file)
@@ -2274,6 +2274,10 @@ private:
   };
   /// The modules we're currently parsing.
   llvm::SmallVector<ModuleScope, 16> ModuleScopes;
+
+  /// For an interface unit, this is the implicitly imported interface unit.
+  clang::Module *ThePrimaryInterface = nullptr;
+
   /// The explicit global module fragment of the current translation unit.
   /// The explicit Global Module Fragment, as specified in C++
   /// [module.global.frag].
index 56042e5..cd78604 100644 (file)
@@ -1600,6 +1600,7 @@ Module *Decl::getOwningModuleForLinkage(bool IgnoreLinkage) const {
     return nullptr;
 
   case Module::ModuleInterfaceUnit:
+  case Module::ModuleImplementationUnit:
   case Module::ModulePartitionInterface:
   case Module::ModulePartitionImplementation:
     return M;
index 0d0b570..9d7284c 100644 (file)
@@ -880,9 +880,11 @@ CodeGenModule::EmitCXXGlobalInitFunc() {
 
   // Include the filename in the symbol name. Including "sub_" matches gcc
   // and makes sure these symbols appear lexicographically behind the symbols
-  // with priority emitted above.
+  // with priority emitted above.  Module implementation units behave the same
+  // way as a non-modular TU with imports.
   llvm::Function *Fn;
-  if (CXX20ModuleInits && getContext().getNamedModuleForCodeGen()) {
+  if (CXX20ModuleInits && getContext().getNamedModuleForCodeGen() &&
+      !getContext().getNamedModuleForCodeGen()->isModuleImplementation()) {
     SmallString<256> InitFnName;
     llvm::raw_svector_ostream Out(InitFnName);
     cast<ItaniumMangleContext>(getCXXABI().getMangleContext())
index 0e33e96..bd1ee2a 100644 (file)
@@ -548,6 +548,8 @@ void CodeGenModule::Release() {
     GlobalTopLevelStmtBlockInFlight = {nullptr, nullptr};
   }
 
+  // Module implementations are initialized the same way as a regular TU that
+  // imports one or more modules.
   if (CXX20ModuleInits && Primary && Primary->isInterfaceOrPartition())
     EmitCXXModuleInitFunc(Primary);
   else
index 2aae41f..05d9fc8 100644 (file)
@@ -759,6 +759,8 @@ static StringRef ModuleKindName(Module::ModuleKind MK) {
     return "Module Map Module";
   case Module::ModuleInterfaceUnit:
     return "Interface Unit";
+  case Module::ModuleImplementationUnit:
+    return "Implementation Unit";
   case Module::ModulePartitionInterface:
     return "Partition Interface";
   case Module::ModulePartitionImplementation:
index 8dead93..f2b2d0b 100644 (file)
@@ -888,23 +888,30 @@ ModuleMap::createPrivateModuleFragmentForInterfaceUnit(Module *Parent,
   return Result;
 }
 
-Module *ModuleMap::createModuleForInterfaceUnit(SourceLocation Loc,
-                                                StringRef Name) {
-  assert(LangOpts.CurrentModule == Name && "module name mismatch");
-  assert(!Modules[Name] && "redefining existing module");
-
+Module *ModuleMap::createModuleUnitWithKind(SourceLocation Loc, StringRef Name,
+                                            Module::ModuleKind Kind) {
   auto *Result =
       new Module(Name, Loc, nullptr, /*IsFramework*/ false,
                  /*IsExplicit*/ false, NumCreatedModules++);
-  Result->Kind = Module::ModuleInterfaceUnit;
-  Modules[Name] = SourceModule = Result;
+  Result->Kind = Kind;
 
-  // Reparent the current global module fragment as a submodule of this module.
+  // Reparent any current global module fragment as a submodule of this module.
   for (auto &Submodule : PendingSubmodules) {
     Submodule->setParent(Result);
     Submodule.release(); // now owned by parent
   }
   PendingSubmodules.clear();
+  return Result;
+}
+
+Module *ModuleMap::createModuleForInterfaceUnit(SourceLocation Loc,
+                                                StringRef Name) {
+  assert(LangOpts.CurrentModule == Name && "module name mismatch");
+  assert(!Modules[Name] && "redefining existing module");
+
+  auto *Result =
+      createModuleUnitWithKind(Loc, Name, Module::ModuleInterfaceUnit);
+  Modules[Name] = SourceModule = Result;
 
   // Mark the main source file as being within the newly-created module so that
   // declarations and macros are properly visibility-restricted to it.
@@ -915,6 +922,25 @@ Module *ModuleMap::createModuleForInterfaceUnit(SourceLocation Loc,
   return Result;
 }
 
+Module *ModuleMap::createModuleForImplementationUnit(SourceLocation Loc,
+                                                     StringRef Name) {
+  assert(LangOpts.CurrentModule == Name && "module name mismatch");
+  // The interface for this implementation must exist and be loaded.
+  assert(Modules[Name] && Modules[Name]->Kind == Module::ModuleInterfaceUnit &&
+         "creating implementation module without an interface");
+
+  auto *Result =
+      createModuleUnitWithKind(Loc, Name, Module::ModuleImplementationUnit);
+  SourceModule = Result;
+
+  // Mark the main source file as being within the newly-created module so that
+  // declarations and macros are properly visibility-restricted to it.
+  auto *MainFile = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
+  assert(MainFile && "no input file for module implementation");
+
+  return Result;
+}
+
 Module *ModuleMap::createHeaderUnit(SourceLocation Loc, StringRef Name,
                                     Module::Header H) {
   assert(LangOpts.CurrentModule == Name && "module name mismatch");
index 6403439..dd001db 100644 (file)
@@ -1661,13 +1661,19 @@ bool Sema::CheckRedeclarationModuleOwnership(NamedDecl *New, NamedDecl *Old) {
   if (NewM == OldM)
     return false;
 
-  // Partitions are part of the module, but a partition could import another
-  // module, so verify that the PMIs agree.
-  if (NewM && OldM &&
-      (NewM->isModulePartition() || OldM->isModulePartition()) &&
-      NewM->getPrimaryModuleInterfaceName() ==
-          OldM->getPrimaryModuleInterfaceName())
-    return false;
+  if (NewM && OldM) {
+    // A module implementation unit has visibility of the decls in its
+    // implicitly imported interface.
+    if (NewM->isModuleImplementation() && OldM == ThePrimaryInterface)
+      return false;
+
+    // Partitions are part of the module, but a partition could import another
+    // module, so verify that the PMIs agree.
+    if ((NewM->isModulePartition() || OldM->isModulePartition()) &&
+        NewM->getPrimaryModuleInterfaceName() ==
+            OldM->getPrimaryModuleInterfaceName())
+      return false;
+  }
 
   bool NewIsModuleInterface = NewM && NewM->isModulePurview();
   bool OldIsModuleInterface = OldM && OldM->isModulePurview();
index 8c120d2..c02b9d2 100644 (file)
@@ -298,8 +298,8 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
   const_cast<LangOptions&>(getLangOpts()).CurrentModule = ModuleName;
 
   auto &Map = PP.getHeaderSearchInfo().getModuleMap();
-  Module *Mod;
-
+  Module *Mod;                 // The module we are creating.
+  Module *Interface = nullptr; // The interface for an implementation.
   switch (MDK) {
   case ModuleDeclKind::Interface:
   case ModuleDeclKind::PartitionInterface: {
@@ -336,18 +336,19 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
     // we're building if `LangOpts.CurrentModule` equals to 'ModuleName'.
     // Change the value for `LangOpts.CurrentModule` temporarily to make the
     // module loader work properly.
-    const_cast<LangOptions&>(getLangOpts()).CurrentModule = "";
-    Mod = getModuleLoader().loadModule(ModuleLoc, {ModuleNameLoc},
-                                       Module::AllVisible,
-                                       /*IsInclusionDirective=*/false);
+    const_cast<LangOptions &>(getLangOpts()).CurrentModule = "";
+    Interface = getModuleLoader().loadModule(ModuleLoc, {ModuleNameLoc},
+                                             Module::AllVisible,
+                                             /*IsInclusionDirective=*/false);
     const_cast<LangOptions&>(getLangOpts()).CurrentModule = ModuleName;
 
-    if (!Mod) {
+    if (!Interface) {
       Diag(ModuleLoc, diag::err_module_not_defined) << ModuleName;
       // Create an empty module interface unit for error recovery.
       Mod = Map.createModuleForInterfaceUnit(ModuleLoc, ModuleName);
+    } else {
+      Mod = Map.createModuleForImplementationUnit(ModuleLoc, ModuleName);
     }
-
   } break;
 
   case ModuleDeclKind::PartitionImplementation:
@@ -386,19 +387,31 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
   // statements, so imports are allowed.
   ImportState = ModuleImportState::ImportAllowed;
 
-  // For an implementation, We already made an implicit import (its interface).
-  // Make and return the import decl to be added to the current TU.
-  if (MDK == ModuleDeclKind::Implementation) {
-    // Make the import decl for the interface.
-    ImportDecl *Import =
-        ImportDecl::Create(Context, CurContext, ModuleLoc, Mod, Path[0].second);
-    // and return it to be added.
+  getASTContext().setNamedModuleForCodeGen(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);
+
+    // 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.
+    Context.addModuleInitializer(ModuleScopes.back().Module, Import);
+    Mod->Imports.insert(Interface); // As if we imported it.
+    // Also save this as a shortcut to checking for decls in the interface
+    ThePrimaryInterface = Interface;
+    // If we made an implicit import of the module interface, then return the
+    // imported module decl.
     return ConvertDeclToDeclGroup(Import);
   }
 
-  getASTContext().setNamedModuleForCodeGen(Mod);
-
-  // FIXME: Create a ModuleDecl.
   return nullptr;
 }
 
@@ -424,19 +437,17 @@ Sema::ActOnPrivateModuleFragmentDecl(SourceLocation ModuleLoc,
     Diag(ModuleScopes.back().BeginLoc, diag::note_previous_definition);
     return nullptr;
 
-  case Module::ModuleInterfaceUnit:
-    break;
-  }
-
-  if (!ModuleScopes.back().ModuleInterface) {
+  case Module::ModuleImplementationUnit:
     Diag(PrivateLoc, diag::err_private_module_fragment_not_module_interface);
     Diag(ModuleScopes.back().BeginLoc,
          diag::note_not_module_interface_add_export)
         << FixItHint::CreateInsertion(ModuleScopes.back().BeginLoc, "export ");
     return nullptr;
+
+  case Module::ModuleInterfaceUnit:
+    break;
   }
 
-  // FIXME: Check this isn't a module interface partition.
   // FIXME: Check that this translation unit does not import any partitions;
   // such imports would violate [basic.link]/2's "shall be the only module unit"
   // restriction.
index 9416040..3e40812 100644 (file)
@@ -2719,7 +2719,7 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
   Abbrev->Add(BitCodeAbbrevOp(SUBMODULE_DEFINITION));
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // ID
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Parent
-  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); // Kind
+  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 4)); // Kind
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsFramework
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsExplicit
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsSystem
index 1542e53..487dbde 100644 (file)
@@ -143,9 +143,6 @@ void use() {
   (void)&inline_var_exported;
   (void)&const_var_exported;
 
-  // CHECK: define {{.*}}@_ZL26used_static_module_linkagev
-  used_static_module_linkage();
-
   // CHECK: define linkonce_odr {{.*}}@_ZW6Module26used_inline_module_linkagev
   used_inline_module_linkage();
 
@@ -154,8 +151,12 @@ void use() {
 
   (void)&extern_var_module_linkage;
   (void)&inline_var_module_linkage;
+
+  // FIXME: Issue #61427 Internal-linkage declarations in the interface TU
+  // should not be not visible here.
   (void)&static_var_module_linkage; // FIXME: Should not be visible here.
-  (void)&const_var_module_linkage;
+
+  (void)&const_var_module_linkage; // FIXME: will be visible after P2788R0
 }
 
 //--- user.cpp
@@ -176,5 +177,6 @@ void use() {
   (void)&inline_var_exported;
   (void)&const_var_exported;
 
+  // Internal-linkage declarations are not visible here.
   // Module-linkage declarations are not visible here.
 }
index e04412e..19761fb 100644 (file)
@@ -39,19 +39,21 @@ void use() {
 }
 
 //--- M.cpp
-// expected-no-diagnostics
+
 module M;
 
-// FIXME: Use of internal linkage entities should be rejected.
 void use_from_module_impl() {
   external_linkage_fn();
   module_linkage_fn();
-  internal_linkage_fn();
+  internal_linkage_fn(); // expected-error {{no matching function for call to 'internal_linkage_fn'}}
   (void)external_linkage_class{};
   (void)module_linkage_class{};
-  (void)internal_linkage_class{};
   (void)external_linkage_var;
   (void)module_linkage_var;
+
+  // FIXME: Issue #61427 Internal-linkage declarations in the interface TU
+  // should not be not visible here.
+  (void)internal_linkage_class{};
   (void)internal_linkage_var;
 }
 
index e514940..d365d18 100644 (file)
 // RUN: -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-P
 
 // RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.cpp \
-// RUN: -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=M-part.pcm \
+// RUN: -fmodule-file=N=N.pcm -fmodule-file=O=O.pcm -fmodule-file=M:Part=M-part.pcm \
 // RUN:    -emit-module-interface -o M.pcm
 // RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.pcm -S -emit-llvm \
 // RUN:  -o - | FileCheck %s --check-prefix=CHECK-M
 
 // RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 useM.cpp \
-// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: -fmodule-file=M=M.pcm -S -emit-llvm  -o - \
 // RUN: | FileCheck %s --check-prefix=CHECK-USE
 
 // RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-impl.cpp \
-// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: -fmodule-file=M=M.pcm -S -emit-llvm  -o - \
 // RUN: | FileCheck %s --check-prefix=CHECK-IMPL
 
 // RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.cpp -S -emit-llvm \
@@ -41,7 +41,7 @@
 // RUN:   -o - | FileCheck %s --check-prefix=CHECK-P
 
 // RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.cpp \
-// RUN:   -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=M-part.pcm \
+// RUN:   -fmodule-file=N.pcm -fmodule-file=O=O.pcm -fmodule-file=M:Part=M-part.pcm \
 // RUN:   -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-M
 
 //--- N-h.h