diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 7fbd8ef7e229e3..3e3fb2b0cc56dc 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 104836aca4a94a..6db3dc3156fdfd 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 823a4e9a75743e..f52c0247f01cdf 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 d940722f8ee8e7..c5d22805d29ea0 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 00000000000000..f252190b7e9c06 --- /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}}