[clang] Revert "Ignore trailing NullStmts in StmtExprs for GCC compatibility." (PR #166036)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 6 02:48:07 PST 2025
https://github.com/KaiYG updated https://github.com/llvm/llvm-project/pull/166036
>From 91c2333455265c6596411d033bb849b4b5254f01 Mon Sep 17 00:00:00 2001
From: Kai Kai-Yi Weng <kaiweng at andestech.com>
Date: Sun, 2 Nov 2025 10:37:32 +0800
Subject: [PATCH 1/2] Revert "Ignore trailing NullStmts in StmtExprs for GCC
compatibility."
This reverts commit b1e511bf5a4c702ace445848b30070ac2e021241.
https://github.com/llvm/llvm-project/issues/160243
Reverting because the GCC C front end is incorrect.
Co-authored-by: Jim Lin <jim at andestech.com>
---
clang/include/clang/AST/Stmt.h | 20 ---------
clang/lib/AST/ByteCode/Compiler.cpp | 2 +-
clang/lib/AST/ComputeDependence.cpp | 2 +-
clang/lib/CodeGen/CGStmt.cpp | 69 ++++++++++++++---------------
clang/lib/Parse/ParseStmt.cpp | 14 ++----
clang/lib/Sema/SemaExpr.cpp | 4 +-
clang/lib/Sema/TreeTransform.h | 5 +--
clang/test/AST/ast-dump-stmt.c | 10 -----
clang/test/CodeGen/exprs.c | 10 -----
clang/test/Sema/statements.c | 18 --------
clang/test/SemaCXX/statements.cpp | 15 -------
11 files changed, 42 insertions(+), 127 deletions(-)
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index 76942f1a84f9a..bec4066cc16eb 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -1831,26 +1831,6 @@ class CompoundStmt final
return const_reverse_body_iterator(body_begin());
}
- // Get the Stmt that StmtExpr would consider to be the result of this
- // compound statement. This is used by StmtExpr to properly emulate the GCC
- // compound expression extension, which ignores trailing NullStmts when
- // getting the result of the expression.
- // i.e. ({ 5;;; })
- // ^^ ignored
- // If we don't find something that isn't a NullStmt, just return the last
- // Stmt.
- Stmt *getStmtExprResult() {
- for (auto *B : llvm::reverse(body())) {
- if (!isa<NullStmt>(B))
- return B;
- }
- return body_back();
- }
-
- const Stmt *getStmtExprResult() const {
- return const_cast<CompoundStmt *>(this)->getStmtExprResult();
- }
-
SourceLocation getBeginLoc() const { return LBraceLoc; }
SourceLocation getEndLoc() const { return RBraceLoc; }
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 6c088469a3ca2..2fe5cb0de66d6 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -4157,7 +4157,7 @@ bool Compiler<Emitter>::VisitStmtExpr(const StmtExpr *E) {
StmtExprScope<Emitter> SS(this);
const CompoundStmt *CS = E->getSubStmt();
- const Stmt *Result = CS->getStmtExprResult();
+ const Stmt *Result = CS->body_back();
for (const Stmt *S : CS->body()) {
if (S != Result) {
if (!this->visitStmt(S))
diff --git a/clang/lib/AST/ComputeDependence.cpp b/clang/lib/AST/ComputeDependence.cpp
index e0cf0deb12bd2..638080ea781a9 100644
--- a/clang/lib/AST/ComputeDependence.cpp
+++ b/clang/lib/AST/ComputeDependence.cpp
@@ -178,7 +178,7 @@ ExprDependence clang::computeDependence(StmtExpr *E, unsigned TemplateDepth) {
auto D = toExprDependenceForImpliedType(E->getType()->getDependence());
// Propagate dependence of the result.
if (const auto *CompoundExprResult =
- dyn_cast_or_null<ValueStmt>(E->getSubStmt()->getStmtExprResult()))
+ dyn_cast_or_null<ValueStmt>(E->getSubStmt()->body_back()))
if (const Expr *ResultExpr = CompoundExprResult->getExprStmt())
D |= ResultExpr->getDependence();
// Note: we treat a statement-expression in a dependent context as always
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index fdc1a11f6c55c..36be3295950b8 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -582,48 +582,45 @@ CodeGenFunction::EmitCompoundStmtWithoutScope(const CompoundStmt &S,
bool GetLast,
AggValueSlot AggSlot) {
- const Stmt *ExprResult = S.getStmtExprResult();
- assert((!GetLast || (GetLast && ExprResult)) &&
- "If GetLast is true then the CompoundStmt must have a StmtExprResult");
+ for (CompoundStmt::const_body_iterator I = S.body_begin(),
+ E = S.body_end() - GetLast;
+ I != E; ++I)
+ EmitStmt(*I);
Address RetAlloca = Address::invalid();
-
- for (auto *CurStmt : S.body()) {
- if (GetLast && ExprResult == CurStmt) {
- // We have to special case labels here. They are statements, but when put
- // at the end of a statement expression, they yield the value of their
- // subexpression. Handle this by walking through all labels we encounter,
- // emitting them before we evaluate the subexpr.
- // Similar issues arise for attributed statements.
- while (!isa<Expr>(ExprResult)) {
- if (const auto *LS = dyn_cast<LabelStmt>(ExprResult)) {
- EmitLabel(LS->getDecl());
- ExprResult = LS->getSubStmt();
- } else if (const auto *AS = dyn_cast<AttributedStmt>(ExprResult)) {
- // FIXME: Update this if we ever have attributes that affect the
- // semantics of an expression.
- ExprResult = AS->getSubStmt();
- } else {
- llvm_unreachable("unknown value statement");
- }
+ if (GetLast) {
+ // We have to special case labels here. They are statements, but when put
+ // at the end of a statement expression, they yield the value of their
+ // subexpression. Handle this by walking through all labels we encounter,
+ // emitting them before we evaluate the subexpr.
+ // Similar issues arise for attributed statements.
+ const Stmt *LastStmt = S.body_back();
+ while (!isa<Expr>(LastStmt)) {
+ if (const auto *LS = dyn_cast<LabelStmt>(LastStmt)) {
+ EmitLabel(LS->getDecl());
+ LastStmt = LS->getSubStmt();
+ } else if (const auto *AS = dyn_cast<AttributedStmt>(LastStmt)) {
+ // FIXME: Update this if we ever have attributes that affect the
+ // semantics of an expression.
+ LastStmt = AS->getSubStmt();
+ } else {
+ llvm_unreachable("unknown value statement");
}
+ }
- EnsureInsertPoint();
+ EnsureInsertPoint();
- const Expr *E = cast<Expr>(ExprResult);
- QualType ExprTy = E->getType();
- if (hasAggregateEvaluationKind(ExprTy)) {
- EmitAggExpr(E, AggSlot);
- } else {
- // We can't return an RValue here because there might be cleanups at
- // the end of the StmtExpr. Because of that, we have to emit the result
- // here into a temporary alloca.
- RetAlloca = CreateMemTemp(ExprTy);
- EmitAnyExprToMem(E, RetAlloca, Qualifiers(),
- /*IsInit*/ false);
- }
+ const Expr *E = cast<Expr>(LastStmt);
+ QualType ExprTy = E->getType();
+ if (hasAggregateEvaluationKind(ExprTy)) {
+ EmitAggExpr(E, AggSlot);
} else {
- EmitStmt(CurStmt);
+ // We can't return an RValue here because there might be cleanups at
+ // the end of the StmtExpr. Because of that, we have to emit the result
+ // here into a temporary alloca.
+ RetAlloca = CreateMemTemp(ExprTy);
+ EmitAnyExprToMem(E, RetAlloca, Qualifiers(),
+ /*IsInit*/ false);
}
}
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 92038985f9163..85c5de8533eb6 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -1079,16 +1079,10 @@ bool Parser::ConsumeNullStmt(StmtVector &Stmts) {
StmtResult Parser::handleExprStmt(ExprResult E, ParsedStmtContext StmtCtx) {
bool IsStmtExprResult = false;
if ((StmtCtx & ParsedStmtContext::InStmtExpr) != ParsedStmtContext()) {
- // For GCC compatibility we skip past NullStmts.
- unsigned LookAhead = 0;
- while (GetLookAheadToken(LookAhead).is(tok::semi)) {
- ++LookAhead;
- }
- // Then look to see if the next two tokens close the statement expression;
- // if so, this expression statement is the last statement in a statement
- // expression.
- IsStmtExprResult = GetLookAheadToken(LookAhead).is(tok::r_brace) &&
- GetLookAheadToken(LookAhead + 1).is(tok::r_paren);
+ // Look ahead to see if the next two tokens close the statement expression;
+ // if so, this expression statement is the last statement in a
+ // statment expression.
+ IsStmtExprResult = Tok.is(tok::r_brace) && NextToken().is(tok::r_paren);
}
if (IsStmtExprResult)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index a50c27610dc96..1fc7ef354a1ba 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -16185,9 +16185,7 @@ ExprResult Sema::BuildStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
QualType Ty = Context.VoidTy;
bool StmtExprMayBindToTemp = false;
if (!Compound->body_empty()) {
- // For GCC compatibility we get the last Stmt excluding trailing NullStmts.
- if (const auto *LastStmt =
- dyn_cast<ValueStmt>(Compound->getStmtExprResult())) {
+ if (const auto *LastStmt = dyn_cast<ValueStmt>(Compound->body_back())) {
if (const Expr *Value = LastStmt->getExprStmt()) {
StmtExprMayBindToTemp = true;
Ty = Value->getType();
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 8c20078e97a13..ee35ef83c8387 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -8080,14 +8080,13 @@ TreeTransform<Derived>::TransformCompoundStmt(CompoundStmt *S,
getSema().resetFPOptions(
S->getStoredFPFeatures().applyOverrides(getSema().getLangOpts()));
- const Stmt *ExprResult = S->getStmtExprResult();
bool SubStmtInvalid = false;
bool SubStmtChanged = false;
SmallVector<Stmt*, 8> Statements;
for (auto *B : S->body()) {
StmtResult Result = getDerived().TransformStmt(
- B, IsStmtExpr && B == ExprResult ? StmtDiscardKind::StmtExprResult
- : StmtDiscardKind::Discarded);
+ B, IsStmtExpr && B == S->body_back() ? StmtDiscardKind::StmtExprResult
+ : StmtDiscardKind::Discarded);
if (Result.isInvalid()) {
// Immediately fail if this was a DeclStmt, since it's very
diff --git a/clang/test/AST/ast-dump-stmt.c b/clang/test/AST/ast-dump-stmt.c
index 5c44fea2df6e7..82f5d8efecfe6 100644
--- a/clang/test/AST/ast-dump-stmt.c
+++ b/clang/test/AST/ast-dump-stmt.c
@@ -399,14 +399,4 @@ void TestMiscStmts(void) {
// CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}} <col:13> 'int' 10
// CHECK-NEXT: ImplicitCastExpr
// CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}} <col:17> 'int' lvalue Var 0x{{[^ ]*}} 'a' 'int'
- ({int a = 10; a;;; });
- // CHECK-NEXT: StmtExpr 0x{{[^ ]*}} <line:[[@LINE-1]]:3, col:23> 'int'
- // CHECK-NEXT: CompoundStmt
- // CHECK-NEXT: DeclStmt
- // CHECK-NEXT: VarDecl 0x{{[^ ]*}} <col:5, col:13> col:9 used a 'int' cinit
- // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}} <col:13> 'int' 10
- // CHECK-NEXT: ImplicitCastExpr
- // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}} <col:17> 'int' lvalue Var 0x{{[^ ]*}} 'a' 'int'
- // CHECK-NEXT: NullStmt
- // CHECK-NEXT: NullStmt
}
diff --git a/clang/test/CodeGen/exprs.c b/clang/test/CodeGen/exprs.c
index 5cca9722dcb3a..4f3c7d50e83bd 100644
--- a/clang/test/CodeGen/exprs.c
+++ b/clang/test/CodeGen/exprs.c
@@ -193,13 +193,3 @@ void f18(void) {
}
// CHECK-LABEL: define{{.*}} void @f18()
// CHECK: call i32 @returns_int()
-
-// Ensure the right stmt is returned
-int f19(void) {
- return ({ 3;;4;; });
-}
-// CHECK-LABEL: define{{.*}} i32 @f19()
-// CHECK: [[T:%.*]] = alloca i32
-// CHECK: store i32 4, ptr [[T]]
-// CHECK: [[L:%.*]] = load i32, ptr [[T]]
-// CHECK: ret i32 [[L]]
diff --git a/clang/test/Sema/statements.c b/clang/test/Sema/statements.c
index d44ab5a65d5af..55b5f2c13a0b8 100644
--- a/clang/test/Sema/statements.c
+++ b/clang/test/Sema/statements.c
@@ -118,21 +118,3 @@ void test_pr22849(void) {
SIZE = sizeof(({unsigned long __ptr; __ptr;}))
};
}
-
-// GCC ignores empty statements at the end of compound expressions where the
-// result type is concerned.
-void test13(void) {
- int a;
- a = ({ 1; });
- a = ({1;; });
- a = ({int x = 1; (void)x; }); // expected-error {{assigning to 'int' from incompatible type 'void'}}
- a = ({int x = 1; (void)x;; }); // expected-error {{assigning to 'int' from incompatible type 'void'}}
-}
-
-void test14(void) { return ({}); }
-void test15(void) {
- return ({;;;; });
-}
-void test16(void) {
- return ({test:;; });
-}
diff --git a/clang/test/SemaCXX/statements.cpp b/clang/test/SemaCXX/statements.cpp
index 48f178dd9a8b3..9f40dd2794709 100644
--- a/clang/test/SemaCXX/statements.cpp
+++ b/clang/test/SemaCXX/statements.cpp
@@ -38,21 +38,6 @@ void test5() {
struct MMX_t {};
void test6() { __asm__("" : "=m"(*(MMX_t *)0)); }
-template <typename T>
-T test7(T v) {
- return ({ // expected-warning{{use of GNU statement expression extension}}
- T a = v;
- a;
- ;
- ;
- });
-}
-
-void test8() {
- int a = test7(1);
- double b = test7(2.0);
-}
-
namespace GH48405 {
void foo() {
struct S {
>From 3b01f313be3ba81ef7a569007a8f3dca2c79e4e5 Mon Sep 17 00:00:00 2001
From: Kai Kai-Yi Weng <kaiweng at andestech.com>
Date: Thu, 6 Nov 2025 18:37:45 +0800
Subject: [PATCH 2/2] Add back testcases with updated checks
---
clang/test/AST/ast-dump-stmt.c | 10 ++++++++++
clang/test/CodeGen/exprs.c | 17 +++++++++++++++++
clang/test/Sema/statements.c | 19 +++++++++++++++++++
clang/test/SemaCXX/statements.cpp | 28 ++++++++++++++++++++++++++++
4 files changed, 74 insertions(+)
diff --git a/clang/test/AST/ast-dump-stmt.c b/clang/test/AST/ast-dump-stmt.c
index 82f5d8efecfe6..6fb01a4b159fa 100644
--- a/clang/test/AST/ast-dump-stmt.c
+++ b/clang/test/AST/ast-dump-stmt.c
@@ -399,4 +399,14 @@ void TestMiscStmts(void) {
// CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}} <col:13> 'int' 10
// CHECK-NEXT: ImplicitCastExpr
// CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}} <col:17> 'int' lvalue Var 0x{{[^ ]*}} 'a' 'int'
+ ({int a = 10; a;;; });
+ // CHECK-NEXT: StmtExpr 0x{{[^ ]*}} <line:[[@LINE-1]]:3, col:23> 'void'
+ // CHECK-NEXT: CompoundStmt
+ // CHECK-NEXT: DeclStmt
+ // CHECK-NEXT: VarDecl 0x{{[^ ]*}} <col:5, col:13> col:9 used a 'int' cinit
+ // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}} <col:13> 'int' 10
+ // CHECK-NEXT: ImplicitCastExpr
+ // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}} <col:17> 'int' lvalue Var 0x{{[^ ]*}} 'a' 'int'
+ // CHECK-NEXT: NullStmt
+ // CHECK-NEXT: NullStmt
}
diff --git a/clang/test/CodeGen/exprs.c b/clang/test/CodeGen/exprs.c
index 4f3c7d50e83bd..93015da074bf2 100644
--- a/clang/test/CodeGen/exprs.c
+++ b/clang/test/CodeGen/exprs.c
@@ -193,3 +193,20 @@ void f18(void) {
}
// CHECK-LABEL: define{{.*}} void @f18()
// CHECK: call i32 @returns_int()
+
+// Ensure the right stmt is returned
+int f19(void) {
+ return ({ 3;;4; });
+}
+// CHECK-LABEL: define{{.*}} i32 @f19()
+// CHECK: [[T:%.*]] = alloca i32
+// CHECK: store i32 4, ptr [[T]]
+// CHECK: [[L:%.*]] = load i32, ptr [[T]]
+// CHECK: ret i32 [[L]]
+
+// PR166036: The trailing NullStmt should result in a void.
+void f20(void) {
+ return ({ 3;;4;; });
+}
+// CHECK-LABEL: define{{.*}} void @f20()
+// CHECK: ret void
diff --git a/clang/test/Sema/statements.c b/clang/test/Sema/statements.c
index 55b5f2c13a0b8..28740fa295768 100644
--- a/clang/test/Sema/statements.c
+++ b/clang/test/Sema/statements.c
@@ -118,3 +118,22 @@ void test_pr22849(void) {
SIZE = sizeof(({unsigned long __ptr; __ptr;}))
};
}
+
+// Empty statements at the end of compound expressions have a result type 'void'.
+void test13(void) {
+ int a;
+ a = ({ 1; });
+ a = ({ 1; 2; }); // expected-warning {{expression result unused}}
+ a = ({ 1;; }); // expected-error {{assigning to 'int' from incompatible type 'void'}}
+ // expected-warning at -1 {{expression result unused}}
+ a = ({int x = 1; (void)x; }); // expected-error {{assigning to 'int' from incompatible type 'void'}}
+ a = ({int x = 1;; }); // expected-error {{assigning to 'int' from incompatible type 'void'}}
+}
+
+void test14(void) { return ({}); }
+void test15(void) {
+ return ({;;;; });
+}
+void test16(void) {
+ return ({test:;; });
+}
diff --git a/clang/test/SemaCXX/statements.cpp b/clang/test/SemaCXX/statements.cpp
index 9f40dd2794709..426e9fa1e585b 100644
--- a/clang/test/SemaCXX/statements.cpp
+++ b/clang/test/SemaCXX/statements.cpp
@@ -38,6 +38,34 @@ void test5() {
struct MMX_t {};
void test6() { __asm__("" : "=m"(*(MMX_t *)0)); }
+template <typename T>
+T test7(T v) {
+ return ({ // expected-warning{{use of GNU statement expression extension}}
+ T a = v;
+ a;
+ });
+}
+
+void test8() {
+ int a = test7(1);
+ double b = test7(2.0);
+}
+
+template <typename T>
+T test9(T v) {
+ return ({ // expected-warning {{use of GNU statement expression extension}}
+ T a = v;
+ a; // expected-warning {{expression result unused}}
+ ;
+ ;
+ });
+}
+
+void test10() {
+ int a = test9(1); // expected-note {{in instantiation of function template specialization 'test9<int>' requested here}}
+ // expected-error at -10 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
+}
+
namespace GH48405 {
void foo() {
struct S {
More information about the cfe-commits
mailing list