[clang] 074f6fd - Revert "[C++20][Modules] Introduce an implementation module."
Mitch Phillips via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 27 05:02:59 PDT 2023
Author: Mitch Phillips
Date: 2023-03-27T05:01:53-07:00
New Revision: 074f6fd61d382ff6bf108472ea701d214b02f64b
URL: https://github.com/llvm/llvm-project/commit/074f6fd61d382ff6bf108472ea701d214b02f64b
DIFF: https://github.com/llvm/llvm-project/commit/074f6fd61d382ff6bf108472ea701d214b02f64b.diff
LOG: Revert "[C++20][Modules] Introduce an implementation module."
This reverts commit c6e9823724ef6bdfee262289ee34d162db436af0.
Reason: Broke the ASan buildbots, see https://reviews.llvm.org/D126959
(the original phabricator review) for more info.
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h
index c0c99eb8b6d62..387ce4d6e9b17 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -103,22 +103,16 @@ class alignas(8) Module {
/// 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 module implementation unit.
- ModuleImplementationUnit,
+ /// This is a C++ 20 header unit.
+ ModuleHeaderUnit,
/// This is a C++ 20 module partition interface.
ModulePartitionInterface,
@@ -175,16 +169,9 @@ class alignas(8) Module {
/// Does this Module scope describe part of the purview of a standard named
/// C++ module?
bool isModulePurview() const {
- switch (Kind) {
- case ModuleInterfaceUnit:
- case ModuleImplementationUnit:
- case ModulePartitionInterface:
- case ModulePartitionImplementation:
- case PrivateModuleFragment:
- return true;
- default:
- return false;
- }
+ return Kind == ModuleInterfaceUnit || Kind == ModulePartitionInterface ||
+ Kind == ModulePartitionImplementation ||
+ Kind == PrivateModuleFragment;
}
/// Does this Module scope describe a fragment of the global module within
@@ -574,11 +561,6 @@ class alignas(8) Module {
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.
diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h
index f155c609b06cb..a0ddd13c11bfd 100644
--- a/clang/include/clang/Lex/ModuleMap.h
+++ b/clang/include/clang/Lex/ModuleMap.h
@@ -560,11 +560,6 @@ class ModuleMap {
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.
@@ -574,13 +569,6 @@ class ModuleMap {
/// \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);
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 9f7d58a4a3cd6..92ca3772dbef8 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2274,10 +2274,6 @@ class Sema final {
};
/// 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].
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index cd786049f914e..56042e5fd252f 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -1600,7 +1600,6 @@ Module *Decl::getOwningModuleForLinkage(bool IgnoreLinkage) const {
return nullptr;
case Module::ModuleInterfaceUnit:
- case Module::ModuleImplementationUnit:
case Module::ModulePartitionInterface:
case Module::ModulePartitionImplementation:
return M;
diff --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index 9d7284cd0e37d..0d0b5707e605a 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -880,11 +880,9 @@ 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. Module implementation units behave the same
- // way as a non-modular TU with imports.
+ // with priority emitted above.
llvm::Function *Fn;
- if (CXX20ModuleInits && getContext().getNamedModuleForCodeGen() &&
- !getContext().getNamedModuleForCodeGen()->isModuleImplementation()) {
+ if (CXX20ModuleInits && getContext().getNamedModuleForCodeGen()) {
SmallString<256> InitFnName;
llvm::raw_svector_ostream Out(InitFnName);
cast<ItaniumMangleContext>(getCXXABI().getMangleContext())
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index bd1ee2a674abb..0e33e9632b3eb 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -548,8 +548,6 @@ 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
diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp
index 05d9fc8208b26..2aae41fe488ff 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -759,8 +759,6 @@ 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:
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index f2b2d0b8c69f1..8dead93b03734 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -888,30 +888,23 @@ ModuleMap::createPrivateModuleFragmentForInterfaceUnit(Module *Parent,
return Result;
}
-Module *ModuleMap::createModuleUnitWithKind(SourceLocation Loc, StringRef Name,
- Module::ModuleKind Kind) {
+Module *ModuleMap::createModuleForInterfaceUnit(SourceLocation Loc,
+ StringRef Name) {
+ assert(LangOpts.CurrentModule == Name && "module name mismatch");
+ assert(!Modules[Name] && "redefining existing module");
+
auto *Result =
new Module(Name, Loc, nullptr, /*IsFramework*/ false,
/*IsExplicit*/ false, NumCreatedModules++);
- Result->Kind = Kind;
+ Result->Kind = Module::ModuleInterfaceUnit;
+ Modules[Name] = SourceModule = Result;
- // Reparent any current global module fragment as a submodule of this module.
+ // Reparent the 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.
@@ -922,25 +915,6 @@ 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");
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index dd001dba2b912..64034393344f0 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -1661,19 +1661,13 @@ bool Sema::CheckRedeclarationModuleOwnership(NamedDecl *New, NamedDecl *Old) {
if (NewM == OldM)
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;
- }
+ // 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;
bool NewIsModuleInterface = NewM && NewM->isModulePurview();
bool OldIsModuleInterface = OldM && OldM->isModulePurview();
diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index c02b9d2ac25b0..8c120d278d634 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -298,8 +298,8 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
const_cast<LangOptions&>(getLangOpts()).CurrentModule = ModuleName;
auto &Map = PP.getHeaderSearchInfo().getModuleMap();
- Module *Mod; // The module we are creating.
- Module *Interface = nullptr; // The interface for an implementation.
+ Module *Mod;
+
switch (MDK) {
case ModuleDeclKind::Interface:
case ModuleDeclKind::PartitionInterface: {
@@ -336,19 +336,18 @@ 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 = "";
- Interface = getModuleLoader().loadModule(ModuleLoc, {ModuleNameLoc},
- Module::AllVisible,
- /*IsInclusionDirective=*/false);
+ const_cast<LangOptions&>(getLangOpts()).CurrentModule = "";
+ Mod = getModuleLoader().loadModule(ModuleLoc, {ModuleNameLoc},
+ Module::AllVisible,
+ /*IsInclusionDirective=*/false);
const_cast<LangOptions&>(getLangOpts()).CurrentModule = ModuleName;
- if (!Interface) {
+ if (!Mod) {
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:
@@ -387,31 +386,19 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
// statements, so imports are allowed.
ImportState = ModuleImportState::ImportAllowed;
- 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.
+ // 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.
return ConvertDeclToDeclGroup(Import);
}
+ getASTContext().setNamedModuleForCodeGen(Mod);
+
+ // FIXME: Create a ModuleDecl.
return nullptr;
}
@@ -437,17 +424,19 @@ Sema::ActOnPrivateModuleFragmentDecl(SourceLocation ModuleLoc,
Diag(ModuleScopes.back().BeginLoc, diag::note_previous_definition);
return nullptr;
- case Module::ModuleImplementationUnit:
+ case Module::ModuleInterfaceUnit:
+ break;
+ }
+
+ if (!ModuleScopes.back().ModuleInterface) {
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.
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index d89ee4e6199fc..f5691e99241c1 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -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, 4)); // Kind
+ Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); // Kind
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsFramework
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsExplicit
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsSystem
diff --git a/clang/test/CXX/module/basic/basic.def.odr/p4.cppm b/clang/test/CXX/module/basic/basic.def.odr/p4.cppm
index 487dbdef283ee..1542e532c635a 100644
--- a/clang/test/CXX/module/basic/basic.def.odr/p4.cppm
+++ b/clang/test/CXX/module/basic/basic.def.odr/p4.cppm
@@ -143,6 +143,9 @@ 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();
@@ -151,12 +154,8 @@ 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; // FIXME: will be visible after P2788R0
+ (void)&const_var_module_linkage;
}
//--- user.cpp
@@ -177,6 +176,5 @@ void use() {
(void)&inline_var_exported;
(void)&const_var_exported;
- // Internal-linkage declarations are not visible here.
// Module-linkage declarations are not visible here.
}
diff --git a/clang/test/CXX/module/basic/basic.link/p2.cppm b/clang/test/CXX/module/basic/basic.link/p2.cppm
index 19761fb3359ce..e04412ea08d4a 100644
--- a/clang/test/CXX/module/basic/basic.link/p2.cppm
+++ b/clang/test/CXX/module/basic/basic.link/p2.cppm
@@ -39,21 +39,19 @@ 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(); // expected-error {{no matching function for call to 'internal_linkage_fn'}}
+ 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;
}
diff --git a/clang/test/CodeGenCXX/module-intializer.cpp b/clang/test/CodeGenCXX/module-intializer.cpp
index d365d180ac59d..e5149401b467a 100644
--- a/clang/test/CodeGenCXX/module-intializer.cpp
+++ b/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=N.pcm -fmodule-file=O=O.pcm -fmodule-file=M:Part=M-part.pcm \
+// RUN: -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=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=M.pcm -S -emit-llvm -o - \
+// RUN: -fmodule-file=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=M.pcm -S -emit-llvm -o - \
+// RUN: -fmodule-file=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=O.pcm -fmodule-file=M:Part=M-part.pcm \
+// RUN: -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=M-part.pcm \
// RUN: -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-M
//--- N-h.h
More information about the cfe-commits
mailing list