domdom updated this revision to Diff 203775.
domdom added a comment.
Sorry I it's taken me a while to get back to this work. I've rebased the
changes and taken advantage of the refactoring to stop modifying the
CompoundStmt after creating it. This definitely simplified the changes required
in Stmt.h, which is nice.
I've addressed the the need to update the TreeTransformer for the template
case, and added a test case for that.
Something I should ask, it seems like GCC only ignores the NullStmts at the end
if it's in C mode. Should clang match this behaviour exactly?
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,16 @@
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,17 @@
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:22> '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,14 @@
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);
+ 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,8 @@
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
