[clang] 4d62929 - [C++20] [Modules] Disambuguous Clang module and C++20 Named module further
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 12 22:58:17 PDT 2024
Author: Chuanqi Xu
Date: 2024-03-13T13:57:52+08:00
New Revision: 4d62929852849f768d7397f634cfdebc85de96a4
URL: https://github.com/llvm/llvm-project/commit/4d62929852849f768d7397f634cfdebc85de96a4
DIFF: https://github.com/llvm/llvm-project/commit/4d62929852849f768d7397f634cfdebc85de96a4.diff
LOG: [C++20] [Modules] Disambuguous Clang module and C++20 Named module further
This patch tries to make the boundary of clang module and C++20 named
module more clear.
The changes included:
- Rename `TranslationUnitKind::TU_Module` to
`TranslationUnitKind::TU_ClangModule`.
- Rename `Sema::ActOnModuleInclude` to `Sema::ActOnAnnotModuleInclude`.
- Rename `ActOnModuleBegin` to `Sema::ActOnAnnotModuleBegin`.
- Rename `Sema::ActOnModuleEnd` to `Sema::ActOnAnnotModuleEnd`.
- Removes a warning if we're trying to compile a non-module unit as
C++20 module unit. This is not actually useful and makes (the future)
implementation unnecessarily complex.
This patch meant to be a NFC fix. But it shows that it fixed a bug
suprisingly that previously we would surppress the unused-value warning
in named modules. Because it shares the same logic with clang modules,
which has headers semantics. This shows the change is meaningful.
Added:
Modified:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Frontend/FrontendActions.h
clang/include/clang/Sema/Sema.h
clang/lib/Parse/Parser.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaModule.cpp
clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm
clang/test/Modules/pr72828.cppm
Removed:
clang/test/Modules/missing-module-declaration.cppm
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c54105507753eb..d7ab1635cf12bc 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11516,8 +11516,6 @@ def err_module_not_defined : Error<
def err_module_redeclaration : Error<
"translation unit contains multiple module declarations">;
def note_prev_module_declaration : Note<"previous module declaration is here">;
-def err_module_declaration_missing : Error<
- "missing 'export module' declaration in module interface unit">;
def err_module_declaration_missing_after_global_module_introducer : Error<
"missing 'module' declaration at end of global module fragment "
"introduced here">;
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index 862952d336ef31..08fc706e3cbf74 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -560,11 +560,6 @@ class LangOptions : public LangOptionsBase {
return getCompilingModule() != CMK_None;
}
- /// Are we compiling a standard c++ module interface?
- bool isCompilingModuleInterface() const {
- return getCompilingModule() == CMK_ModuleInterface;
- }
-
/// Are we compiling a module implementation?
bool isCompilingModuleImplementation() const {
return !isCompilingModule() && !ModuleName.empty();
@@ -993,8 +988,8 @@ enum TranslationUnitKind {
/// not complete.
TU_Prefix,
- /// The translation unit is a module.
- TU_Module,
+ /// The translation unit is a clang module.
+ TU_ClangModule,
/// The translation unit is a is a complete translation unit that we might
/// incrementally extend later.
diff --git a/clang/include/clang/Frontend/FrontendActions.h b/clang/include/clang/Frontend/FrontendActions.h
index 8441af2ee3e718..a620ddfc40447d 100644
--- a/clang/include/clang/Frontend/FrontendActions.h
+++ b/clang/include/clang/Frontend/FrontendActions.h
@@ -125,7 +125,7 @@ class GenerateModuleAction : public ASTFrontendAction {
StringRef InFile) override;
TranslationUnitKind getTranslationUnitKind() override {
- return TU_Module;
+ return TU_ClangModule;
}
bool hasASTFileSupport() const override { return false; }
@@ -138,7 +138,9 @@ class GenerateInterfaceStubsAction : public ASTFrontendAction {
std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
StringRef InFile) override;
- TranslationUnitKind getTranslationUnitKind() override { return TU_Module; }
+ TranslationUnitKind getTranslationUnitKind() override {
+ return TU_ClangModule;
+ }
bool hasASTFileSupport() const override { return false; }
};
@@ -159,6 +161,8 @@ class GenerateModuleInterfaceAction : public GenerateModuleAction {
std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
StringRef InFile) override;
+ TranslationUnitKind getTranslationUnitKind() override { return TU_Complete; }
+
std::unique_ptr<raw_pwrite_stream>
CreateOutputFile(CompilerInstance &CI, StringRef InFile) override;
};
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index b226851f03038d..d6ab2b0c2def99 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8064,13 +8064,13 @@ class Sema final {
/// The parser has processed a module import translated from a
/// #include or similar preprocessing directive.
- void ActOnModuleInclude(SourceLocation DirectiveLoc, Module *Mod);
+ void ActOnAnnotModuleInclude(SourceLocation DirectiveLoc, Module *Mod);
void BuildModuleInclude(SourceLocation DirectiveLoc, Module *Mod);
/// The parsed has entered a submodule.
- void ActOnModuleBegin(SourceLocation DirectiveLoc, Module *Mod);
+ void ActOnAnnotModuleBegin(SourceLocation DirectiveLoc, Module *Mod);
/// The parser has left a submodule.
- void ActOnModuleEnd(SourceLocation DirectiveLoc, Module *Mod);
+ void ActOnAnnotModuleEnd(SourceLocation DirectiveLoc, Module *Mod);
/// Create an implicit import of the given module at the given
/// source location, for error recovery, if possible.
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index 1701d153bd0eca..cc0e41ed221c4f 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -685,7 +685,7 @@ bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result,
// FIXME: We need a better way to disambiguate C++ clang modules and
// standard C++ modules.
if (!getLangOpts().CPlusPlusModules || !Mod->isHeaderUnit())
- Actions.ActOnModuleInclude(Loc, Mod);
+ Actions.ActOnAnnotModuleInclude(Loc, Mod);
else {
DeclResult Import =
Actions.ActOnModuleImport(Loc, SourceLocation(), Loc, Mod);
@@ -697,15 +697,17 @@ bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result,
}
case tok::annot_module_begin:
- Actions.ActOnModuleBegin(Tok.getLocation(), reinterpret_cast<Module *>(
- Tok.getAnnotationValue()));
+ Actions.ActOnAnnotModuleBegin(
+ Tok.getLocation(),
+ reinterpret_cast<Module *>(Tok.getAnnotationValue()));
ConsumeAnnotationToken();
ImportState = Sema::ModuleImportState::NotACXX20Module;
return false;
case tok::annot_module_end:
- Actions.ActOnModuleEnd(Tok.getLocation(), reinterpret_cast<Module *>(
- Tok.getAnnotationValue()));
+ Actions.ActOnAnnotModuleEnd(
+ Tok.getLocation(),
+ reinterpret_cast<Module *>(Tok.getAnnotationValue()));
ConsumeAnnotationToken();
ImportState = Sema::ModuleImportState::NotACXX20Module;
return false;
@@ -2708,9 +2710,9 @@ bool Parser::parseMisplacedModuleImport() {
// happens.
if (MisplacedModuleBeginCount) {
--MisplacedModuleBeginCount;
- Actions.ActOnModuleEnd(Tok.getLocation(),
- reinterpret_cast<Module *>(
- Tok.getAnnotationValue()));
+ Actions.ActOnAnnotModuleEnd(
+ Tok.getLocation(),
+ reinterpret_cast<Module *>(Tok.getAnnotationValue()));
ConsumeAnnotationToken();
continue;
}
@@ -2720,18 +2722,18 @@ bool Parser::parseMisplacedModuleImport() {
return true;
case tok::annot_module_begin:
// Recover by entering the module (Sema will diagnose).
- Actions.ActOnModuleBegin(Tok.getLocation(),
- reinterpret_cast<Module *>(
- Tok.getAnnotationValue()));
+ Actions.ActOnAnnotModuleBegin(
+ Tok.getLocation(),
+ reinterpret_cast<Module *>(Tok.getAnnotationValue()));
ConsumeAnnotationToken();
++MisplacedModuleBeginCount;
continue;
case tok::annot_module_include:
// Module import found where it should not be, for instance, inside a
// namespace. Recover by importing the module.
- Actions.ActOnModuleInclude(Tok.getLocation(),
- reinterpret_cast<Module *>(
- Tok.getAnnotationValue()));
+ Actions.ActOnAnnotModuleInclude(
+ Tok.getLocation(),
+ reinterpret_cast<Module *>(Tok.getAnnotationValue()));
ConsumeAnnotationToken();
// If there is another module import, process it.
continue;
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 720d5fd5f0428d..cd0c42d5ffbacd 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1207,26 +1207,35 @@ void Sema::ActOnEndOfTranslationUnit() {
}
// A global-module-fragment is only permitted within a module unit.
- bool DiagnosedMissingModuleDeclaration = false;
if (!ModuleScopes.empty() && ModuleScopes.back().Module->Kind ==
Module::ExplicitGlobalModuleFragment) {
Diag(ModuleScopes.back().BeginLoc,
diag::err_module_declaration_missing_after_global_module_introducer);
- DiagnosedMissingModuleDeclaration = true;
- }
-
- if (TUKind == TU_Module) {
- // If we are building a module interface unit, we need to have seen the
- // module declaration by now.
- if (getLangOpts().getCompilingModule() ==
- LangOptions::CMK_ModuleInterface &&
- !isCurrentModulePurview() && !DiagnosedMissingModuleDeclaration) {
- // FIXME: Make a better guess as to where to put the module declaration.
- Diag(getSourceManager().getLocForStartOfFile(
- getSourceManager().getMainFileID()),
- diag::err_module_declaration_missing);
- }
+ }
+
+ // Now we can decide whether the modules we're building need an initializer.
+ if (Module *CurrentModule = getCurrentModule();
+ CurrentModule && CurrentModule->isInterfaceOrPartition()) {
+ auto DoesModNeedInit = [this](Module *M) {
+ if (!getASTContext().getModuleInitializers(M).empty())
+ return true;
+ for (auto [Exported, _] : M->Exports)
+ if (Exported->isNamedModuleInterfaceHasInit())
+ return true;
+ for (Module *I : M->Imports)
+ if (I->isNamedModuleInterfaceHasInit())
+ return true;
+
+ return false;
+ };
+ CurrentModule->NamedModuleHasInit =
+ DoesModNeedInit(CurrentModule) ||
+ llvm::any_of(CurrentModule->submodules(),
+ [&](auto *SubM) { return DoesModNeedInit(SubM); });
+ }
+
+ if (TUKind == TU_ClangModule) {
// If we are building a module, resolve all of the exported declarations
// now.
if (Module *CurrentModule = PP.getCurrentModule()) {
@@ -1251,28 +1260,6 @@ void Sema::ActOnEndOfTranslationUnit() {
}
}
- // Now we can decide whether the modules we're building need an initializer.
- if (Module *CurrentModule = getCurrentModule();
- CurrentModule && CurrentModule->isInterfaceOrPartition()) {
- auto DoesModNeedInit = [this](Module *M) {
- if (!getASTContext().getModuleInitializers(M).empty())
- return true;
- for (auto [Exported, _] : M->Exports)
- if (Exported->isNamedModuleInterfaceHasInit())
- return true;
- for (Module *I : M->Imports)
- if (I->isNamedModuleInterfaceHasInit())
- return true;
-
- return false;
- };
-
- CurrentModule->NamedModuleHasInit =
- DoesModNeedInit(CurrentModule) ||
- llvm::any_of(CurrentModule->submodules(),
- [&](auto *SubM) { return DoesModNeedInit(SubM); });
- }
-
// Warnings emitted in ActOnEndOfTranslationUnit() should be emitted for
// modules when they are built, not every time they are used.
emitAndClearUnusedLocalTypedefWarnings();
@@ -1358,7 +1345,7 @@ void Sema::ActOnEndOfTranslationUnit() {
// noise. Don't warn for a use from a module: either we should warn on all
// file-scope declarations in modules or not at all, but whether the
// declaration is used is immaterial.
- if (!Diags.hasErrorOccurred() && TUKind != TU_Module) {
+ if (!Diags.hasErrorOccurred() && TUKind != TU_ClangModule) {
// Output warning for unused file scoped decls.
for (UnusedFileScopedDeclsType::iterator
I = UnusedFileScopedDecls.begin(ExternalSource.get()),
diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index f08c1cb3a13ef5..2ddf9d70263a09 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -713,7 +713,7 @@ DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,
return Import;
}
-void Sema::ActOnModuleInclude(SourceLocation DirectiveLoc, Module *Mod) {
+void Sema::ActOnAnnotModuleInclude(SourceLocation DirectiveLoc, Module *Mod) {
checkModuleImportContext(*this, Mod, DirectiveLoc, CurContext, true);
BuildModuleInclude(DirectiveLoc, Mod);
}
@@ -723,9 +723,9 @@ void Sema::BuildModuleInclude(SourceLocation DirectiveLoc, Module *Mod) {
// in that buffer do not qualify as module imports; they're just an
// implementation detail of us building the module.
//
- // FIXME: Should we even get ActOnModuleInclude calls for those?
+ // FIXME: Should we even get ActOnAnnotModuleInclude calls for those?
bool IsInModuleIncludes =
- TUKind == TU_Module &&
+ TUKind == TU_ClangModule &&
getSourceManager().isWrittenInMainFile(DirectiveLoc);
// If we are really importing a module (not just checking layering) due to an
@@ -752,7 +752,7 @@ void Sema::BuildModuleInclude(SourceLocation DirectiveLoc, Module *Mod) {
}
}
-void Sema::ActOnModuleBegin(SourceLocation DirectiveLoc, Module *Mod) {
+void Sema::ActOnAnnotModuleBegin(SourceLocation DirectiveLoc, Module *Mod) {
checkModuleImportContext(*this, Mod, DirectiveLoc, CurContext, true);
ModuleScopes.push_back({});
@@ -776,7 +776,7 @@ void Sema::ActOnModuleBegin(SourceLocation DirectiveLoc, Module *Mod) {
}
}
-void Sema::ActOnModuleEnd(SourceLocation EomLoc, Module *Mod) {
+void Sema::ActOnAnnotModuleEnd(SourceLocation EomLoc, Module *Mod) {
if (getLangOpts().ModulesLocalVisibility) {
VisibleModules = std::move(ModuleScopes.back().OuterVisibleModules);
// Leaving a module hides namespace names, so our visible namespace cache
diff --git a/clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm b/clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm
index 3072b760f9d7dd..1a01ffac0154ae 100644
--- a/clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm
+++ b/clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm
@@ -15,10 +15,6 @@ module A; // #module-decl
// expected-error at -2 {{missing 'export' specifier in module declaration while building module interface}}
#define INTERFACE
#endif
-#else
- #ifdef BUILT_AS_INTERFACE
- // expected-error at 1 {{missing 'export module' declaration in module interface unit}}
- #endif
#endif
#ifndef INTERFACE
diff --git a/clang/test/Modules/missing-module-declaration.cppm b/clang/test/Modules/missing-module-declaration.cppm
deleted file mode 100644
index d52f6639fe4f64..00000000000000
--- a/clang/test/Modules/missing-module-declaration.cppm
+++ /dev/null
@@ -1,13 +0,0 @@
-// RUN: rm -rf %t
-// RUN: split-file %s %t
-// RUN: cd %t
-//
-// RUN: %clang_cc1 -std=c++20 %t/B.cppm -I%t -emit-module-interface -o %t/B.pcm
-// RUN: %clang_cc1 -std=c++20 %t/A.cppm -I%t -fprebuilt-module-path=%t -emit-module-interface -verify
-
-//--- A.cppm
-import B; // expected-error{{missing 'export module' declaration in module interface unit}}
-
-//--- B.cppm
-module;
-export module B;
diff --git a/clang/test/Modules/pr72828.cppm b/clang/test/Modules/pr72828.cppm
index 574523188507a1..7432f2831f248a 100644
--- a/clang/test/Modules/pr72828.cppm
+++ b/clang/test/Modules/pr72828.cppm
@@ -17,7 +17,7 @@ struct s {
void f() {
auto [x] = s();
- [x] {};
+ (void) [x] {};
}
// Check that we can generate the LLVM IR expectedly.
More information about the cfe-commits
mailing list