This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG53a1314ed1b5: [C++20][Modules] Fix named module import
diagnostics. (authored by iains).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140927/new/
https://reviews.llvm.org/D140927
Files:
clang/include/clang/Sema/Sema.h
clang/lib/Parse/Parser.cpp
clang/lib/Sema/SemaModule.cpp
clang/test/Modules/cxx20-import-diagnostics-a.cpp
clang/test/Modules/cxx20-import-diagnostics-b.cpp
Index: clang/test/Modules/cxx20-import-diagnostics-b.cpp
===================================================================
--- /dev/null
+++ 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}}
Index: clang/test/Modules/cxx20-import-diagnostics-a.cpp
===================================================================
--- clang/test/Modules/cxx20-import-diagnostics-a.cpp
+++ clang/test/Modules/cxx20-import-diagnostics-a.cpp
@@ -95,14 +95,13 @@
//--- 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 @@
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
Index: clang/lib/Sema/SemaModule.cpp
===================================================================
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -591,9 +591,6 @@
(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.
Index: clang/lib/Parse/Parser.cpp
===================================================================
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -750,6 +750,10 @@
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 @@
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 @@
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);
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3146,12 +3146,15 @@
/// 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:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
