-
Notifications
You must be signed in to change notification settings - Fork 13.4k
[Clang] Fix the trailing comma regression #136273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
925e195 introduced a regression since which we started to accept invalid trailing commas in many expression lists where they're not allowed by the grammar. The issue came from the fact that an additional invalid state - previously handled by ParseExpressionList - was overlooked in that patch.
@llvm/pr-subscribers-clang Author: Younan Zhang (zyn0217) Changes925e195 introduced a regression since which we started to accept invalid trailing commas in many expression lists where they're not allowed by the grammar. The issue came from the fact that an additional invalid state - previously handled by ParseExpressionList - was overlooked in that patch. Fixes #136254 No release entry because I want to backport it. Full diff: https://github.com/llvm/llvm-project/pull/136273.diff 2 Files Affected:
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 0a22f7372a9f9..2282fbe04839e 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -2239,8 +2239,6 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
if (PP.isCodeCompletionReached() && !CalledSignatureHelp)
RunSignatureHelp();
LHS = ExprError();
- } else if (!HasError && HasTrailingComma) {
- Diag(Tok, diag::err_expected_expression);
} else if (LHS.isInvalid()) {
for (auto &E : ArgExprs)
Actions.CorrectDelayedTyposInExpr(E);
@@ -3750,7 +3748,6 @@ bool Parser::ParseExpressionList(SmallVectorImpl<Expr *> &Exprs,
if (Tok.is(tok::r_paren)) {
if (HasTrailingComma)
*HasTrailingComma = true;
- break;
}
}
if (SawError) {
diff --git a/clang/test/Parser/recovery.cpp b/clang/test/Parser/recovery.cpp
index 2fce67a52c6b6..261f5dc99bad4 100644
--- a/clang/test/Parser/recovery.cpp
+++ b/clang/test/Parser/recovery.cpp
@@ -222,3 +222,21 @@ void k() {
func(1, ); // expected-error {{expected expression}}
}
}
+
+namespace GH136254 {
+
+void call() {
+ [a(42, )]() {} (); // expected-error {{expected expression}}
+
+ int *b = new int(42, ); // expected-error {{expected expression}}
+
+ struct S {
+ int c;
+
+ S() : c(42, ) {} // expected-error {{expected expression}}
+ };
+
+ int d(42, ); // expected-error {{expected expression}}
+}
+
+}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
/cherry-pick c7daab2 |
/pull-request #136283 |
925e195 introduced a regression since which we started to accept invalid trailing commas in many expression lists where they're not allowed by the grammar. The issue came from the fact that an additional invalid state - previously handled by ParseExpressionList - was overlooked in that patch. Fixes llvm#136254 No release entry because I want to backport it.
925e195 introduced a regression since which we started to accept invalid trailing commas in many expression lists where they're not allowed by the grammar. The issue came from the fact that an additional invalid state - previously handled by ParseExpressionList - was overlooked in that patch. Fixes llvm#136254 No release entry because I want to backport it.
925e195 introduced a regression since which we started to accept invalid trailing commas in many expression lists where they're not allowed by the grammar. The issue came from the fact that an additional invalid state - previously handled by ParseExpressionList - was overlooked in that patch. Fixes llvm#136254 No release entry because I want to backport it. (cherry picked from commit c7daab2)
925e195 introduced a regression since which we started to accept invalid trailing commas in many expression lists where they're not allowed by the grammar. The issue came from the fact that an additional invalid state - previously handled by ParseExpressionList - was overlooked in that patch.
Fixes #136254
No release entry because I want to backport it.