[clang] [Clang] fix missing source location for errors in macro-expanded (PR #143460)
Oleksandr T. via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 11 12:25:29 PDT 2025
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.tarasiuk at 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.tarasiuk at 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.tarasiuk at 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.tarasiuk at 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.tarasiuk at 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;
More information about the cfe-commits
mailing list