Author: KaiWeng Date: 2025-11-07T09:30:53-05:00 New Revision: d9c7c762695e8b73d71dbc8dbedcc033030e25eb
URL: https://github.com/llvm/llvm-project/commit/d9c7c762695e8b73d71dbc8dbedcc033030e25eb DIFF: https://github.com/llvm/llvm-project/commit/d9c7c762695e8b73d71dbc8dbedcc033030e25eb.diff LOG: Revert "Ignore trailing NullStmts in StmtExprs for GCC compatibility." (#166036) 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]> Added: Modified: clang/docs/ReleaseNotes.rst clang/include/clang/AST/Stmt.h clang/lib/AST/ByteCode/Compiler.cpp clang/lib/AST/ComputeDependence.cpp clang/lib/CodeGen/CGStmt.cpp clang/lib/Parse/ParseStmt.cpp clang/lib/Sema/SemaExpr.cpp clang/lib/Sema/TreeTransform.h clang/test/AST/ast-dump-stmt.c clang/test/CodeGen/exprs.c clang/test/Sema/statements.c clang/test/SemaCXX/statements.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index e8339fa13ffba..a59f6bd50fc9e 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 ------------------------------------------- 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 84f7e6287609c..7353cbce5558b 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -4175,7 +4175,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 fb45db1139349..7e73d89c2a18c 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 3eb935c833b87..2159a0dc2a5d7 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 de210c47b43ff..94105f10d71b5 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -8076,14 +8076,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..6fb01a4b159fa 100644 --- a/clang/test/AST/ast-dump-stmt.c +++ b/clang/test/AST/ast-dump-stmt.c @@ -400,7 +400,7 @@ void TestMiscStmts(void) { // 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: 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 diff --git a/clang/test/CodeGen/exprs.c b/clang/test/CodeGen/exprs.c index 5cca9722dcb3a..93015da074bf2 100644 --- a/clang/test/CodeGen/exprs.c +++ b/clang/test/CodeGen/exprs.c @@ -196,10 +196,17 @@ void f18(void) { // Ensure the right stmt is returned int f19(void) { - return ({ 3;;4;; }); + 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 d44ab5a65d5af..28740fa295768 100644 --- a/clang/test/Sema/statements.c +++ b/clang/test/Sema/statements.c @@ -119,14 +119,15 @@ void test_pr22849(void) { }; } -// GCC ignores empty statements at the end of compound expressions where the -// result type is concerned. +// Empty statements at the end of compound expressions have a result type 'void'. void test13(void) { int a; a = ({ 1; }); - 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; (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 ({}); } diff --git a/clang/test/SemaCXX/statements.cpp b/clang/test/SemaCXX/statements.cpp index 48f178dd9a8b3..426e9fa1e585b 100644 --- a/clang/test/SemaCXX/statements.cpp +++ b/clang/test/SemaCXX/statements.cpp @@ -43,8 +43,6 @@ T test7(T v) { return ({ // expected-warning{{use of GNU statement expression extension}} T a = v; a; - ; - ; }); } @@ -53,6 +51,21 @@ void test8() { 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 { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
