[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

[Dominic Ferreira via Phabricator via cfe-commits]

domdom created this revision.
domdom added reviewers: lattner, rsmith.
Herald added a subscriber: cfe-commits.

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.


Repository:
  rC Clang

https://reviews.llvm.org/D57086

Files:
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaExpr.cpp
  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/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13320,11 +13320,22 @@
   // 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();
+// GCC ignores empty statements at the end of compound expressions
+// i.e. ({ 5;;; })
+//   ^^ ignored
+// This code skips past these NullStmts
+Stmt *LastStmt = nullptr;
+for (Stmt *I : llvm::make_range(Compound->body_rbegin(),
+Compound->body_rend())) {
+  LastStmt = I;
+  if (!isa(LastStmt))
+break;
+}
 LabelStmt *LastLabelStmt = nullptr;
 // If LastStmt is a label, skip down through into the body.
 while (LabelStmt *Label = dyn_cast(LastStmt)) {
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -959,10 +959,18 @@
 
 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
+int 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/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/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13320,11 +13320,22 @@
   // 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();
+// GCC ignores empty statements at the end of compound expressions
+// i.e. ({ 5;;; })
+//   ^^ ignored
+// This code skips past these NullStmts
+Stmt *LastStmt = nullptr;
+for (Stmt *I : llvm::make_range(Compound->body_rbegin(),
+  

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

[Dominic Ferreira via Phabricator via cfe-commits]

domdom updated this revision to Diff 183043.
domdom added a comment.

Formatting


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57086/new/

https://reviews.llvm.org/D57086

Files:
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaExpr.cpp
  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/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13320,11 +13320,22 @@
   // 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();
+// GCC ignores empty statements at the end of compound expressions
+// i.e. ({ 5;;; })
+//   ^^ ignored
+// This code skips past these NullStmts
+Stmt *LastStmt = nullptr;
+for (Stmt *I : llvm::make_range(Compound->body_rbegin(),
+Compound->body_rend())) {
+  LastStmt = I;
+  if (!isa(LastStmt))
+break;
+}
 LabelStmt *LastLabelStmt = nullptr;
 // If LastStmt is a label, skip down through into the body.
 while (LabelStmt *Label = dyn_cast(LastStmt)) {
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -959,10 +959,18 @@
 
 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
+int 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/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/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13320,11 +13320,22 @@
   // 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();
+// GCC ignores empty statements at the end of compound expressions
+// i.e. ({ 5;;; })
+//   ^^ ignored
+// This code skips past these NullStmts
+Stmt *LastStmt = nullptr;
+for (Stmt *I : llvm::make_range(Compound->body_rbegin(),
+Compound->body_rend())) {
+  LastStmt = I;
+  if (!isa(LastStmt))
+break;
+}
 LabelStmt *LastLabelStmt = nullptr;
 // If LastStmt

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

[Dominic Ferreira via Phabricator via cfe-commits]

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{{[^ ]*}}  'int' 10
   // CHECK-NEXT: ImplicitCastExpr
   // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}}  'int' lvalue Var 0x{{[^ ]*}} 'a' 'int'
+  ({int a = 10; a;;;});
+  // CHECK-NEXT: StmtExpr 0x{{[^ ]*}}  'int'
+  // CHECK-NEXT: CompoundStmt
+  // CHECK-NEXT: DeclStmt
+  // CHECK-NEXT: VarDecl 0x{{[^ ]*}}  col:9 used a 'int' cinit
+  // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}}  'int' 10
+  // CHECK-NEXT: ImplicitCastExpr
+  // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}}  '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(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/li

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

[Dominic Ferreira via Phabricator via cfe-commits]

domdom updated this revision to Diff 184413.
domdom marked 6 inline comments as done.

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


Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -382,7 +382,7 @@
   bool GetLast,
   AggValueSlot AggSlot) {
 
-  Stmt *ExprResult = GetLast ? S.getStmtExprResult() : nullptr;
+  const Stmt *ExprResult = GetLast ? S.getStmtExprResult() : nullptr;
   Address RetAlloca = Address::invalid();
 
   for (CompoundStmt::const_body_iterator I = S.body_begin(), E = S.body_end();
Index: clang/include/clang/AST/Stmt.h
===
--- clang/include/clang/AST/Stmt.h
+++ clang/include/clang/AST/Stmt.h
@@ -1256,8 +1256,8 @@
   // 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");
+  unsigned getIndexOfLastNonNullStmt() const {
+assert(!body_empty() && "getIndexOfLastNonNullStmt");
 for (unsigned I = size(), E = 0; I != E; I--) {
   if (!isa(body_begin()[I - 1]))
 return I - 1;
@@ -1298,7 +1298,7 @@
   // another Stmt.
   void setStmtExpr(Stmt *S) {
 assert(!body_empty() && "setStmtExpr");
-unsigned ExprResult = getLastNonNullStmt();
+unsigned ExprResult = getIndexOfLastNonNullStmt();
 assert(!isa(body_begin()[ExprResult])
 && "setStmtExpr but there is no non-NullStmt");
 body_begin()[ExprResult] = S;
@@ -1306,7 +1306,7 @@
 
   // Get the Stmt representing the result of this compound expression.
   Stmt *getStmtExprResult() const {
-unsigned ExprResult = getLastNonNullStmt();
+unsigned ExprResult = getIndexOfLastNonNullStmt();
 return body_begin()[ExprResult];
   }
 


Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -382,7 +382,7 @@
   bool GetLast,
   AggValueSlot AggSlot) {
 
-  Stmt *ExprResult = GetLast ? S.getStmtExprResult() : nullptr;
+  const Stmt *ExprResult = GetLast ? S.getStmtExprResult() : nullptr;
   Address RetAlloca = Address::invalid();
 
   for (CompoundStmt::const_body_iterator I = S.body_begin(), E = S.body_end();
Index: clang/include/clang/AST/Stmt.h
===
--- clang/include/clang/AST/Stmt.h
+++ clang/include/clang/AST/Stmt.h
@@ -1256,8 +1256,8 @@
   // 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");
+  unsigned getIndexOfLastNonNullStmt() const {
+assert(!body_empty() && "getIndexOfLastNonNullStmt");
 for (unsigned I = size(), E = 0; I != E; I--) {
   if (!isa(body_begin()[I - 1]))
 return I - 1;
@@ -1298,7 +1298,7 @@
   // another Stmt.
   void setStmtExpr(Stmt *S) {
 assert(!body_empty() && "setStmtExpr");
-unsigned ExprResult = getLastNonNullStmt();
+unsigned ExprResult = getIndexOfLastNonNullStmt();
 assert(!isa(body_begin()[ExprResult])
 && "setStmtExpr but there is no non-NullStmt");
 body_begin()[ExprResult] = S;
@@ -1306,7 +1306,7 @@
 
   // Get the Stmt representing the result of this compound expression.
   Stmt *getStmtExprResult() const {
-unsigned ExprResult = getLastNonNullStmt();
+unsigned ExprResult = getIndexOfLastNonNullStmt();
 return body_begin()[ExprResult];
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

[Dominic Ferreira via Phabricator via cfe-commits]

domdom updated this revision to Diff 184414.

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{{[^ ]*}}  'int' 10
   // CHECK-NEXT: ImplicitCastExpr
   // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}}  'int' lvalue Var 0x{{[^ ]*}} 'a' 'int'
+  ({int a = 10; a;;;});
+  // CHECK-NEXT: StmtExpr 0x{{[^ ]*}}  'int'
+  // CHECK-NEXT: CompoundStmt
+  // CHECK-NEXT: DeclStmt
+  // CHECK-NEXT: VarDecl 0x{{[^ ]*}}  col:9 used a 'int' cinit
+  // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}}  'int' 10
+  // CHECK-NEXT: ImplicitCastExpr
+  // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}}  '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(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.b

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

[Dominic Ferreira via Phabricator via cfe-commits]

domdom marked an inline comment as not done.
domdom added inline comments.



Comment at: clang/include/clang/AST/Stmt.h:1259
+  // the index of the last one.
+  unsigned getLastNonNullStmt() const {
+assert(!body_empty() && "getLastNonNullStmt");

aaron.ballman wrote:
> How about `getIndexOfLastNonNullStmt()` since it doesn't return the `Stmt*` 
> itself?
Agreed.



Comment at: clang/include/clang/AST/Stmt.h:1300
+  void setStmtExpr(Stmt *S) {
+assert(!body_empty() && "setStmtExpr");
+unsigned ExprResult = getLastNonNullStmt();

getLastNonNullStmt asserts anyway, should I remove this?



Comment at: clang/lib/CodeGen/CGStmt.cpp:390
+  E = S.body_end(); I != E; ++I) {
+if (GetLast && ExprResult == *I) {
+  // We have to special case labels here.  They are statements, but when 
put

aaron.ballman wrote:
> What happens if `ExprResult` is `nullptr`?
Then ExprResult == *I should not be evaluated. (Since GetLast would be false)



Comment at: clang/test/CodeGen/exprs.c:202
+int f19() {
+  return ({ 3;;4;; });
+}

aaron.ballman wrote:
> Does this test need the extra null statement between the `3;` and `4;`?
Not strictly speaking, no. Just added it to ensure it has no effect.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57086/new/

https://reviews.llvm.org/D57086



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

[Dominic Ferreira via Phabricator via cfe-commits]

domdom updated this revision to Diff 185196.
domdom added a comment.

Switched  getIndexOfLastNonNullStmt to return Optional instead of 
Stmt* as per comments.


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{{[^ ]*}}  'int' 10
   // CHECK-NEXT: ImplicitCastExpr
   // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}}  'int' lvalue Var 0x{{[^ ]*}} 'a' 'int'
+  ({int a = 10; a;;;});
+  // CHECK-NEXT: StmtExpr 0x{{[^ ]*}}  'int'
+  // CHECK-NEXT: CompoundStmt
+  // CHECK-NEXT: DeclStmt
+  // CHECK-NEXT: VarDecl 0x{{[^ ]*}}  col:9 used a 'int' cinit
+  // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}}  'int' 10
+  // CHECK-NEXT: ImplicitCastExpr
+  // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}}  '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(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,40 @@
   bool GetLast,
  

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

[Dominic Ferreira via Phabricator via cfe-commits]

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
+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{{[^ ]*}}  'int' 10
   // CHECK-NEXT: ImplicitCastExpr
   // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}}  'int' lvalue Var 0x{{[^ ]*}} 'a' 'int'
+  ({int a = 10; a;;;});
+  // CHECK-NEXT: StmtExpr 0x{{[^ ]*}}  'int'
+  // CHECK-NEXT: CompoundStmt
+  // CHECK-NEXT: DeclStmt
+  // CHECK-NEXT: VarDecl 0x{{[^ ]*}}  col:9 used a 'int' cinit
+  // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}}  'int' 10
+  // CHECK-NEXT: ImplicitCastExpr
+  // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}}  '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 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(Compound->body_back())) {
+// For GCC compatibility we get the last Stmt excluding trailing NullStmts.
+if (const auto *LastStmt = dyn_cast(Compound->getStmtExprResult())) {
   if (const Expr *Value = LastStmt->getExprStmt()) {
 StmtExprMayBindToTemp = true;
 Ty = Value->getType();
Index: clang/lib/Parse/P

[PATCH] D62571: Implement codegen for MSVC unions with reference members

[Dominic Ferreira via Phabricator via cfe-commits]

domdom updated this revision to Diff 205495.
domdom added a comment.

clang-formatted the test.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62571/new/

https://reviews.llvm.org/D62571

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGenCXX/ms-union-member-ref.cpp


Index: clang/test/CodeGenCXX/ms-union-member-ref.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ms-union-member-ref.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -fms-extensions %s -emit-llvm -o- | FileCheck %s
+
+union A {
+  int *&ref;
+  int **ptr;
+};
+
+int *f1(A *a) {
+  return a->ref;
+}
+// CHECK-LABEL: define {{.*}}i32* @_Z2f1P1A(%union.A* %a)
+// CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:   ret i32* [[IP]]
+
+void f2(A *a) {
+  *a->ref = 1;
+}
+// CHECK-LABEL: define {{.*}}void @_Z2f2P1A(%union.A* %a)
+// CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:   store i32 1, i32* [[IP]]
+
+bool f3(A *a, int *b) {
+  return a->ref != b;
+}
+// CHECK-LABEL: define {{.*}}i1 @_Z2f3P1APi(%union.A* %a, i32* %b)
+// CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:   [[IP2:%[^[:space:]]+]]  = load i32*, i32** %b.addr
+// CHECK:   icmp ne i32* [[IP]], [[IP2]]
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3929,29 +3929,32 @@
   unsigned RecordCVR = base.getVRQualifiers();
   if (rec->isUnion()) {
 // For unions, there is no pointer adjustment.
-assert(!FieldType->isReferenceType() && "union has reference member");
 if (CGM.getCodeGenOpts().StrictVTablePointers &&
 hasAnyVptr(FieldType, getContext()))
   // Because unions can easily skip invariant.barriers, we need to add
   // a barrier every time CXXRecord field with vptr is referenced.
   addr = Address(Builder.CreateLaunderInvariantGroup(addr.getPointer()),
  addr.getAlignment());
+
+if (FieldType->isReferenceType())
+  addr = Builder.CreateElementBitCast(
+  addr, CGM.getTypes().ConvertTypeForMem(FieldType), field->getName());
   } else {
 // For structs, we GEP to the field that the record layout suggests.
 addr = emitAddrOfFieldStorage(*this, addr, field);
+  }
 
-// If this is a reference field, load the reference right now.
-if (FieldType->isReferenceType()) {
-  LValue RefLVal = MakeAddrLValue(addr, FieldType, FieldBaseInfo,
-  FieldTBAAInfo);
-  if (RecordCVR & Qualifiers::Volatile)
-RefLVal.getQuals().addVolatile();
-  addr = EmitLoadOfReference(RefLVal, &FieldBaseInfo, &FieldTBAAInfo);
-
-  // Qualifiers on the struct don't apply to the referencee.
-  RecordCVR = 0;
-  FieldType = FieldType->getPointeeType();
-}
+  // If this is a reference field, load the reference right now.
+  if (FieldType->isReferenceType()) {
+LValue RefLVal = MakeAddrLValue(addr, FieldType, FieldBaseInfo,
+FieldTBAAInfo);
+if (RecordCVR & Qualifiers::Volatile)
+  RefLVal.getQuals().addVolatile();
+addr = EmitLoadOfReference(RefLVal, &FieldBaseInfo, &FieldTBAAInfo);
+
+// Qualifiers on the struct don't apply to the referencee.
+RecordCVR = 0;
+FieldType = FieldType->getPointeeType();
   }
 
   // Make sure that the address is pointing to the right type.  This is 
critical


Index: clang/test/CodeGenCXX/ms-union-member-ref.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ms-union-member-ref.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -fms-extensions %s -emit-llvm -o- | FileCheck %s
+
+union A {
+  int *&ref;
+  int **ptr;
+};
+
+int *f1(A *a) {
+  return a->ref;
+}
+// CHECK-LABEL: define {{.*}}i32* @_Z2f1P1A(%union.A* %a)
+// CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:   ret i32* [[IP]]
+
+void f2(A *a) {
+  *a->ref = 1;
+}
+// CHECK-LABEL: define {{.*}}void @_Z2f2P1A(%union.A* %a)
+// CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:   store i32 1, i32* [[IP]]
+
+bool f3(A *a, int *b) {
+  

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

[Dominic Ferreira via Phabricator via cfe-commits]

domdom updated this revision to Diff 205501.
domdom added a comment.

clang-format the patch


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 
+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{{[^ ]*}}  'int' 10
   // CHECK-NEXT: ImplicitCastExpr
   // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}}  'int' lvalue Var 0x{{[^ ]*}} 'a' 'int'
+  ({int a = 10; a;;; });
+  // CHECK-NEXT: StmtExpr 0x{{[^ ]*}}  'int'
+  // CHECK-NEXT: CompoundStmt
+  // CHECK-NEXT: DeclStmt
+  // CHECK-NEXT: VarDecl 0x{{[^ ]*}}  col:9 used a 'int' cinit
+  // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}}  'int' 10
+  // CHECK-NEXT: ImplicitCastExpr
+  // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}}  '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 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(Compound->body_back())) {
+// For GCC compatibility we get the last Stmt excluding trailing NullStmts.
+if (const auto *LastStmt =
+dyn_cast(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;
-//

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

[Dominic Ferreira via Phabricator via cfe-commits]

domdom added a comment.

In D57086#1550514 , @aaron.ballman 
wrote:

> In D57086#1549632 , @domdom wrote:
>
> > clang-format the patch
>
>
> Thanks! Do you need someone to commit on your behalf?


You are very welcome; thank you both for your comments!

I do need someone to commit on my behalf :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57086/new/

https://reviews.llvm.org/D57086



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

[Dominic Ferreira via Phabricator via cfe-commits]

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 
+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{{[^ ]*}}  'int' 10
   // CHECK-NEXT: ImplicitCastExpr
   // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}}  'int' lvalue Var 0x{{[^ ]*}} 'a' 'int'
+  ({int a = 10; a;;; });
+  // CHECK-NEXT: StmtExpr 0x{{[^ ]*}}  'int'
+  // CHECK-NEXT: CompoundStmt
+  // CHECK-NEXT: DeclStmt
+  // CHECK-NEXT: VarDecl 0x{{[^ ]*}}  col:9 used a 'int' cinit
+  // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}}  'int' 10
+  // CHECK-NEXT: ImplicitCastExpr
+  // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}}  '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 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(Compound->body_back())) {
+// For GCC compatibility we get the last Stmt excluding trailing NullStmts.
+if (const auto *LastStmt =
+dyn_cast(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

[PATCH] D62571: Implement codegen for MSVC unions with reference members

[Dominic Ferreira via Phabricator via cfe-commits]

domdom created this revision.
domdom added a reviewer: asl.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently, clang accepts a union with a reference member when given the 
-fms-extensions flag. This change fixes the codegen for this case.


Repository:
  rC Clang

https://reviews.llvm.org/D62571

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGenCXX/ms-union-member-ref.cpp


Index: clang/test/CodeGenCXX/ms-union-member-ref.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ms-union-member-ref.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -fms-extensions %s -emit-llvm -o- | FileCheck %s
+
+union A {
+int *&ref;
+int **ptr;
+};
+
+int *f1(A *a) {
+return a->ref;
+}
+// CHECK-LABEL: define {{.*}}i32* @_Z2f1P1A(%union.A* %a)
+// CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:   ret i32* [[IP]]
+
+void f2(A *a) {
+*a->ref = 1;
+}
+// CHECK-LABEL: define {{.*}}void @_Z2f2P1A(%union.A* %a)
+// CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:   store i32 1, i32* [[IP]]
+
+bool f3(A *a, int *b) {
+  return a->ref != b;
+}
+// CHECK-LABEL: define {{.*}}i1 @_Z2f3P1APi(%union.A* %a, i32* %b)
+// CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:   [[IP2:%[^[:space:]]+]]  = load i32*, i32** %b.addr
+// CHECK:   icmp ne i32* [[IP]], [[IP2]]
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3929,29 +3929,32 @@
   unsigned RecordCVR = base.getVRQualifiers();
   if (rec->isUnion()) {
 // For unions, there is no pointer adjustment.
-assert(!FieldType->isReferenceType() && "union has reference member");
 if (CGM.getCodeGenOpts().StrictVTablePointers &&
 hasAnyVptr(FieldType, getContext()))
   // Because unions can easily skip invariant.barriers, we need to add
   // a barrier every time CXXRecord field with vptr is referenced.
   addr = Address(Builder.CreateLaunderInvariantGroup(addr.getPointer()),
  addr.getAlignment());
+
+if (FieldType->isReferenceType())
+  addr = Builder.CreateElementBitCast(
+  addr, CGM.getTypes().ConvertTypeForMem(FieldType), field->getName());
   } else {
 // For structs, we GEP to the field that the record layout suggests.
 addr = emitAddrOfFieldStorage(*this, addr, field);
+  }
 
-// If this is a reference field, load the reference right now.
-if (FieldType->isReferenceType()) {
-  LValue RefLVal = MakeAddrLValue(addr, FieldType, FieldBaseInfo,
-  FieldTBAAInfo);
-  if (RecordCVR & Qualifiers::Volatile)
-RefLVal.getQuals().addVolatile();
-  addr = EmitLoadOfReference(RefLVal, &FieldBaseInfo, &FieldTBAAInfo);
-
-  // Qualifiers on the struct don't apply to the referencee.
-  RecordCVR = 0;
-  FieldType = FieldType->getPointeeType();
-}
+  // If this is a reference field, load the reference right now.
+  if (FieldType->isReferenceType()) {
+LValue RefLVal = MakeAddrLValue(addr, FieldType, FieldBaseInfo,
+FieldTBAAInfo);
+if (RecordCVR & Qualifiers::Volatile)
+  RefLVal.getQuals().addVolatile();
+addr = EmitLoadOfReference(RefLVal, &FieldBaseInfo, &FieldTBAAInfo);
+
+// Qualifiers on the struct don't apply to the referencee.
+RecordCVR = 0;
+FieldType = FieldType->getPointeeType();
   }
 
   // Make sure that the address is pointing to the right type.  This is 
critical


Index: clang/test/CodeGenCXX/ms-union-member-ref.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ms-union-member-ref.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -fms-extensions %s -emit-llvm -o- | FileCheck %s
+
+union A {
+int *&ref;
+int **ptr;
+};
+
+int *f1(A *a) {
+return a->ref;
+}
+// CHECK-LABEL: define {{.*}}i32* @_Z2f1P1A(%union.A* %a)
+// CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:   ret i32* [[IP]]
+
+void f2(A *a) {
+*a->ref = 1;
+}
+// CHECK-LABEL: define {{.*}}void @_Z2f2P1A(%union.A* %a)
+// CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*

[PATCH] D62571: Implement codegen for MSVC unions with reference members

[Dominic Ferreira via Phabricator via cfe-commits]

domdom added a comment.

Ping!

Cheers,
Dom


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62571/new/

https://reviews.llvm.org/D62571



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62571: Implement codegen for MSVC unions with reference members

[Dominic Ferreira via Phabricator via cfe-commits]

domdom added a comment.

Thanks @aaron.ballman!

I will need someone to commit this for me :)

Cheers,
Dom


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62571/new/

https://reviews.llvm.org/D62571



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62571: Implement codegen for MSVC unions with reference members

[Dominic Ferreira via Phabricator via cfe-commits]

domdom updated this revision to Diff 217289.
domdom added a comment.

Rebased onto master, clang format the patch.

Merge conflict resolve by having the bitcast of the field reference happening 
after recording access index.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62571/new/

https://reviews.llvm.org/D62571

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGenCXX/ms-union-member-ref.cpp


Index: clang/test/CodeGenCXX/ms-union-member-ref.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ms-union-member-ref.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -fms-extensions %s -emit-llvm -o- | FileCheck %s
+
+union A {
+  int *&ref;
+  int **ptr;
+};
+
+int *f1(A *a) {
+  return a->ref;
+}
+// CHECK-LABEL: define {{.*}}i32* @_Z2f1P1A(%union.A* %a)
+// CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:   ret i32* [[IP]]
+
+void f2(A *a) {
+  *a->ref = 1;
+}
+// CHECK-LABEL: define {{.*}}void @_Z2f2P1A(%union.A* %a)
+// CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:   store i32 1, i32* [[IP]]
+
+bool f3(A *a, int *b) {
+  return a->ref != b;
+}
+// CHECK-LABEL: define {{.*}}i1 @_Z2f3P1APi(%union.A* %a, i32* %b)
+// CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:   [[IP2:%[^[:space:]]+]]  = load i32*, i32** %b.addr
+// CHECK:   icmp ne i32* [[IP]], [[IP2]]
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -4057,7 +4057,6 @@
   unsigned RecordCVR = base.getVRQualifiers();
   if (rec->isUnion()) {
 // For unions, there is no pointer adjustment.
-assert(!FieldType->isReferenceType() && "union has reference member");
 if (CGM.getCodeGenOpts().StrictVTablePointers &&
 hasAnyVptr(FieldType, getContext()))
   // Because unions can easily skip invariant.barriers, we need to add
@@ -4074,27 +4073,30 @@
   addr.getPointer(), getDebugInfoFIndex(rec, 
field->getFieldIndex()), DbgInfo),
   addr.getAlignment());
 }
-  } else {
 
+if (FieldType->isReferenceType())
+  addr = Builder.CreateElementBitCast(
+  addr, CGM.getTypes().ConvertTypeForMem(FieldType), field->getName());
+  } else {
 if (!IsInPreservedAIRegion)
   // For structs, we GEP to the field that the record layout suggests.
   addr = emitAddrOfFieldStorage(*this, addr, field);
 else
   // Remember the original struct field index
   addr = emitPreserveStructAccess(*this, addr, field);
+  }
 
-// If this is a reference field, load the reference right now.
-if (FieldType->isReferenceType()) {
-  LValue RefLVal = MakeAddrLValue(addr, FieldType, FieldBaseInfo,
-  FieldTBAAInfo);
-  if (RecordCVR & Qualifiers::Volatile)
-RefLVal.getQuals().addVolatile();
-  addr = EmitLoadOfReference(RefLVal, &FieldBaseInfo, &FieldTBAAInfo);
-
-  // Qualifiers on the struct don't apply to the referencee.
-  RecordCVR = 0;
-  FieldType = FieldType->getPointeeType();
-}
+  // If this is a reference field, load the reference right now.
+  if (FieldType->isReferenceType()) {
+LValue RefLVal =
+MakeAddrLValue(addr, FieldType, FieldBaseInfo, FieldTBAAInfo);
+if (RecordCVR & Qualifiers::Volatile)
+  RefLVal.getQuals().addVolatile();
+addr = EmitLoadOfReference(RefLVal, &FieldBaseInfo, &FieldTBAAInfo);
+
+// Qualifiers on the struct don't apply to the referencee.
+RecordCVR = 0;
+FieldType = FieldType->getPointeeType();
   }
 
   // Make sure that the address is pointing to the right type.  This is 
critical


Index: clang/test/CodeGenCXX/ms-union-member-ref.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ms-union-member-ref.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -fms-extensions %s -emit-llvm -o- | FileCheck %s
+
+union A {
+  int *&ref;
+  int **ptr;
+};
+
+int *f1(A *a) {
+  return a->ref;
+}
+// CHECK-LABEL: define {{.*}}i32* @_Z2f1P1A(%union.A* %a)
+// CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:   ret i32* [[IP]]
+
+void f2(A *a) {
+  *a->ref = 1;
+}
+// CHECK-LABEL: define {{.*}}void @_Z2f2P1A(%union.A* %a)
+// CHECK:   [[REF:%[