https://github.com/a-tarasyuk updated https://github.com/llvm/llvm-project/pull/143460
>From dd4953312066cb63ae1a3882270426c87b1f5b7a Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk <oleksandr.taras...@outlook.com> Date: Tue, 10 Jun 2025 02:47:51 +0300 Subject: [PATCH 1/5] [Clang] fix missing source location for ':' error in macro-expanded case statements --- clang/docs/ReleaseNotes.rst | 1 + clang/include/clang/Basic/DiagnosticCommonKinds.td | 1 + clang/lib/Parse/ParseStmt.cpp | 14 ++++++++++++++ clang/test/Parser/switch-recovery.cpp | 13 +++++++++++++ 4 files changed, 29 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 322686fce0b04..0ecbb4864050c 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -689,6 +689,7 @@ Bug Fixes in This Version - Fixed type mismatch error when 'builtin-elementwise-math' arguments have different qualifiers, this should be well-formed. (#GH141397) - Constant evaluation now correctly runs the destructor of a variable declared in the second clause of a C-style ``for`` loop. (#GH139818) +- Fixed incorrect diagnostic location for missing ``:`` in case statements expanded from macros. (#GH143216) Bug Fixes to Compiler Builtins ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td b/clang/include/clang/Basic/DiagnosticCommonKinds.td index 0bd8a423c393e..ee8fc66c1822c 100644 --- a/clang/include/clang/Basic/DiagnosticCommonKinds.td +++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td @@ -83,6 +83,7 @@ let CategoryName = "Parse Issue" in { def err_expected : Error<"expected %0">; def err_expected_either : Error<"expected %0 or %1">; def err_expected_after : Error<"expected %1 after %0">; +def note_macro_expansion : Note<"expanded from macro '%0'">; def err_param_redefinition : Error<"redefinition of parameter %0">; def warn_method_param_redefinition : Warning<"redefinition of method parameter %0">; diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp index c788723023c8b..5db6dd36f840b 100644 --- a/clang/lib/Parse/ParseStmt.cpp +++ b/clang/lib/Parse/ParseStmt.cpp @@ -833,9 +833,23 @@ StmtResult Parser::ParseCaseStatement(ParsedStmtContext StmtCtx, << FixItHint::CreateReplacement(ColonLoc, ":"); } else { SourceLocation ExpectedLoc = PP.getLocForEndOfToken(PrevTokLocation); + SourceLocation ExprLoc = + LHS.get() ? LHS.get()->getExprLoc() : SourceLocation(); + + if (ExpectedLoc.isInvalid() && ExprLoc.isMacroID()) { + ExpectedLoc = PP.getSourceManager().getSpellingLoc(ExprLoc); + } + Diag(ExpectedLoc, diag::err_expected_after) << "'case'" << tok::colon << FixItHint::CreateInsertion(ExpectedLoc, ":"); + + if (ExprLoc.isMacroID()) { + Diag(ExprLoc, diag::note_macro_expansion) + << Lexer::getImmediateMacroNameForDiagnostics( + ExprLoc, PP.getSourceManager(), getLangOpts()); + } + ColonLoc = ExpectedLoc; } diff --git a/clang/test/Parser/switch-recovery.cpp b/clang/test/Parser/switch-recovery.cpp index baf703cd03aed..5966b04b3f636 100644 --- a/clang/test/Parser/switch-recovery.cpp +++ b/clang/test/Parser/switch-recovery.cpp @@ -229,3 +229,16 @@ void fn1() { } } // expected-error{{expected statement}} } + +namespace GH143216 { +#define FOO 1 case 3: // expected-error {{expected ':' after 'case'}} + +int f(int x) { + switch (x) { + case FOO // expected-note {{expanded from macro 'FOO'}} + return 0; + default: + return 1; + } +} +} >From 971c095fe4cb84dcb5c1a2583a6570b9cc45dedd Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk <oleksandr.taras...@outlook.com> Date: Tue, 10 Jun 2025 16:18:29 +0300 Subject: [PATCH 2/5] remove duplicate note diagnostic --- .../clang/Basic/DiagnosticCommonKinds.td | 1 - clang/lib/Parse/ParseStmt.cpp | 17 +++++------------ clang/test/Parser/switch-recovery.cpp | 2 +- 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td b/clang/include/clang/Basic/DiagnosticCommonKinds.td index ee8fc66c1822c..0bd8a423c393e 100644 --- a/clang/include/clang/Basic/DiagnosticCommonKinds.td +++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td @@ -83,7 +83,6 @@ let CategoryName = "Parse Issue" in { def err_expected : Error<"expected %0">; def err_expected_either : Error<"expected %0 or %1">; def err_expected_after : Error<"expected %1 after %0">; -def note_macro_expansion : Note<"expanded from macro '%0'">; def err_param_redefinition : Error<"redefinition of parameter %0">; def warn_method_param_redefinition : Warning<"redefinition of method parameter %0">; diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp index 5db6dd36f840b..87472fd6f2a4f 100644 --- a/clang/lib/Parse/ParseStmt.cpp +++ b/clang/lib/Parse/ParseStmt.cpp @@ -833,22 +833,15 @@ StmtResult Parser::ParseCaseStatement(ParsedStmtContext StmtCtx, << FixItHint::CreateReplacement(ColonLoc, ":"); } else { SourceLocation ExpectedLoc = PP.getLocForEndOfToken(PrevTokLocation); - SourceLocation ExprLoc = - LHS.get() ? LHS.get()->getExprLoc() : SourceLocation(); - - if (ExpectedLoc.isInvalid() && ExprLoc.isMacroID()) { - ExpectedLoc = PP.getSourceManager().getSpellingLoc(ExprLoc); + if (ExpectedLoc.isInvalid() && LHS.get()) { + ExpectedLoc = + PP.getSourceManager().getSpellingLoc(LHS.get()->getExprLoc()); } Diag(ExpectedLoc, diag::err_expected_after) << "'case'" << tok::colon - << FixItHint::CreateInsertion(ExpectedLoc, ":"); - - if (ExprLoc.isMacroID()) { - Diag(ExprLoc, diag::note_macro_expansion) - << Lexer::getImmediateMacroNameForDiagnostics( - ExprLoc, PP.getSourceManager(), getLangOpts()); - } + << FixItHint::CreateInsertion(ExpectedLoc, + tok::getTokenName(tok::colon)); ColonLoc = ExpectedLoc; } diff --git a/clang/test/Parser/switch-recovery.cpp b/clang/test/Parser/switch-recovery.cpp index 5966b04b3f636..a9c9d8fddc009 100644 --- a/clang/test/Parser/switch-recovery.cpp +++ b/clang/test/Parser/switch-recovery.cpp @@ -235,7 +235,7 @@ namespace GH143216 { int f(int x) { switch (x) { - case FOO // expected-note {{expanded from macro 'FOO'}} + case FOO return 0; default: return 1; >From 31d3b2d096a0b9af484ab6e36f7ab6cd46e98c39 Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk <oleksandr.taras...@outlook.com> Date: Wed, 11 Jun 2025 03:39:51 +0300 Subject: [PATCH 3/5] use spelling location as fallback when end-of-token location is invalid --- clang/include/clang/Parse/Parser.h | 4 +--- clang/lib/Parse/ParseExprCXX.cpp | 4 ++-- clang/lib/Parse/ParseStmt.cpp | 6 +----- clang/lib/Parse/Parser.cpp | 11 ++++++++++ .../test/Parser/macro-expansion-recovery.cpp | 20 +++++++++++++++++++ 5 files changed, 35 insertions(+), 10 deletions(-) create mode 100644 clang/test/Parser/macro-expansion-recovery.cpp diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index 0b2fab4a45c96..d99de77a52919 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -290,9 +290,7 @@ class Parser : public CodeCompletionHandler { return ConsumeToken(); } - SourceLocation getEndOfPreviousToken() { - return PP.getLocForEndOfToken(PrevTokLocation); - } + SourceLocation getEndOfPreviousToken() const; /// GetLookAheadToken - This peeks ahead N tokens and returns that token /// without consuming any tokens. LookAhead(0) returns 'Tok', LookAhead(1) diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp index d95260829e4a0..55ad7f256fa82 100644 --- a/clang/lib/Parse/ParseExprCXX.cpp +++ b/clang/lib/Parse/ParseExprCXX.cpp @@ -421,8 +421,8 @@ bool Parser::ParseOptionalCXXScopeSpecifier( // like we never saw it. Token Identifier = Tok; // Stash away the identifier. ConsumeToken(); // Eat the identifier, current token is now '::'. - Diag(PP.getLocForEndOfToken(ConsumeToken()), diag::err_expected) - << tok::identifier; + ConsumeToken(); + Diag(getEndOfPreviousToken(), diag::err_expected) << tok::identifier; UnconsumeToken(Identifier); // Stick the identifier back. Next = NextToken(); // Point Next at the '{' token. } diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp index 87472fd6f2a4f..c00759893b0c4 100644 --- a/clang/lib/Parse/ParseStmt.cpp +++ b/clang/lib/Parse/ParseStmt.cpp @@ -832,11 +832,7 @@ StmtResult Parser::ParseCaseStatement(ParsedStmtContext StmtCtx, << "'case'" << tok::colon << FixItHint::CreateReplacement(ColonLoc, ":"); } else { - SourceLocation ExpectedLoc = PP.getLocForEndOfToken(PrevTokLocation); - if (ExpectedLoc.isInvalid() && LHS.get()) { - ExpectedLoc = - PP.getSourceManager().getSpellingLoc(LHS.get()->getExprLoc()); - } + SourceLocation ExpectedLoc = getEndOfPreviousToken(); Diag(ExpectedLoc, diag::err_expected_after) << "'case'" << tok::colon diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp index db65c05cc114a..d32262cfd2734 100644 --- a/clang/lib/Parse/Parser.cpp +++ b/clang/lib/Parse/Parser.cpp @@ -1873,6 +1873,17 @@ Parser::TryAnnotateName(CorrectionCandidateCallback *CCC, return AnnotatedNameKind::Unresolved; } +SourceLocation Parser::getEndOfPreviousToken() const { + SourceLocation TokenEndLoc = PP.getLocForEndOfToken(PrevTokLocation); + if (TokenEndLoc.isValid()) + return TokenEndLoc; + + if (Tok.getLocation().isMacroID()) + return PP.getSourceManager().getSpellingLoc(Tok.getLocation()); + + return Tok.getLocation(); +} + bool Parser::TryKeywordIdentFallback(bool DisableKeyword) { assert(Tok.isNot(tok::identifier)); Diag(Tok, diag::ext_keyword_as_ident) diff --git a/clang/test/Parser/macro-expansion-recovery.cpp b/clang/test/Parser/macro-expansion-recovery.cpp new file mode 100644 index 0000000000000..c66e1963ca256 --- /dev/null +++ b/clang/test/Parser/macro-expansion-recovery.cpp @@ -0,0 +1,20 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +namespace GH143216 { +#define A x y // expected-error {{missing ',' between enumerators}} +enum { A }; + +#define B x y // expected-error {{expected ','}} +void f() { + int a[2]; + auto [B] = a; +} + +#define C <int! // expected-error {{expected '>'}} +template <class T> class D; +D C; // expected-error {{expected unqualified-id}} \ + // expected-note {{to match this '<'}} + +#define E F::{ // expected-error {{expected identifier}} +class F { E }}; // expected-error {{expected member name or ';' after declaration specifiers}} +} >From 5efe64efd71a314f057aae052703fbf6d19a2c4e Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk <oleksandr.taras...@outlook.com> Date: Wed, 11 Jun 2025 03:41:13 +0300 Subject: [PATCH 4/5] update release notes --- clang/docs/ReleaseNotes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index b7bca45a7201f..e482b7e63a15e 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -695,7 +695,7 @@ Bug Fixes in This Version - Fixed type mismatch error when 'builtin-elementwise-math' arguments have different qualifiers, this should be well-formed. (#GH141397) - Constant evaluation now correctly runs the destructor of a variable declared in the second clause of a C-style ``for`` loop. (#GH139818) -- Fixed incorrect diagnostic location for missing ``:`` in case statements expanded from macros. (#GH143216) +- Fixed incorrect token location when emitting diagnostics for tokens expanded from macros. (#GH143216) Bug Fixes to Compiler Builtins ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >From 437bd8875f97573e46d7e148b769b433cbdae67a Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk <oleksandr.taras...@outlook.com> Date: Wed, 11 Jun 2025 22:24:36 +0300 Subject: [PATCH 5/5] remove fallback to spelling location for macro tokens --- clang/lib/Parse/Parser.cpp | 8 +------- clang/test/Parser/macro-expansion-recovery.cpp | 16 +++++++++------- clang/test/Parser/switch-recovery.cpp | 4 ++-- 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp index d32262cfd2734..788ed79e0c1fa 100644 --- a/clang/lib/Parse/Parser.cpp +++ b/clang/lib/Parse/Parser.cpp @@ -1875,13 +1875,7 @@ Parser::TryAnnotateName(CorrectionCandidateCallback *CCC, SourceLocation Parser::getEndOfPreviousToken() const { SourceLocation TokenEndLoc = PP.getLocForEndOfToken(PrevTokLocation); - if (TokenEndLoc.isValid()) - return TokenEndLoc; - - if (Tok.getLocation().isMacroID()) - return PP.getSourceManager().getSpellingLoc(Tok.getLocation()); - - return Tok.getLocation(); + return TokenEndLoc.isValid() ? TokenEndLoc : Tok.getLocation(); } bool Parser::TryKeywordIdentFallback(bool DisableKeyword) { diff --git a/clang/test/Parser/macro-expansion-recovery.cpp b/clang/test/Parser/macro-expansion-recovery.cpp index c66e1963ca256..6826cc04e4df5 100644 --- a/clang/test/Parser/macro-expansion-recovery.cpp +++ b/clang/test/Parser/macro-expansion-recovery.cpp @@ -1,20 +1,22 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s namespace GH143216 { -#define A x y // expected-error {{missing ',' between enumerators}} -enum { A }; +#define A x y +enum { A }; // expected-error {{missing ',' between enumerators}} -#define B x y // expected-error {{expected ','}} +#define B x y void f() { int a[2]; - auto [B] = a; + auto [B] = a; // expected-error {{expected ','}} } -#define C <int! // expected-error {{expected '>'}} +#define C <int! template <class T> class D; D C; // expected-error {{expected unqualified-id}} \ + // expected-error {{expected '>'}} \ // expected-note {{to match this '<'}} -#define E F::{ // expected-error {{expected identifier}} -class F { E }}; // expected-error {{expected member name or ';' after declaration specifiers}} +#define E F::{ +class F { E }}; // expected-error {{expected identifier}} \ + // expected-error {{expected member name or ';' after declaration specifiers}} } diff --git a/clang/test/Parser/switch-recovery.cpp b/clang/test/Parser/switch-recovery.cpp index a9c9d8fddc009..7b3909e3b0d32 100644 --- a/clang/test/Parser/switch-recovery.cpp +++ b/clang/test/Parser/switch-recovery.cpp @@ -231,11 +231,11 @@ void fn1() { } namespace GH143216 { -#define FOO 1 case 3: // expected-error {{expected ':' after 'case'}} +#define FOO 1 case 3: int f(int x) { switch (x) { - case FOO + case FOO // expected-error {{expected ':' after 'case'}} return 0; default: return 1; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits