domdom updated this revision to Diff 184001.
domdom added a comment.
Thanks for your comments, @aaron.ballman. I've addressed the comments and added
a test case as suggested.
This revealed an issue with the code-gen side of things, so I fixed that and
added another test case for that as well.
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/test/AST/ast-dump-stmt.c
clang/test/CodeGen/exprs.c
clang/test/Sema/statements.c
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/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13320,11 +13320,12 @@
// More semantic analysis is needed.
// If there are sub-stmts in the compound stmt, take the type of the last one
- // as the type of the stmtexpr.
+ // as the type of the stmtexpr. For GCC compatibility this excludes trailing
+ // NullStmts.
QualType Ty = Context.VoidTy;
bool StmtExprMayBindToTemp = false;
if (!Compound->body_empty()) {
- Stmt *LastStmt = Compound->body_back();
+ Stmt *LastStmt = Compound->getStmtExprResult();
LabelStmt *LastLabelStmt = nullptr;
// If LastStmt is a label, skip down through into the body.
while (LabelStmt *Label = dyn_cast<LabelStmt>(LastStmt)) {
@@ -13360,7 +13361,7 @@
return ExprError();
if (LastExpr.get() != nullptr) {
if (!LastLabelStmt)
- Compound->setLastStmt(LastExpr.get());
+ Compound->setStmtExpr(LastExpr.get());
else
LastLabelStmt->setSubStmt(LastExpr.get());
StmtExprMayBindToTemp = true;
Index: clang/lib/Parse/ParseStmt.cpp
===================================================================
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -959,10 +959,16 @@
bool Parser::isExprValueDiscarded() {
if (Actions.isCurCompoundStmtAStmtExpr()) {
- // 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.
- return Tok.isNot(tok::r_brace) || NextToken().isNot(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.
+ return GetLookAheadToken(LookAhead).isNot(tok::r_brace) ||
+ GetLookAheadToken(LookAhead + 1).isNot(tok::r_paren);
}
return true;
}
Index: clang/lib/CodeGen/CGStmt.cpp
===================================================================
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -382,36 +382,38 @@
bool GetLast,
AggValueSlot AggSlot) {
- for (CompoundStmt::const_body_iterator I = S.body_begin(),
- E = S.body_end()-GetLast; I != E; ++I)
- EmitStmt(*I);
-
+ Stmt *ExprResult = GetLast ? S.getStmtExprResult() : nullptr;
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.
- const Stmt *LastStmt = S.body_back();
- while (const LabelStmt *LS = dyn_cast<LabelStmt>(LastStmt)) {
- EmitLabel(LS->getDecl());
- LastStmt = LS->getSubStmt();
- }
- EnsureInsertPoint();
+ for (CompoundStmt::const_body_iterator I = S.body_begin(),
+ E = S.body_end(); I != E; ++I) {
+ if (GetLast && ExprResult == *I) {
+ // 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.
+ const Stmt *LastStmt = ExprResult;
+ while (const LabelStmt *LS = dyn_cast<LabelStmt>(LastStmt)) {
+ EmitLabel(LS->getDecl());
+ LastStmt = LS->getSubStmt();
+ }
+
+ EnsureInsertPoint();
- QualType ExprTy = cast<Expr>(LastStmt)->getType();
- if (hasAggregateEvaluationKind(ExprTy)) {
- EmitAggExpr(cast<Expr>(LastStmt), AggSlot);
+ QualType ExprTy = cast<Expr>(LastStmt)->getType();
+ if (hasAggregateEvaluationKind(ExprTy)) {
+ EmitAggExpr(cast<Expr>(LastStmt), 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(cast<Expr>(LastStmt), 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(cast<Expr>(LastStmt), RetAlloca, Qualifiers(),
- /*IsInit*/false);
+ EmitStmt(*I);
}
-
}
return RetAlloca;
Index: clang/include/clang/AST/Stmt.h
===================================================================
--- clang/include/clang/AST/Stmt.h
+++ clang/include/clang/AST/Stmt.h
@@ -1250,6 +1250,21 @@
void setStmts(ArrayRef<Stmt *> Stmts);
+ // GCC ignores empty statements at the end of compound expressions.
+ // i.e. ({ 5;;; })
+ // ^^ ignored
+ // This gets the index of the last Stmt before the trailing NullStmts. If
+ // this compound expression contains nothing but NullStmts, then we return
+ // the index of the last one.
+ unsigned getLastNonNullStmt() const {
+ assert(!body_empty() && "getLastNonNullStmt");
+ for (unsigned I = size(), E = 0; I != E; I--) {
+ if (!isa<NullStmt>(body_begin()[I - 1]))
+ return I - 1;
+ }
+ return size() - 1;
+ }
+
public:
static CompoundStmt *Create(const ASTContext &C, ArrayRef<Stmt *> Stmts,
SourceLocation LB, SourceLocation RB);
@@ -1279,9 +1294,20 @@
return !body_empty() ? body_begin()[size() - 1] : nullptr;
}
- void setLastStmt(Stmt *S) {
- assert(!body_empty() && "setLastStmt");
- body_begin()[size() - 1] = S;
+ // Replace the Stmt that would be the result of this compound expression with
+ // another Stmt.
+ void setStmtExpr(Stmt *S) {
+ assert(!body_empty() && "setStmtExpr");
+ unsigned ExprResult = getLastNonNullStmt();
+ assert(!isa<NullStmt>(body_begin()[ExprResult])
+ && "setStmtExpr but there is no non-NullStmt");
+ body_begin()[ExprResult] = S;
+ }
+
+ // Get the Stmt representing the result of this compound expression.
+ Stmt *getStmtExprResult() const {
+ unsigned ExprResult = getLastNonNullStmt();
+ return body_begin()[ExprResult];
}
using const_body_iterator = Stmt *const *;
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
