domdom updated this revision to Diff 208550.
domdom added a comment.
Thank you for catching that @aaron.ballman!
I forgot to update the test after the running clang-format. I thought I ran the
tests again before submitting, but I guess I didn't :P
Thanks again!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57086/new/
https://reviews.llvm.org/D57086
Files:
clang/include/clang/AST/Stmt.h
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
Index: clang/test/SemaCXX/statements.cpp
===================================================================
--- clang/test/SemaCXX/statements.cpp
+++ clang/test/SemaCXX/statements.cpp
@@ -37,3 +37,18 @@
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);
+}
Index: clang/test/Sema/statements.c
===================================================================
--- clang/test/Sema/statements.c
+++ clang/test/Sema/statements.c
@@ -119,3 +119,21 @@
SIZE = sizeof(({unsigned long __ptr; __ptr;}))
};
}
+
+// GCC ignores empty statements at the end of compound expressions where the
+// result type is concerned.
+void test13() {
+ 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() { return ({}); }
+void test15() {
+ return ({;;;; });
+}
+void test16() {
+ return ({test:;; });
+}
Index: clang/test/CodeGen/exprs.c
===================================================================
--- clang/test/CodeGen/exprs.c
+++ clang/test/CodeGen/exprs.c
@@ -196,3 +196,13 @@
}
// CHECK-LABEL: define void @f18()
// CHECK: call i32 @returns_int()
+
+// Ensure the right stmt is returned
+int f19() {
+ return ({ 3;;4;; });
+}
+// CHECK-LABEL: define i32 @f19()
+// CHECK: [[T:%.*]] = alloca i32
+// CHECK: store i32 4, i32* [[T]]
+// CHECK: [[L:%.*]] = load i32, i32* [[T]]
+// CHECK: ret i32 [[L]]
Index: clang/test/AST/ast-dump-stmt.c
===================================================================
--- clang/test/AST/ast-dump-stmt.c
+++ clang/test/AST/ast-dump-stmt.c
@@ -372,4 +372,14 @@
// 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
}
Index: clang/lib/Sema/TreeTransform.h
===================================================================
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -6604,13 +6604,13 @@
bool IsStmtExpr) {
Sema::CompoundScopeRAII CompoundScope(getSema());
+ 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 == S->body_back() ? SDK_StmtExprResult : SDK_Discarded);
+ B, IsStmtExpr && B == ExprResult ? SDK_StmtExprResult : SDK_Discarded);
if (Result.isInvalid()) {
// Immediately fail if this was a DeclStmt, since it's very
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13372,7 +13372,9 @@
QualType Ty = Context.VoidTy;
bool StmtExprMayBindToTemp = false;
if (!Compound->body_empty()) {
- if (const auto *LastStmt = dyn_cast<ValueStmt>(Compound->body_back())) {
+ // For GCC compatibility we get the last Stmt excluding trailing NullStmts.
+ if (const auto *LastStmt =
+ dyn_cast<ValueStmt>(Compound->getStmtExprResult())) {
if (const Expr *Value = LastStmt->getExprStmt()) {
StmtExprMayBindToTemp = true;
Ty = Value->getType();
Index: clang/lib/Parse/ParseStmt.cpp
===================================================================
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -972,10 +972,16 @@
StmtResult Parser::handleExprStmt(ExprResult E, ParsedStmtContext StmtCtx) {
bool IsStmtExprResult = false;
if ((StmtCtx & ParsedStmtContext::InStmtExpr) != ParsedStmtContext()) {
- // 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);
+ // 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 statment
+ // expression.
+ IsStmtExprResult = GetLookAheadToken(LookAhead).is(tok::r_brace) &&
+ GetLookAheadToken(LookAhead + 1).is(tok::r_paren);
}
if (IsStmtExprResult)
Index: clang/lib/CodeGen/CGStmt.cpp
===================================================================
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -381,44 +381,48 @@
bool GetLast,
AggValueSlot AggSlot) {
- for (CompoundStmt::const_body_iterator I = S.body_begin(),
- E = S.body_end()-GetLast; I != E; ++I)
- EmitStmt(*I);
+ const Stmt *ExprResult = S.getStmtExprResult();
+ assert((!GetLast || (GetLast && ExprResult)) &&
+ "If GetLast is true then the CompoundStmt must have a StmtExprResult");
Address RetAlloca = Address::invalid();
- 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");
+
+ 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");
+ }
}
- }
- EnsureInsertPoint();
+ EnsureInsertPoint();
- const Expr *E = cast<Expr>(LastStmt);
- QualType ExprTy = E->getType();
- if (hasAggregateEvaluationKind(ExprTy)) {
- EmitAggExpr(E, AggSlot);
+ 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);
+ }
} 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);
+ EmitStmt(CurStmt);
}
}
Index: clang/include/clang/AST/Stmt.h
===================================================================
--- clang/include/clang/AST/Stmt.h
+++ clang/include/clang/AST/Stmt.h
@@ -1323,11 +1323,6 @@
return !body_empty() ? body_begin()[size() - 1] : nullptr;
}
- void setLastStmt(Stmt *S) {
- assert(!body_empty() && "setLastStmt");
- body_begin()[size() - 1] = S;
- }
-
using const_body_iterator = Stmt *const *;
using body_const_range = llvm::iterator_range<const_body_iterator>;
@@ -1370,6 +1365,26 @@
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 CompoundStmtBits.LBraceLoc; }
SourceLocation getEndLoc() const { return RBraceLoc; }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
