iains updated this revision to Diff 508662.
iains added a comment.
rebased, reworked to have the module owned.
The implementation module needs to be owned by the mpodul map so that
it is released when done. Reworked the comments and assert to check the
main file presence.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126959/new/
https://reviews.llvm.org/D126959
Files:
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: clang/test/CodeGenCXX/module-intializer.cpp
===================================================================
--- clang/test/CodeGenCXX/module-intializer.cpp
+++ clang/test/CodeGenCXX/module-intializer.cpp
@@ -18,17 +18,17 @@
// 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
Index: clang/test/CXX/module/basic/basic.link/p2.cppm
===================================================================
--- clang/test/CXX/module/basic/basic.link/p2.cppm
+++ clang/test/CXX/module/basic/basic.link/p2.cppm
@@ -39,19 +39,21 @@
}
//--- 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: clang/test/CXX/module/basic/basic.def.odr/p4.cppm
===================================================================
--- clang/test/CXX/module/basic/basic.def.odr/p4.cppm
+++ clang/test/CXX/module/basic/basic.def.odr/p4.cppm
@@ -143,9 +143,6 @@
(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)&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)&inline_var_exported;
(void)&const_var_exported;
+ // Internal-linkage declarations are not visible here.
// Module-linkage declarations are not visible here.
}
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -2719,7 +2719,7 @@
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: clang/lib/Sema/SemaModule.cpp
===================================================================
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -298,8 +298,8 @@
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 @@
// 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 @@
// 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 @@
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: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1661,13 +1661,19 @@
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: clang/lib/Lex/ModuleMap.cpp
===================================================================
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -888,23 +888,30 @@
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,30 @@
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");
+
+ // Create an entry in the modules map to own the implementation unit module.
+ // User module names must not start with a period (so that this cannot clash
+ // with any legal user-defined module name).
+ StringRef IName = ".ImplementationUnit";
+ assert(!Modules[IName] && "multiple implementation units?");
+
+ auto *Result =
+ createModuleUnitWithKind(Loc, Name, Module::ModuleImplementationUnit);
+ Modules[IName] = SourceModule = Result;
+
+ // Check that the main file is present.
+ assert(SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()) &&
+ "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: clang/lib/Frontend/FrontendActions.cpp
===================================================================
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -759,6 +759,8 @@
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: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -548,6 +548,8 @@
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: clang/lib/CodeGen/CGDeclCXX.cpp
===================================================================
--- clang/lib/CodeGen/CGDeclCXX.cpp
+++ clang/lib/CodeGen/CGDeclCXX.cpp
@@ -880,9 +880,11 @@
// 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: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -1600,6 +1600,7 @@
return nullptr;
case Module::ModuleInterfaceUnit:
+ case Module::ModuleImplementationUnit:
case Module::ModulePartitionInterface:
case Module::ModulePartitionImplementation:
return M;
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2274,6 +2274,10 @@
};
/// 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: clang/include/clang/Lex/ModuleMap.h
===================================================================
--- clang/include/clang/Lex/ModuleMap.h
+++ clang/include/clang/Lex/ModuleMap.h
@@ -560,6 +560,11 @@
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 @@
/// \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: clang/include/clang/Basic/Module.h
===================================================================
--- clang/include/clang/Basic/Module.h
+++ clang/include/clang/Basic/Module.h
@@ -103,16 +103,22 @@
/// 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 @@
/// 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 @@
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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
