Skip to content

[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

Merged
merged 1 commit into from
Apr 18, 2025
Merged

Conversation

zyn0217
Copy link
Contributor

@zyn0217 zyn0217 commented Apr 18, 2025

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.

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.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 18, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 18, 2025

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

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.


Full diff: https://github.com/llvm/llvm-project/pull/136273.diff

2 Files Affected:

  • (modified) clang/lib/Parse/ParseExpr.cpp (-3)
  • (modified) clang/test/Parser/recovery.cpp (+18)
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}}
+}
+
+}

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@zyn0217 zyn0217 merged commit c7daab2 into llvm:main Apr 18, 2025
14 checks passed
@zyn0217 zyn0217 deleted the fix-trailing-comma branch April 18, 2025 08:27
@zyn0217 zyn0217 added this to the LLVM 20.X Release milestone Apr 18, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Apr 18, 2025
@zyn0217
Copy link
Contributor Author

zyn0217 commented Apr 18, 2025

/cherry-pick c7daab2

@llvmbot
Copy link
Member

llvmbot commented Apr 18, 2025

/pull-request #136283

@llvmbot llvmbot moved this from Needs Triage to Done in LLVM Release Status Apr 18, 2025
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
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.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
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.
swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request May 9, 2025
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

[Regression] Clang accepts a syntactically invalid use of uniform_int_distribution<>(1, );
3 participants