[clang] 53a1314 - [C++20][Modules] Fix named module import diagnostics.
Iain Sandoe via cfe-commits
cfe-commits at lists.llvm.org
Sun Jan 22 02:22:58 PST 2023
Author: Iain Sandoe
Date: 2023-01-22T10:22:36Z
New Revision: 53a1314ed1b5021822071d3c7a751a5ec52619b7
URL: https://github.com/llvm/llvm-project/commit/53a1314ed1b5021822071d3c7a751a5ec52619b7
DIFF: https://github.com/llvm/llvm-project/commit/53a1314ed1b5021822071d3c7a751a5ec52619b7.diff
LOG: [C++20][Modules] Fix named module import diagnostics.
We have been incorrectly disallowing imports of named modules in the
global and private module fragments.
This addresses: https://github.com/llvm/llvm-project/issues/59688
Differential Revision: https://reviews.llvm.org/D140927
Added:
clang/test/Modules/cxx20-import-diagnostics-b.cpp
Modified:
clang/include/clang/Sema/Sema.h
clang/lib/Parse/Parser.cpp
clang/lib/Sema/SemaModule.cpp
clang/test/Modules/cxx20-import-diagnostics-a.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 7fbd8ef7e229e..3e3fb2b0cc56d 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3146,12 +3146,15 @@ class Sema final {
/// fragments and imports. If we are not parsing a C++20 TU, or we find
/// an error in state transition, the state is set to NotACXX20Module.
enum class ModuleImportState {
- FirstDecl, ///< Parsing the first decl in a TU.
- GlobalFragment, ///< after 'module;' but before 'module X;'
- ImportAllowed, ///< after 'module X;' but before any non-import decl.
- ImportFinished, ///< after any non-import decl.
- PrivateFragment, ///< after 'module :private;'.
- NotACXX20Module ///< Not a C++20 TU, or an invalid state was found.
+ FirstDecl, ///< Parsing the first decl in a TU.
+ GlobalFragment, ///< after 'module;' but before 'module X;'
+ ImportAllowed, ///< after 'module X;' but before any non-import decl.
+ ImportFinished, ///< after any non-import decl.
+ PrivateFragmentImportAllowed, ///< after 'module :private;' but before any
+ ///< non-import decl.
+ PrivateFragmentImportFinished, ///< after 'module :private;' but a
+ ///< non-import decl has already been seen.
+ NotACXX20Module ///< Not a C++20 TU, or an invalid state was found.
};
private:
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index 104836aca4a94..6db3dc3156fdf 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -750,6 +750,10 @@ bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result,
else if (ImportState == Sema::ModuleImportState::ImportAllowed)
// Non-imports disallow further imports.
ImportState = Sema::ModuleImportState::ImportFinished;
+ else if (ImportState ==
+ Sema::ModuleImportState::PrivateFragmentImportAllowed)
+ // Non-imports disallow further imports.
+ ImportState = Sema::ModuleImportState::PrivateFragmentImportFinished;
}
return false;
}
@@ -2427,7 +2431,9 @@ Parser::ParseModuleDecl(Sema::ModuleImportState &ImportState) {
SourceLocation PrivateLoc = ConsumeToken();
DiagnoseAndSkipCXX11Attributes();
ExpectAndConsumeSemi(diag::err_private_module_fragment_expected_semi);
- ImportState = Sema::ModuleImportState::PrivateFragment;
+ ImportState = ImportState == Sema::ModuleImportState::ImportAllowed
+ ? Sema::ModuleImportState::PrivateFragmentImportAllowed
+ : Sema::ModuleImportState::PrivateFragmentImportFinished;
return Actions.ActOnPrivateModuleFragmentDecl(ModuleLoc, PrivateLoc);
}
@@ -2544,23 +2550,28 @@ Decl *Parser::ParseModuleImport(SourceLocation AtLoc,
SeenError = false;
break;
case Sema::ModuleImportState::GlobalFragment:
- // We can only have pre-processor directives in the global module
- // fragment. We cannot import a named modules here, however we have a
- // header unit import.
- if (!HeaderUnit || HeaderUnit->Kind != Module::ModuleKind::ModuleHeaderUnit)
- Diag(ImportLoc, diag::err_import_in_wrong_fragment) << IsPartition << 0;
+ case Sema::ModuleImportState::PrivateFragmentImportAllowed:
+ // We can only have pre-processor directives in the global module fragment
+ // which allows pp-import, but not of a partition (since the global module
+ // does not have partitions).
+ // We cannot import a partition into a private module fragment, since
+ // [module.private.frag]/1 disallows private module fragments in a multi-
+ // TU module.
+ if (IsPartition || (HeaderUnit && HeaderUnit->Kind !=
+ Module::ModuleKind::ModuleHeaderUnit))
+ Diag(ImportLoc, diag::err_import_in_wrong_fragment)
+ << IsPartition
+ << (ImportState == Sema::ModuleImportState::GlobalFragment ? 0 : 1);
else
SeenError = false;
break;
case Sema::ModuleImportState::ImportFinished:
+ case Sema::ModuleImportState::PrivateFragmentImportFinished:
if (getLangOpts().CPlusPlusModules)
Diag(ImportLoc, diag::err_import_not_allowed_here);
else
SeenError = false;
break;
- case Sema::ModuleImportState::PrivateFragment:
- Diag(ImportLoc, diag::err_import_in_wrong_fragment) << IsPartition << 1;
- break;
}
if (SeenError) {
ExpectAndConsumeSemi(diag::err_module_expected_semi);
diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index 823a4e9a75743..f52c0247f01cd 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -591,9 +591,6 @@ DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,
(ModuleScopes.back().ModuleInterface ||
(getLangOpts().CPlusPlusModules &&
ModuleScopes.back().Module->isGlobalModule()))) {
- assert((!ModuleScopes.back().Module->isGlobalModule() ||
- Mod->Kind == Module::ModuleKind::ModuleHeaderUnit) &&
- "should only be importing a header unit into the GMF");
// Re-export the module if the imported module is exported.
// Note that we don't need to add re-exported module to Imports field
// since `Exports` implies the module is imported already.
diff --git a/clang/test/Modules/cxx20-import-diagnostics-a.cpp b/clang/test/Modules/cxx20-import-diagnostics-a.cpp
index d940722f8ee8e..c5d22805d29ea 100644
--- a/clang/test/Modules/cxx20-import-diagnostics-a.cpp
+++ b/clang/test/Modules/cxx20-import-diagnostics-a.cpp
@@ -95,14 +95,13 @@ import C; // expected-error {{imports must immediately follow the module declara
//--- import-diags-tu7.cpp
module;
-// We can only have preprocessor directives here, which permits an include-
-// translated header unit. However those are identified specifically by the
-// preprocessor; non-preprocessed user code should not contain an 'import' here.
-import B; // expected-error {{module imports cannot be in the global module fragment}}
-
+// We can only have preprocessor directives here, which permits
+// header units (include-translated or not) and named modules.
+import B;
export module D;
int delta ();
+// expected-no-diagnostics
//--- import-diags-tu8.cpp
@@ -112,7 +111,7 @@ int delta ();
module :private;
-import B; // expected-error {{module imports cannot be in the private module fragment}}
+import B; // expected-error {{imports must immediately follow the module declaration}}
//--- import-diags-tu9.cpp
diff --git a/clang/test/Modules/cxx20-import-diagnostics-b.cpp b/clang/test/Modules/cxx20-import-diagnostics-b.cpp
new file mode 100644
index 0000000000000..f252190b7e9c0
--- /dev/null
+++ b/clang/test/Modules/cxx20-import-diagnostics-b.cpp
@@ -0,0 +1,61 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/a.cpp -o %t/a.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/c.cpp \
+// RUN: -fmodule-file=%t/a.pcm -o %t/c.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/d.cpp \
+// RUN: -fmodule-file=%t/a.pcm -o %t/d.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/e.cpp \
+// RUN: -fmodule-file=%t/a.pcm -o %t/e.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/a-part.cpp \
+// RUN: -o %t/a-part.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/f.cpp \
+// RUN: -fmodule-file=%t/a.pcm -o %t/f.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/g.cpp \
+// RUN: -fmodule-file=%t/a.pcm -o %t/g.pcm -verify
+
+//--- a.cpp
+export module a;
+
+//--- b.hpp
+import a;
+
+//--- c.cpp
+module;
+#include "b.hpp"
+export module c;
+
+//--- d.cpp
+module;
+import a;
+
+export module d;
+
+//--- e.cpp
+export module e;
+
+module :private;
+import a;
+
+//--- a-part.cpp
+export module a:part;
+
+//--- f.cpp
+module;
+import :part ; // expected-error {{module partition imports cannot be in the global module fragment}}
+
+export module f;
+
+//--- g.cpp
+
+export module g;
+module :private;
+import :part; // expected-error {{module partition imports cannot be in the private module fragment}}
More information about the cfe-commits
mailing list