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 <[email protected]> Date: Sun, 2 Nov 2025 10:37:32 +0800 Subject: [PATCH 1/3] 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 <[email protected]> --- 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 <[email protected]> Date: Thu, 6 Nov 2025 18:37:45 +0800 Subject: [PATCH 2/3] 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@-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@-10 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}} +} + namespace GH48405 { void foo() { struct S { >From 0e185ed92074b038a970b2845941f7f36ab46ce9 Mon Sep 17 00:00:00 2001 From: Kai Kai-Yi Weng <[email protected]> Date: Fri, 7 Nov 2025 10:10:32 +0800 Subject: [PATCH 3/3] Add a note to release note --- clang/docs/ReleaseNotes.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 92fc9381a5868..8993336c6f657 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -69,6 +69,16 @@ Potentially Breaking Changes call the member ``operator delete`` instead of the expected global delete operator. The old behavior is retained under ``-fclang-abi-compat=21`` flag. +- Trailing null statements in GNU statement expressions are no longer + ignored by Clang; they now result in a void type. Clang previously + matched GCC's behavior, which was recently clarified to be incorrect. + + .. code-block:: c++ + + // The resulting type is 'void', not 'int' + void foo(void) { + return ({ 1;; }); + } C/C++ Language Potentially Breaking Changes ------------------------------------------- _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
