https://github.com/yronglin created https://github.com/llvm/llvm-project/pull/187846
The PR https://github.com/llvm/llvm-project/pull/107168 implement [CWG2947](https://cplusplus.github.io/CWG/issues/2947.html). And clang checks next pp-token after module name in phase 4. This PR move the check to phase 7, there are three main motivations: 1. We already convert the module name pp-tokens sequence into a annot_module_name token, so for the following case, macro expansion will be disabled. 2. In the original implementation, we need to look ahead a token even if we already hit `eod`, but in phase7, we don't need to look ahead a token. 3. The original implementation checked and emit a diagnosis in phase 4, and then skipped these unexpected tokens in phase 7. Now we've merged them into one step and are able to get a more approximate diagnosis. ```cpp // #define m(x) x export module m (foo); // Before: unexpected preprocessing token '(' after module name ..... // After: unexpected '(' after module name .... ``` But in the following will be breaking current behavior: ```cpp // #define m(x) x // #define LPAREN ( export module m LPAREN foo); // Before: unexpected preprocessing token 'LPAREN' after module name ..... // After: unexpected '(' after module name .... ``` >From 716f4c45eff998a5f29cdc04c5a5bb58d0793208 Mon Sep 17 00:00:00 2001 From: yronglin <[email protected]> Date: Sat, 21 Mar 2026 15:54:55 +0800 Subject: [PATCH] [clang] Diag unexpected token after module name in phase 7 Signed-off-by: yronglin <[email protected]> --- clang/docs/ReleaseNotes.rst | 1 + clang/include/clang/Basic/DiagnosticLexKinds.td | 3 --- clang/include/clang/Basic/DiagnosticParseKinds.td | 3 +++ clang/lib/Lex/PPDirectives.cpp | 15 --------------- clang/lib/Parse/Parser.cpp | 6 ++++-- clang/test/CXX/basic/basic.link/p3.cpp | 2 +- clang/test/CXX/drs/cwg2947.cpp | 4 ++-- .../basic/basic.link/module-declaration.cpp | 2 +- clang/test/CXX/module/cpp.pre/p1.cpp | 6 +++--- 9 files changed, 15 insertions(+), 27 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index b13171f7ecafc..9e2531e2c5645 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -339,6 +339,7 @@ Bug Fixes in This Version - Fixed a crash when normalizing constraints involving concept template parameters whose index coincided with non-concept template parameters in the same parameter mapping. - Fixed a crash caused by accessing dependent diagnostics of a non-dependent context. - Fixed a crash when substituting into a non-type template parameter that has a type containing an undeduced placeholder type. +- Fixed a crash when module directive export module foo not following a semicolon and there are no rest pp-tokens in current module file. (#GH187771) Bug Fixes to Compiler Builtins ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td index 5eceeced311f2..ed448fdfb2011 100644 --- a/clang/include/clang/Basic/DiagnosticLexKinds.td +++ b/clang/include/clang/Basic/DiagnosticLexKinds.td @@ -1006,9 +1006,6 @@ def err_pp_module_name_is_macro : Error< "%select{module|partition}0 name component %1 cannot be a object-like macro">; def err_pp_module_expected_ident : Error< "expected %select{identifier after '.' in |}0module name">; -def err_pp_unexpected_tok_after_module_name : Error< - "unexpected preprocessing token '%0' after module name, " - "only ';' and '[' (start of attribute specifier sequence) are allowed">; def warn_pp_extra_tokens_at_module_directive_eol : Warning<"extra tokens after semicolon in '%0' directive">, InGroup<ExtraTokens>; diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td index 19edc7f7a3b23..42d2f20f8c8c3 100644 --- a/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -1782,6 +1782,9 @@ def ext_bit_int : Extension< } // end of Parse Issue category. let CategoryName = "Modules Issue" in { +def err_unexpected_tok_after_module_name : Error< + "unexpected '%0' after module name, only ';' and '[' (start of attribute " + "specifier sequence) are allowed">; def err_unexpected_module_or_import_decl : Error< "%select{module|import}0 declaration can only appear at the top level">; def err_attribute_not_module_attr : Error< diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 09f0c3fe8c61e..2a1b2792ae548 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -4405,21 +4405,6 @@ void Preprocessor::HandleCXXModuleDirective(Token ModuleTok) { break; } - // Consume the pp-import-suffix and expand any macros in it now, if we're not - // at the semicolon already. - std::optional<Token> NextPPTok = DirToks.back(); - if (DirToks.back().is(tok::eod)) { - NextPPTok = peekNextPPToken(); - if (NextPPTok && NextPPTok->is(tok::raw_identifier)) - LookUpIdentifierInfo(*NextPPTok); - } - - // Only ';' and '[' are allowed after module name. - // We also check 'private' because the previous is not a module name. - if (!NextPPTok->isOneOf(tok::semi, tok::eod, tok::l_square, tok::kw_private)) - Diag(*NextPPTok, diag::err_pp_unexpected_tok_after_module_name) - << getSpelling(*NextPPTok); - if (!DirToks.back().isOneOf(tok::semi, tok::eod)) { // Consume the pp-import-suffix and expand any macros in it now. We'll add // it back into the token stream later. diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp index 5d18414b1a746..61b64133d35d2 100644 --- a/clang/lib/Parse/Parser.cpp +++ b/clang/lib/Parse/Parser.cpp @@ -2375,9 +2375,11 @@ Parser::ParseModuleDecl(Sema::ModuleImportState &ImportState) { } // This should already diagnosed in phase 4, just skip unil semicolon. - if (!Tok.isOneOf(tok::semi, tok::l_square)) + if (!Tok.isOneOf(tok::semi, tok::l_square)) { + Diag(Tok, diag::err_unexpected_tok_after_module_name) + << PP.getSpelling(Tok); SkipUntil(tok::semi, SkipUntilFlags::StopBeforeMatch); - + } // We don't support any module attributes yet; just parse them and diagnose. ParsedAttributes Attrs(AttrFactory); MaybeParseCXX11Attributes(Attrs); diff --git a/clang/test/CXX/basic/basic.link/p3.cpp b/clang/test/CXX/basic/basic.link/p3.cpp index bc3622c7bbd64..0784a7d879d7d 100644 --- a/clang/test/CXX/basic/basic.link/p3.cpp +++ b/clang/test/CXX/basic/basic.link/p3.cpp @@ -14,7 +14,7 @@ constexpr int n = 123; export module m; // #1 module y = {}; // expected-error {{multiple module declarations}} -// expected-error@-1 {{unexpected preprocessing token '=' after module name, only ';' and '[' (start of attribute specifier sequence) are allowed}} +// expected-error@-1 {{unexpected '=' after module name, only ';' and '[' (start of attribute specifier sequence) are allowed}} // expected-note@#1 {{previous module declaration}} ::import x = {}; diff --git a/clang/test/CXX/drs/cwg2947.cpp b/clang/test/CXX/drs/cwg2947.cpp index dd66a2cfcfc4d..b932fca4a8175 100644 --- a/clang/test/CXX/drs/cwg2947.cpp +++ b/clang/test/CXX/drs/cwg2947.cpp @@ -37,7 +37,7 @@ //--- cwg2947_example1.cpp // #define DOT_BAR .bar export module foo DOT_BAR; // error: expansion of DOT_BAR; does not begin with ; or [ -// expected-error@-1 {{unexpected preprocessing token '.' after module name, only ';' and '[' (start of attribute specifier sequence) are allowed}} +// expected-error@-1 {{unexpected '.' after module name, only ';' and '[' (start of attribute specifier sequence) are allowed}} //--- cwg2947_example2.cpp export module M MOD_ATTR; // OK @@ -46,7 +46,7 @@ export module M MOD_ATTR; // OK //--- cwg2947_example3.cpp export module a .b; // error: preprocessing token after pp-module-name is not ; or [ -// expected-error@-1 {{unexpected preprocessing token '.' after module name, only ';' and '[' (start of attribute specifier sequence) are allowed}} +// expected-error@-1 {{unexpected '.' after module name, only ';' and '[' (start of attribute specifier sequence) are allowed}} //--- cwg2947_example4.cpp export module M [[ diff --git a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp index 52ba1d9f82f2f..b22fc87aa296c 100644 --- a/clang/test/CXX/module/basic/basic.link/module-declaration.cpp +++ b/clang/test/CXX/module/basic/basic.link/module-declaration.cpp @@ -47,7 +47,7 @@ export module x; //--- invalid_module_name.cppm export module z elderberry; -// expected-error@-1 {{unexpected preprocessing token 'elderberry' after module name, only ';' and '[' (start of attribute specifier sequence) are allowed}} +// expected-error@-1 {{unexpected 'elderberry' after module name, only ';' and '[' (start of attribute specifier sequence) are allowed}} //--- empty_attribute.cppm // expected-no-diagnostics diff --git a/clang/test/CXX/module/cpp.pre/p1.cpp b/clang/test/CXX/module/cpp.pre/p1.cpp index 989915004ff57..dc5a3c36d4139 100644 --- a/clang/test/CXX/module/cpp.pre/p1.cpp +++ b/clang/test/CXX/module/cpp.pre/p1.cpp @@ -153,7 +153,7 @@ export module m:n; //--- unexpected_character_in_pp_module_suffix.cpp export module m(); -// expected-error@-1 {{unexpected preprocessing token '(' after module name, only ';' and '[' (start of attribute specifier sequence) are allowed}} +// expected-error@-1 {{unexpected '(' after module name, only ';' and '[' (start of attribute specifier sequence) are allowed}} //--- semi_in_same_line.cpp export module m // OK @@ -189,13 +189,13 @@ module x; //--- func_like_macro.cpp // #define m(x) x export module m - (foo); // expected-error {{unexpected preprocessing token '(' after module name, only ';' and '[' (start of attribute specifier sequence) are allowed}} + (foo); // expected-error {{unexpected '(' after module name, only ';' and '[' (start of attribute specifier sequence) are allowed}} //--- lparen.cpp // #define m(x) x // #define LPAREN ( export module m - LPAREN foo); // expected-error {{unexpected preprocessing token 'LPAREN' after module name, only ';' and '[' (start of attribute specifier sequence) are allowed}} + LPAREN foo); // expected-error {{unexpected '(' after module name, only ';' and '[' (start of attribute specifier sequence) are allowed}} //--- control_line.cpp #if 0 // #1 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
