[C++20][Modules][1/8] Track valid import state.

[iains]

Summary

In C++20 modules imports must be together and at the start of the module.
Rather than growing more ad-hoc flags to test state, this keeps track of the
phase of of a valid module TU (first decl, global module frag, module,
private module frag). If the phasing is broken (with some diagnostic) the
pattern does not conform to a valid C++20 module, and we set the state
accordingly.

We can thus issue diagnostics when imports appear in the wrong places and
decouple the C++20 modules state from other module variants (modules-ts and
clang modules). Additionally, we attempt to diagnose wrong imports before
trying to find the module where possible (the latter will generally emit an
unhelpful diagnostic about the module not being available).

Although this generally simplifies the handling of C++20 module import
diagnostics, the motivation was that, in particular, it allows detecting
invalid imports like:

import module A;

int some_decl();

import module B;

where being in a module purview is insufficient to identify them.

[iains]

Rebased onto other modules work.

[iains]

repushing the rebase with clang-format available.

[iains]

This is the first in a series of 8 patches to implement basic C++20 module partition support in clang. This is an enabler, particularly in forming diagnostics.

[urnathan]

Looks right to me -- I also implemented such a state machine in GCC's parser. There are a bunch of formatting issues to fix of course.

clang/include/clang/Basic/DiagnosticParseKinds.td

1543

"imports must immediately follow a module declaration"? (the contiguous isn't adding anything IMHO)

1544–1547

You could I suppose have a single diagnostuc and select between global & private?

[iains]

clang/include/clang/Basic/DiagnosticParseKinds.td

1543

um, maybe I need two diagnostics then - the "contiguous" aspect applies when imports are split by a non-import statement.

Selecting between the two case on the basis of a flag seems unproductive here. Would two diagnostics seem more reasonable to you?

1544–1547

good idea, I'll do that.

[urnathan]

clang/include/clang/Basic/DiagnosticParseKinds.td

1543

I don't follow. If the imports are split by a non-import, then the latter ones don't immediately follow the module declaration.

[ChuanqiXu]

clang/include/clang/Basic/DiagnosticParseKinds.td

1543

The imports could exist in non-module unit. A example could be found at: http://eel.is/c++draft/basic.lookup.general#example-1. So the wording might not be accurate. I think we could refer to the wording in the standard [[ http://eel.is/c++draft/module.import#1 | [module.import]p1 ]]:

In a module unit, all module-import-declarations and export-declarations exporting module-import-declarations shall appear before all other declarations in the declaration-seq of the translation-unit and of the private-module-fragment (if any).

If I don't misread this, I think it implies that imports could not be the first decl in non-module unit.

clang/test/Modules/cxx20-import-diagnostics-a.cpp

126

It should be allowed to import a module in non-module unit. For example:

import std;
int main() {
    std::cout << "Hello World.\n";
    return 0;
}

I guess we lack a test for this.

[ChuanqiXu]

BTW, I guess it would be helpful to add the index in the series in the title. Like [C++20][Modules] Track valid import state (1/8).

[iains]

Address review comments.

[iains]

also reformatted.

[urnathan]

thanks for indexing the subject lines!

[ChuanqiXu]

clang/include/clang/Sema/Sema.h

2955–2960

I guess it is worth to add comment for these fields. Also I could guess the meaning, I think it is better to comment the semantics accurately.

clang/lib/Parse/ParseObjc.cpp

82–83

We could remove the IS variable

clang/lib/Parse/Parser.cpp

913

I think it is better to:

SingleDecl = ParseModuleImport(SourceLocation(), ModuleImportState::NotACXX20Module);

2466

Generally, we would add a TODO or FIXME for such comments.
(This is not need to be addressed if the following patch would be landed quickly)

clang/lib/Sema/SemaModule.cpp

144–148

I feel better to add an assertion here:

assert(SeenGMF == GlobalModuleFragment && "msg);

[iains]

address review comments, rebase

[iains]

clang/lib/Parse/ParseObjc.cpp

82–83

ParseModuleImport takes a reference so that it can update the the ModuleImportState if an invalid state is detected in parsing. So, although the state variable is a dummy here, we cannot remove the IS var.

clang/lib/Parse/Parser.cpp

913

see comment above, ParseModuleImport takes a reference.

2466

I hope that the main changes will be landed soon, but adding the TODO anyway.

clang/lib/Sema/SemaModule.cpp

144–148

we also have to check that we are in C++20 modules mode, since implicit global module fragments are allowed elsewhere - the test below already guards on C++20 modules.

I've made this change, although the assert seems quite heavyweight.

[ChuanqiXu]

LGTM.

clang/lib/Sema/SemaModule.cpp

144–148

nit: Is the

(SeenGMF == (GlobalModuleFragment != nullptr))

not equal to:

SeenGMF == GlobalModuleFragment

? Or we could add a cast here:

SeenGMF == (bool) GlobalModuleFragment

[iains]

clang/lib/Sema/SemaModule.cpp

144–148

nit: Is the

SeenGMF == GlobalModuleFragment

clang complains of pointer / integer comparison.
I guess we could use the cast.

[iains]

amended an assert.

clang/include/clang/Basic/DiagnosticParseKinds.td

def err_global_module_introducer_not_at_start : Error<
   "at the start of the translation unit">;
 def err_module_fragment_exported : Error<
   "%select{global|private}0 module fragment cannot be exported">;
 def err_private_module_fragment_expected_semi : Error<
   "expected ';' after private module fragment declaration">;
 def err_missing_before_module_end : Error<"expected %0 at end of module">;
 def err_unsupported_module_partition : Error<
   "sorry, module partitions are not yet supported">;
+def err_import_not_allowed_here : Error<
+  "imports must immediately follow the module declaration">;
+def err_import_in_wrong_fragment : Error<
+  "module%select{| partition}0 imports cannot be in the %select{global|private}1 module fragment">;
 
 def err_export_empty : Error<"export declaration cannot be empty">;
 }
 
 let CategoryName = "Generics Issue" in {
 
 def err_objc_expected_type_parameter : Error<
   "expected type parameter name">;
 
 def err_objc_parameterized_implementation : Error<

clang/include/clang/Parse/Parser.h

public:
 
   // Parsing methods.
 
   /// Initialize - Warm up the parser.
   ///
   void Initialize();
 
   /// Parse the first top-level declaration in a translation unit.
-  bool ParseFirstTopLevelDecl(DeclGroupPtrTy &Result);
+  bool ParseFirstTopLevelDecl(DeclGroupPtrTy &Result,
+                              Sema::ModuleImportState &ImportState);
 
   /// ParseTopLevelDecl - Parse one top-level declaration. Returns true if
   /// the EOF was encountered.
-  bool ParseTopLevelDecl(DeclGroupPtrTy &Result, bool IsFirstDecl = false);
+  bool ParseTopLevelDecl(DeclGroupPtrTy &Result,
+                         Sema::ModuleImportState &ImportState);
   bool ParseTopLevelDecl() {
     DeclGroupPtrTy Result;
-    return ParseTopLevelDecl(Result);
+    Sema::ModuleImportState IS = Sema::ModuleImportState::NotACXX20Module;
+    return ParseTopLevelDecl(Result, IS);
   }
 
   /// ConsumeToken - Consume the current 'peek token' and lex the next one.
   /// This does not work with special tokens: string literals, code completion,
   /// annotation tokens and balanced tokens must be handled using the specific
   /// consume methods.
   /// Returns the location of the consumed token.
   SourceLocation ConsumeToken() {
Decl *ParseExplicitInstantiation(DeclaratorContext Context,
                                    AccessSpecifier AS = AS_none);
   // C++2a: Template, concept definition [temp]
   Decl *
   ParseConceptDefinition(const ParsedTemplateInfo &TemplateInfo,
                          SourceLocation &DeclEnd);
 
   //===--------------------------------------------------------------------===//
   // Modules
-  DeclGroupPtrTy ParseModuleDecl(bool IsFirstDecl);
-  Decl *ParseModuleImport(SourceLocation AtLoc);
+  DeclGroupPtrTy ParseModuleDecl(Sema::ModuleImportState &ImportState);
+  Decl *ParseModuleImport(SourceLocation AtLoc,
+                          Sema::ModuleImportState &ImportState);
   bool parseMisplacedModuleImport();
   bool tryParseMisplacedModuleImport() {
     tok::TokenKind Kind = Tok.getKind();
     if (Kind == tok::annot_module_begin || Kind == tok::annot_module_end ||
         Kind == tok::annot_module_include)
       return parseMisplacedModuleImport();
     return false;
   }

clang/include/clang/Sema/Sema.h

public:
   Decl *ActOnEmptyDeclaration(Scope *S, const ParsedAttributesView &AttrList,
                               SourceLocation SemiLoc);
 
   enum class ModuleDeclKind {
     Interface,      ///< 'export module X;'
     Implementation, ///< 'module X;'
   };
 
+  /// An enumeration to represent the transition of states in parsing module
+  /// 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.
+  };
+
   /// The parser has processed a module-declaration that begins the definition
   /// of a module interface or implementation.
   DeclGroupPtrTy ActOnModuleDecl(SourceLocation StartLoc,
                                  SourceLocation ModuleLoc, ModuleDeclKind MDK,
-                                 ModuleIdPath Path, bool IsFirstDecl);
+                                 ModuleIdPath Path,
+                                 ModuleImportState &ImportState);
 
   /// The parser has processed a global-module-fragment declaration that begins
   /// the definition of the global module fragment of the current module unit.
   /// \param ModuleLoc The location of the 'module' keyword.
   DeclGroupPtrTy ActOnGlobalModuleFragmentDecl(SourceLocation ModuleLoc);
 
   /// The parser has processed a private-module-fragment declaration that begins
   /// the definition of the private module fragment of the current module unit.

clang/lib/Interpreter/IncrementalParser.cpp

if (P->getCurToken().is(tok::eof)) {
     P->ExitScope();
     S.CurContext = nullptr;
     // Start a new PTU.
     P->EnterScope(Scope::DeclScope);
     S.ActOnTranslationUnitScope(P->getCurScope());
   }
 
   Parser::DeclGroupPtrTy ADecl;
-  for (bool AtEOF = P->ParseFirstTopLevelDecl(ADecl); !AtEOF;
-       AtEOF = P->ParseTopLevelDecl(ADecl)) {
+  Sema::ModuleImportState ImportState;
+  for (bool AtEOF = P->ParseFirstTopLevelDecl(ADecl, ImportState); !AtEOF;
+       AtEOF = P->ParseTopLevelDecl(ADecl, ImportState)) {
     // If we got a null return and something *was* parsed, ignore it.  This
     // is due to a top-level semicolon, an action override, or a parse error
     // skipping something.
     if (ADecl && !Consumer->HandleTopLevelDecl(ADecl.get()))
       return llvm::make_error<llvm::StringError>("Parsing failed. "
                                                  "The consumer rejected a decl",
                                                  std::error_code());
   }

clang/lib/Parse/ParseAST.cpp

void clang::ParseAST(Sema &S, bool PrintStats, bool SkipFunctionBodies) {
   // the source, or a PCH is being created with #pragma hdrstop with nothing
   // after the pragma, there won't be any tokens or a Lexer.
   bool HaveLexer = S.getPreprocessor().getCurrentLexer();
 
   if (HaveLexer) {
     llvm::TimeTraceScope TimeScope("Frontend");
     P.Initialize();
     Parser::DeclGroupPtrTy ADecl;
-    for (bool AtEOF = P.ParseFirstTopLevelDecl(ADecl); !AtEOF;
-         AtEOF = P.ParseTopLevelDecl(ADecl)) {
+    Sema::ModuleImportState ImportState;
+    for (bool AtEOF = P.ParseFirstTopLevelDecl(ADecl, ImportState); !AtEOF;
+         AtEOF = P.ParseTopLevelDecl(ADecl, ImportState)) {
       // If we got a null return and something *was* parsed, ignore it.  This
       // is due to a top-level semicolon, an action override, or a parse error
       // skipping something.
       if (ADecl && !Consumer->HandleTopLevelDecl(ADecl.get()))
         return;
     }
   }
 

clang/lib/Parse/ParseObjc.cpp

Parser::ParseObjCAtDirectives(ParsedAttributesWithRange &Attrs) {
   case tok::objc_synthesize:
     SingleDecl = ParseObjCPropertySynthesize(AtLoc);
     break;
   case tok::objc_dynamic:
     SingleDecl = ParseObjCPropertyDynamic(AtLoc);
     break;
   case tok::objc_import:
     if (getLangOpts().Modules || getLangOpts().DebuggerSupport) {
-      SingleDecl = ParseModuleImport(AtLoc);
+      Sema::ModuleImportState IS = Sema::ModuleImportState::NotACXX20Module;
+      SingleDecl = ParseModuleImport(AtLoc, IS);
       break;
     }
     Diag(AtLoc, diag::err_atimport);
     SkipUntil(tok::semi);
     return Actions.ConvertDeclToDeclGroup(nullptr);
   default:
     Diag(AtLoc, diag::err_unexpected_at);
     SkipUntil(tok::semi);

clang/lib/Parse/Parser.cpp

 ///   translation-unit:
 /// [C]     external-declaration
 /// [C]     translation-unit external-declaration
 /// [C++]   top-level-declaration-seq[opt]
 /// [C++20] global-module-fragment[opt] module-declaration
 ///                 top-level-declaration-seq[opt] private-module-fragment[opt]
 ///
 /// Note that in C, it is an error if there is no first declaration.
-bool Parser::ParseFirstTopLevelDecl(DeclGroupPtrTy &Result) {
+bool Parser::ParseFirstTopLevelDecl(DeclGroupPtrTy &Result,
+                                    Sema::ModuleImportState &ImportState) {
   Actions.ActOnStartOfTranslationUnit();
 
+  // For C++20 modules, a module decl must be the first in the TU.  We also
+  // need to track module imports.
+  ImportState = Sema::ModuleImportState::FirstDecl;
+  bool NoTopLevelDecls = ParseTopLevelDecl(Result, ImportState);
+
   // C11 6.9p1 says translation units must have at least one top-level
   // declaration. C++ doesn't have this restriction. We also don't want to
   // complain if we have a precompiled header, although technically if the PCH
   // is empty we should still emit the (pedantic) diagnostic.
   // If the main file is a header, we're only pretending it's a TU; don't warn.
-  bool NoTopLevelDecls = ParseTopLevelDecl(Result, true);
   if (NoTopLevelDecls && !Actions.getASTContext().getExternalSource() &&
       !getLangOpts().CPlusPlus && !getLangOpts().IsHeaderFile)
     Diag(diag::ext_empty_translation_unit);
 
   return NoTopLevelDecls;
 }
 
 /// ParseTopLevelDecl - Parse one top-level declaration, return whatever the
 /// action tells us to.  This returns true if the EOF was encountered.
 ///
 ///   top-level-declaration:
 ///           declaration
 /// [C++20]   module-import-declaration
-bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result, bool IsFirstDecl) {
+bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result,
+                               Sema::ModuleImportState &ImportState) {
   DestroyTemplateIdAnnotationsRAIIObj CleanupRAII(*this);
 
   // Skip over the EOF token, flagging end of previous input for incremental
   // processing
   if (PP.isIncrementalProcessingEnabled() && Tok.is(tok::eof))
     ConsumeToken();
 
   Result = nullptr;
case tok::kw_export:
 
     default:
       break;
     }
     break;
 
   case tok::kw_module:
   module_decl:
-    Result = ParseModuleDecl(IsFirstDecl);
+    Result = ParseModuleDecl(ImportState);
     return false;
 
-  // tok::kw_import is handled by ParseExternalDeclaration. (Under the Modules
-  // TS, an import can occur within an export block.)
+  case tok::kw_import:
   import_decl: {
-    Decl *ImportDecl = ParseModuleImport(SourceLocation());
+    Decl *ImportDecl = ParseModuleImport(SourceLocation(), ImportState);
     Result = Actions.ConvertDeclToDeclGroup(ImportDecl);
     return false;
   }
 
   case tok::annot_module_include:
     Actions.ActOnModuleInclude(Tok.getLocation(),
                                reinterpret_cast<Module *>(
                                    Tok.getAnnotationValue()));
     ConsumeAnnotationToken();
     return false;
 
   case tok::annot_module_begin:
     Actions.ActOnModuleBegin(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()));
     ConsumeAnnotationToken();
+    ImportState = Sema::ModuleImportState::NotACXX20Module;
     return false;
 
   case tok::eof:
     // Check whether -fmax-tokens= was reached.
     if (PP.getMaxTokens() != 0 && PP.getTokenCount() > PP.getMaxTokens()) {
       PP.Diag(Tok.getLocation(), diag::warn_max_tokens_total)
           << PP.getTokenCount() << PP.getMaxTokens();
       SourceLocation OverrideLoc = PP.getMaxTokensOverrideLoc();
bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result,
   default:
     break;
   }
 
   ParsedAttributesWithRange attrs(AttrFactory);
   MaybeParseCXX11Attributes(attrs);
 
   Result = ParseExternalDeclaration(attrs);
+  // An empty Result might mean a line with ';' or some parsing error, ignore
+  // it.
+  if (Result) {
+    if (ImportState == Sema::ModuleImportState::FirstDecl)
+      // First decl was not modular.
+      ImportState = Sema::ModuleImportState::NotACXX20Module;
+    else if (ImportState == Sema::ModuleImportState::ImportAllowed)
+      // Non-imports disallow further imports.
+      ImportState = Sema::ModuleImportState::ImportFinished;
+  }
   return false;
 }
 
 /// ParseExternalDeclaration:
 ///
 ///       external-declaration: [C99 6.9], declaration: [C++ dcl.dcl]
 ///         function-definition
 ///         declaration
if (CurParsedObjCImpl) {
       Actions.CodeCompleteObjCMethodDecl(getCurScope(),
                                          /*IsInstanceMethod=*/None,
                                          /*ReturnType=*/nullptr);
     }
     Actions.CodeCompleteOrdinaryName(
         getCurScope(),
         CurParsedObjCImpl ? Sema::PCC_ObjCImplementation : Sema::PCC_Namespace);
     return nullptr;
-  case tok::kw_import:
-    SingleDecl = ParseModuleImport(SourceLocation());
-    break;
+  case tok::kw_import: {
+    Sema::ModuleImportState IS = Sema::ModuleImportState::NotACXX20Module;
+    if (getLangOpts().CPlusPlusModules) {
+      llvm_unreachable("not expecting a c++20 import here");
+      ProhibitAttributes(attrs);
+    }
+    SingleDecl = ParseModuleImport(SourceLocation(), IS);
+  } break;
   case tok::kw_export:
     if (getLangOpts().CPlusPlusModules || getLangOpts().ModulesTS) {
+      ProhibitAttributes(attrs);
       SingleDecl = ParseExportDeclaration();
       break;
     }
     // This must be 'export template'. Parse it so we can diagnose our lack
     // of support.
     LLVM_FALLTHROUGH;
   case tok::kw_using:
   case tok::kw_namespace:

 ///
 ///   global-module-fragment:  [C++2a]
 ///     'module' ';' top-level-declaration-seq[opt]
 ///   module-declaration:      [C++2a]
 ///     'export'[opt] 'module' module-name module-partition[opt]
 ///            attribute-specifier-seq[opt] ';'
 ///   private-module-fragment: [C++2a]
 ///     'module' ':' 'private' ';' top-level-declaration-seq[opt]
-Parser::DeclGroupPtrTy Parser::ParseModuleDecl(bool IsFirstDecl) {
+Parser::DeclGroupPtrTy
+Parser::ParseModuleDecl(Sema::ModuleImportState &ImportState) {
   SourceLocation StartLoc = Tok.getLocation();
 
   Sema::ModuleDeclKind MDK = TryConsumeToken(tok::kw_export)
                                  ? Sema::ModuleDeclKind::Interface
                                  : Sema::ModuleDeclKind::Implementation;
 
   assert(
       (Tok.is(tok::kw_module) ||
        (Tok.is(tok::identifier) && Tok.getIdentifierInfo() == Ident_module)) &&
       "not a module declaration");
   SourceLocation ModuleLoc = ConsumeToken();
 
   // Attributes appear after the module name, not before.
   // FIXME: Suggest moving the attributes later with a fixit.
   DiagnoseAndSkipCXX11Attributes();
 
   // Parse a global-module-fragment, if present.
   if (getLangOpts().CPlusPlusModules && Tok.is(tok::semi)) {
     SourceLocation SemiLoc = ConsumeToken();
-    if (!IsFirstDecl) {
+    if (ImportState != Sema::ModuleImportState::FirstDecl) {
       Diag(StartLoc, diag::err_global_module_introducer_not_at_start)
         << SourceRange(StartLoc, SemiLoc);
       return nullptr;
     }
     if (MDK == Sema::ModuleDeclKind::Interface) {
       Diag(StartLoc, diag::err_module_fragment_exported)
         << /*global*/0 << FixItHint::CreateRemoval(StartLoc);
     }
+    ImportState = Sema::ModuleImportState::GlobalFragment;
     return Actions.ActOnGlobalModuleFragmentDecl(ModuleLoc);
   }
 
   // Parse a private-module-fragment, if present.
   if (getLangOpts().CPlusPlusModules && Tok.is(tok::colon) &&
       NextToken().is(tok::kw_private)) {
     if (MDK == Sema::ModuleDeclKind::Interface) {
       Diag(StartLoc, diag::err_module_fragment_exported)
         << /*private*/1 << FixItHint::CreateRemoval(StartLoc);
     }
     ConsumeToken();
     SourceLocation PrivateLoc = ConsumeToken();
     DiagnoseAndSkipCXX11Attributes();
     ExpectAndConsumeSemi(diag::err_private_module_fragment_expected_semi);
+    ImportState = Sema::ModuleImportState::PrivateFragment;
     return Actions.ActOnPrivateModuleFragmentDecl(ModuleLoc, PrivateLoc);
   }
 
   SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 2> Path;
   if (ParseModuleName(ModuleLoc, Path, /*IsImport*/false))
     return nullptr;
 
   // Parse the optional module-partition.
Parser::ParseModuleDecl(Sema::ModuleImportState &ImportState) {
 
   // We don't support any module attributes yet; just parse them and diagnose.
   ParsedAttributesWithRange Attrs(AttrFactory);
   MaybeParseCXX11Attributes(Attrs);
   ProhibitCXX11Attributes(Attrs, diag::err_attribute_not_module_attr);
 
   ExpectAndConsumeSemi(diag::err_module_expected_semi);
 
-  return Actions.ActOnModuleDecl(StartLoc, ModuleLoc, MDK, Path, IsFirstDecl);
+  return Actions.ActOnModuleDecl(StartLoc, ModuleLoc, MDK, Path, ImportState);
 }
 
 /// Parse a module import declaration. This is essentially the same for
 /// Objective-C and the C++ Modules TS, except for the leading '@' (in ObjC)
 /// and the trailing optional attributes (in C++).
 ///
 /// [ObjC]  @import declaration:
 ///           '@' 'import' module-name ';'
 /// [ModTS] module-import-declaration:
 ///           'import' module-name attribute-specifier-seq[opt] ';'
 /// [C++2a] module-import-declaration:
 ///           'export'[opt] 'import' module-name
 ///                   attribute-specifier-seq[opt] ';'
 ///           'export'[opt] 'import' module-partition
 ///                   attribute-specifier-seq[opt] ';'
 ///           'export'[opt] 'import' header-name
 ///                   attribute-specifier-seq[opt] ';'
-Decl *Parser::ParseModuleImport(SourceLocation AtLoc) {
+Decl *Parser::ParseModuleImport(SourceLocation AtLoc,
+                                Sema::ModuleImportState &ImportState) {
   SourceLocation StartLoc = AtLoc.isInvalid() ? Tok.getLocation() : AtLoc;
 
   SourceLocation ExportLoc;
   TryConsumeToken(tok::kw_export, ExportLoc);
 
   assert((AtLoc.isInvalid() ? Tok.isOneOf(tok::kw_import, tok::identifier)
                             : Tok.isObjCAtKeyword(tok::objc_import)) &&
          "Improper start to module import");
Decl *Parser::ParseModuleImport(SourceLocation AtLoc,
   ProhibitCXX11Attributes(Attrs, diag::err_attribute_not_import_attr);
 
   if (PP.hadModuleLoaderFatalFailure()) {
     // With a fatal failure in the module loader, we abort parsing.
     cutOffParsing();
     return nullptr;
   }
 
+  // Diagnose mis-imports.
+  bool SeenError = true;
+  switch (ImportState) {
+  case Sema::ModuleImportState::ImportAllowed:
+    SeenError = false;
+    break;
+  case Sema::ModuleImportState::FirstDecl:
+  case Sema::ModuleImportState::NotACXX20Module:
+    // TODO: These cases will be an error when partitions are implemented.
+    SeenError = false;
+    break;
+  case Sema::ModuleImportState::GlobalFragment:
+    // We can only have pre-processor directives in the global module
+    // fragment.  We can, however have a header unit import here.
+    if (!HeaderUnit)
+      // We do not have partition support yet, so first arg is 0.
+      Diag(ImportLoc, diag::err_import_in_wrong_fragment) << 0 << 0;
+    else
+      SeenError = false;
+    break;
+  case Sema::ModuleImportState::ImportFinished:
+    if (getLangOpts().CPlusPlusModules)
+      Diag(ImportLoc, diag::err_import_not_allowed_here);
+    else
+      SeenError = false;
+    break;
+  case Sema::ModuleImportState::PrivateFragment:
+    // We do not have partition support yet, so first arg is 0.
+    Diag(ImportLoc, diag::err_import_in_wrong_fragment) << 0 << 1;
+    break;
+  }
+  if (SeenError) {
+    ExpectAndConsumeSemi(diag::err_module_expected_semi);
+    return nullptr;
+  }
+
   DeclResult Import;
   if (HeaderUnit)
     Import =
         Actions.ActOnModuleImport(StartLoc, ExportLoc, ImportLoc, HeaderUnit);
   else if (!Path.empty())
     Import = Actions.ActOnModuleImport(StartLoc, ExportLoc, ImportLoc, Path);
   ExpectAndConsumeSemi(diag::err_module_expected_semi);
   if (Import.isInvalid())

clang/lib/Sema/SemaModule.cpp

Sema::ActOnGlobalModuleFragmentDecl(SourceLocation ModuleLoc) {
   auto *TU = Context.getTranslationUnitDecl();
   TU->setModuleOwnershipKind(Decl::ModuleOwnershipKind::Visible);
   TU->setLocalOwningModule(GlobalModule);
 
   // FIXME: Consider creating an explicit representation of this declaration.
   return nullptr;
 }
 
-Sema::DeclGroupPtrTy
-Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
-                      ModuleDeclKind MDK, ModuleIdPath Path, bool IsFirstDecl) {
+Sema::DeclGroupPtrTy Sema::ActOnModuleDecl(SourceLocation StartLoc,
+                                           SourceLocation ModuleLoc,
+                                           ModuleDeclKind MDK,
+                                           ModuleIdPath Path,
+                                           ModuleImportState &ImportState) {
   assert((getLangOpts().ModulesTS || getLangOpts().CPlusPlusModules) &&
          "should only have module decl in Modules TS or C++20");
 
+  bool IsFirstDecl = ImportState == ModuleImportState::FirstDecl;
+  bool SeenGMF = ImportState == ModuleImportState::GlobalFragment;
+  // If any of the steps here fail, we count that as invalidating C++20
+  // module state;
+  ImportState = ModuleImportState::NotACXX20Module;
+
   // A module implementation unit requires that we are not compiling a module
   // of any kind. A module interface unit requires that we are not compiling a
   // module map.
   switch (getLangOpts().getCompilingModule()) {
   case LangOptions::CMK_None:
     // It's OK to compile a module interface as a normal translation unit.
     break;
 
if (!ModuleScopes.empty() &&
     return nullptr;
   }
 
   // Find the global module fragment we're adopting into this module, if any.
   Module *GlobalModuleFragment = nullptr;
   if (!ModuleScopes.empty() &&
       ModuleScopes.back().Module->Kind == Module::GlobalModuleFragment)
     GlobalModuleFragment = ModuleScopes.back().Module;
 
+  assert((!getLangOpts().CPlusPlusModules ||
+          SeenGMF == (bool)GlobalModuleFragment) &&
+         "mismatched global module state");
+
   // In C++20, the module-declaration must be the first declaration if there
   // is no global module fragment.
-  if (getLangOpts().CPlusPlusModules && !IsFirstDecl && !GlobalModuleFragment) {
+  if (getLangOpts().CPlusPlusModules && !IsFirstDecl && !SeenGMF) {
     Diag(ModuleLoc, diag::err_module_decl_not_at_start);
     SourceLocation BeginLoc =
         ModuleScopes.empty()
             ? SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID())
             : ModuleScopes.back().BeginLoc;
     if (BeginLoc.isValid()) {
       Diag(BeginLoc, diag::note_global_module_introducer_missing)
           << FixItHint::CreateInsertion(BeginLoc, "module;\n");
Sema::DeclGroupPtrTy Sema::ActOnModuleDecl(SourceLocation StartLoc,
 
   // From now on, we have an owning module for all declarations we see.
   // However, those declarations are module-private unless explicitly
   // exported.
   auto *TU = Context.getTranslationUnitDecl();
   TU->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ModulePrivate);
   TU->setLocalOwningModule(Mod);
 
+  // We are in the module purview, but before any other (non import)
+  // statements, so imports are allowed.
+  ImportState = ModuleImportState::ImportAllowed;
+
   // FIXME: Create a ModuleDecl.
   return nullptr;
 }
 
 Sema::DeclGroupPtrTy
 Sema::ActOnPrivateModuleFragmentDecl(SourceLocation ModuleLoc,
                                      SourceLocation PrivateLoc) {
   // C++20 [basic.link]/2:
Sema::ActOnPrivateModuleFragmentDecl(SourceLocation ModuleLoc,
   // FIXME: Consider creating an explicit representation of this declaration.
   return nullptr;
 }
 
 DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,
                                    SourceLocation ExportLoc,
                                    SourceLocation ImportLoc,
                                    ModuleIdPath Path) {
-  // Flatten the module path for a Modules TS module name.
+  // Flatten the module path for a C++20 or Modules TS module name.
   std::pair<IdentifierInfo *, SourceLocation> ModuleNameLoc;
-  if (getLangOpts().ModulesTS) {
-    std::string ModuleName;
+  if (getLangOpts().CPlusPlusModules || getLangOpts().ModulesTS) {
     for (auto &Piece : Path) {
       if (!ModuleName.empty())
         ModuleName += ".";
       ModuleName += Piece.first->getName();
     }
     ModuleNameLoc = {PP.getIdentifierInfo(ModuleName), Path[0].second};
     Path = ModuleIdPath(ModuleNameLoc);
   }
 
+  // Diagnose self-import before attempting a load.
+  if (getLangOpts().CPlusPlusModules && isCurrentModulePurview() &&
+      getCurrentModule()->Name == ModuleName) {
+    Diag(ImportLoc, diag::err_module_self_import)
+        << ModuleName << getLangOpts().CurrentModule;
+    return true;
+  }
+
   Module *Mod =
       getModuleLoader().loadModule(ImportLoc, Path, Module::AllVisible,
                                    /*IsInclusionDirective=*/false);
   if (!Mod)
     return true;
 
   return ActOnModuleImport(StartLoc, ExportLoc, ImportLoc, Mod, Path);
 }
DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,
                                    Module *Mod, ModuleIdPath Path) {
   VisibleModules.setVisible(Mod, ImportLoc);
 
   checkModuleImportContext(*this, Mod, ImportLoc, CurContext);
 
   // FIXME: we should support importing a submodule within a different submodule
   // of the same top-level module. Until we do, make it an error rather than
   // silently ignoring the import.
-  // Import-from-implementation is valid in the Modules TS. FIXME: Should we
-  // warn on a redundant import of the current module?
-  // FIXME: Import of a module from an implementation partition of the same
-  // module is permitted.
-  if (Mod->getTopLevelModuleName() == getLangOpts().CurrentModule &&
+  // FIXME: Should we warn on a redundant import of the current module?
+  if (!getLangOpts().CPlusPlusModules &&
+      Mod->getTopLevelModuleName() == getLangOpts().CurrentModule &&
       (getLangOpts().isCompilingModule() || !getLangOpts().ModulesTS)) {
     Diag(ImportLoc, getLangOpts().isCompilingModule()
                         ? diag::err_module_self_import
                         : diag::err_module_import_in_implementation)
         << Mod->getFullModuleName() << getLangOpts().CurrentModule;
   }
 
   SmallVector<SourceLocation, 2> IdentifierLocs;

clang/test/Modules/cxx20-import-diagnostics-a.cpp

+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=0 -x c++ %s \
+// RUN:  -o %t/B.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=1 -x c++ %s \
+// RUN:  -o %t/C.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=2 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -fmodule-file=%t/C.pcm -o %t/AOK1.pcm
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=3 -x c++ %s \
+// RUN:  -fmodule-file=%t/AOK1.pcm -o %t/tu_3.s -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=4 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -fmodule-file=%t/C.pcm -o %t/BC.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=5 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -fmodule-file=%t/C.pcm -o %t/tu_5.s -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=6 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -o %t/D.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=7 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -o %t/D.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=8 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -o %t/tu_8.s -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=9 -x c++ %s \
+// RUN:  -o %t/B.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj -D TU=10 -x c++ %s \
+// RUN:  -fmodule-file=%t/C.pcm  -o %t/impl.o
+
+// Test diagnostics for incorrect module import sequences.
+
+#if TU == 0
+
+export module B;
+
+int foo ();
+
+// expected-no-diagnostics
+
+#elif TU == 1
+
+export module C;
+
+int bar ();
+
+// expected-no-diagnostics
+
+#elif TU == 2
+
+export module AOK1;
+
+import B;
+export import C;
+
+export int theAnswer ();
+
+// expected-no-diagnostics
+
+#elif TU == 3
+
+module;
+
+module AOK1;
+
+export import C; // expected-error {{export declaration can only be used within a module interface unit}}
+
+int theAnswer () { return 42; }
+
+#elif TU == 4
+
+export module BC;
+
+export import B;
+
+int foo () { return 10; }
+
+import C; // expected-error {{imports must immediately follow the module declaration}}
+
+#elif TU == 5
+
+module B; // implicitly imports B.
+
+int foo () { return 10; }
+
+import C; // expected-error {{imports must immediately follow the module declaration}}
+
+#elif TU == 6
+
+module;
+// We can only have preprocessor commands here, which could include 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}}
+
+export module D;
+
+int delta ();
+
+#elif TU == 7
+
+export module D;
+
+int delta ();
+
+module :private;
+
+import B; // expected-error {{module imports cannot be in the private module fragment}}
+
+#elif TU == 8
+
+module B;
+
+import B; // expected-error {{import of module 'B' appears within same top-level module 'B'}}
+
+#elif TU == 9
+
+export module B;
+
+import B; // expected-error {{import of module 'B' appears within same top-level module 'B'}}
+
+#elif TU == 10
+
+int x;
+
+import C;
+
+int baz() { return 6174; }
+
+// expected-no-diagnostics
+
+#else
+#error "no MODE set"
+#endif