Ignore trailing NullStmts in StmtExprs for GCC compatibility.

Ignore trailing NullStmts in compound expressions when determining the result type and value. This is to match the GCC behavior which ignores semicolons at the end of compound expressions.

Patch by Dominic Ferreira.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@365498 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/Stmt.h b/include/clang/AST/Stmt.h
index 88aa535af2737c61934eacc9c737c556c9e414bf..403b88ac3a3c72e1a28662c7c530969a5ebf488f 100644 (file)
--- a/include/clang/AST/Stmt.h
+++ b/include/clang/AST/Stmt.h
@@ -1349,11 +1349,6 @@ public:
     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>;
 
@@ -1396,6 +1391,26 @@ public:
     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; }
 

diff --git a/lib/CodeGen/CGStmt.cpp b/lib/CodeGen/CGStmt.cpp
index 8909f8fe1eefddcc96204b337476612e8eda24ad..dd0dea5b94a03d221da444ccf506c1e917ff2091 100644 (file)
--- a/lib/CodeGen/CGStmt.cpp
+++ b/lib/CodeGen/CGStmt.cpp
@@ -381,44 +381,48 @@ CodeGenFunction::EmitCompoundStmtWithoutScope(const CompoundStmt &S,
                                               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);
     }
   }
 

diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
index cee160565f5000fad2339d7948fb7eef671351e6..bf04253ab7fdc7e335a400a579ae264c42e4fc16 100644 (file)
--- a/lib/Parse/ParseStmt.cpp
+++ b/lib/Parse/ParseStmt.cpp
@@ -972,10 +972,16 @@ bool Parser::ConsumeNullStmt(StmtVector &Stmts) {
 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)

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 466d9fdb9b974c47d6ca88e19c1cf30e160cd186..668abd2bec27ab71382eed3ba6f80479d89a2596 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -13432,7 +13432,9 @@ Sema::ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
   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();

diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h
index 7edb2a8d809f845f168f2d852baedec6e35d39ed..a6122c933848b0936b34cb797b5d1d7d41822936 100644 (file)
--- a/lib/Sema/TreeTransform.h
+++ b/lib/Sema/TreeTransform.h
@@ -6612,13 +6612,13 @@ TreeTransform<Derived>::TransformCompoundStmt(CompoundStmt *S,
                                               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

diff --git a/test/AST/ast-dump-stmt.c b/test/AST/ast-dump-stmt.c
index 8fec31d95a0e716131c7a1fd885d6e70810f3f96..46fd0b68bfe3bea41e88dc955799deb09e803f10 100644 (file)
--- a/test/AST/ast-dump-stmt.c
+++ b/test/AST/ast-dump-stmt.c
@@ -372,4 +372,14 @@ void TestMiscStmts(void) {
   // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}} <col:13> 'int' 10\r
   // CHECK-NEXT: ImplicitCastExpr\r
   // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}} <col:17> 'int' lvalue Var 0x{{[^ ]*}} 'a' 'int'\r
+  ({int a = 10; a;;; });\r
+  // CHECK-NEXT: StmtExpr 0x{{[^ ]*}} <line:[[@LINE-1]]:3, col:23> 'int'\r
+  // CHECK-NEXT: CompoundStmt\r
+  // CHECK-NEXT: DeclStmt\r
+  // CHECK-NEXT: VarDecl 0x{{[^ ]*}} <col:5, col:13> col:9 used a 'int' cinit\r
+  // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}} <col:13> 'int' 10\r
+  // CHECK-NEXT: ImplicitCastExpr\r
+  // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}} <col:17> 'int' lvalue Var 0x{{[^ ]*}} 'a' 'int'\r
+  // CHECK-NEXT: NullStmt\r
+  // CHECK-NEXT: NullStmt\r
 }\r

diff --git a/test/CodeGen/exprs.c b/test/CodeGen/exprs.c
index f46b5748f2386783d59820087be4e6b82b7cd70a..14f58957bc3d76073e997d7e966a8046f13cf397 100644 (file)
--- a/test/CodeGen/exprs.c
+++ b/test/CodeGen/exprs.c
@@ -196,3 +196,13 @@ void f18() {
 }
 // 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]]

diff --git a/test/Sema/statements.c b/test/Sema/statements.c
index ddaec8d433efda01ab9fbfe53e584c7872f20809..b410383673e8429aed646bcac2325abd5df6b0d3 100644 (file)
--- a/test/Sema/statements.c
+++ b/test/Sema/statements.c
@@ -119,3 +119,21 @@ void test_pr22849() {
     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:;; });
+}

diff --git a/test/SemaCXX/statements.cpp b/test/SemaCXX/statements.cpp
index 15b0b50ddc75240c3158c4e6f35edc0fbac8b5f6..75ed983ba84b26ad7c382ef1badb9f51ce99760a 100644 (file)
--- a/test/SemaCXX/statements.cpp
+++ b/test/SemaCXX/statements.cpp
@@ -37,3 +37,18 @@ 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);
+}