Fix handling of pointers-to-members and comma expressions when

lifetime-extending temporaries in reference bindings.

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

diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h
index 9509456839c45c1f81599b72efcf25c19f477d18..ee0a3fd145cbfef46a98b795e642da5b6ff523a1 100644 (file)
--- a/include/clang/AST/Expr.h
+++ b/include/clang/AST/Expr.h
@@ -760,10 +760,10 @@ public:
 
   /// Walk outwards from an expression we want to bind a reference to and
   /// find the expression whose lifetime needs to be extended. Record
-  /// the adjustments needed along the path.
-  const Expr *
-  skipRValueSubobjectAdjustments(
-                       SmallVectorImpl<SubobjectAdjustment> &Adjustments) const;
+  /// the LHSs of comma expressions and adjustments needed along the path.
+  const Expr *skipRValueSubobjectAdjustments(
+      SmallVectorImpl<const Expr *> &CommaLHS,
+      SmallVectorImpl<SubobjectAdjustment> &Adjustments) const;
 
   /// Skip irrelevant expressions to find what should be materialize for
   /// binding with a reference.

diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
index f587dfa93e972bbb0a9af432140fd1c89ed964f0..2a9b9eb4534e4dcb8bdbc32c28de69d4b7326570 100644 (file)
--- a/lib/AST/Expr.cpp
+++ b/lib/AST/Expr.cpp
@@ -50,9 +50,9 @@ const CXXRecordDecl *Expr::getBestDynamicClassType() const {
   return cast<CXXRecordDecl>(D);
 }
 
-const Expr *
-Expr::skipRValueSubobjectAdjustments(
-                     SmallVectorImpl<SubobjectAdjustment> &Adjustments) const  {
+const Expr *Expr::skipRValueSubobjectAdjustments(
+    SmallVectorImpl<const Expr *> &CommaLHSs,
+    SmallVectorImpl<SubobjectAdjustment> &Adjustments) const {
   const Expr *E = this;
   while (true) {
     E = E->IgnoreParens();
@@ -88,6 +88,11 @@ Expr::skipRValueSubobjectAdjustments(
         const MemberPointerType *MPT =
           BO->getRHS()->getType()->getAs<MemberPointerType>();
         Adjustments.push_back(SubobjectAdjustment(MPT, BO->getRHS()));
+        continue;
+      } else if (BO->getOpcode() == BO_Comma) {
+        CommaLHSs.push_back(BO->getLHS());
+        E = BO->getRHS();
+        continue;
       }
     }
 

diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp
index 8ba5a1ce5223a68f7431203b588b7a353bd2a754..28eae50832afc30a1c7e86b1ee4746e04b7c42e3 100644 (file)
--- a/lib/CodeGen/CGExpr.cpp
+++ b/lib/CodeGen/CGExpr.cpp
@@ -295,8 +295,13 @@ EmitExprForReferenceBinding(CodeGenFunction &CGF, const Expr *E,
     return ReferenceTemporary;
   }
 
+  SmallVector<const Expr *, 2> CommaLHSs;
   SmallVector<SubobjectAdjustment, 2> Adjustments;
-  E = E->skipRValueSubobjectAdjustments(Adjustments);
+  E = E->skipRValueSubobjectAdjustments(CommaLHSs, Adjustments);
+
+  for (unsigned I = 0, N = CommaLHSs.size(); I != N; ++I)
+    CGF.EmitIgnoredExpr(CommaLHSs[I]);
+
   if (const OpaqueValueExpr *opaque = dyn_cast<OpaqueValueExpr>(E))
     if (opaque->getType()->isRecordType())
       return CGF.EmitOpaqueValueLValue(opaque).getAddress();
@@ -332,6 +337,10 @@ EmitExprForReferenceBinding(CodeGenFunction &CGF, const Expr *E,
 
   RValue RV = CGF.EmitAnyExpr(E, AggSlot);
 
+  // FIXME: This is wrong. We need to register the destructor for the temporary
+  // now, *before* we perform the adjustments, because in the case of a
+  // pointer-to-member adjustment, the adjustment might throw.
+
   // Check if need to perform derived-to-base casts and/or field accesses, to
   // get from the temporary object we created (and, potentially, for which we
   // extended the lifetime) to the subobject we're binding the reference to.

diff --git a/lib/Sema/JumpDiagnostics.cpp b/lib/Sema/JumpDiagnostics.cpp
index 5f92cfffc6b02f041671f99b69814965eae496c9..8e446f8b9bff77df6c0e3f7de73fbaa8f7dd97f5 100644 (file)
--- a/lib/Sema/JumpDiagnostics.cpp
+++ b/lib/Sema/JumpDiagnostics.cpp
@@ -172,14 +172,19 @@ static ScopePair GetDiagForGotoScopeDecl(ASTContext &Context, const Decl *D) {
       if (EWC)
         Init = EWC->getSubExpr();
 
+      // FIXME: Why are we looking through reference initialization?
+      //        This causes us to incorrectly accept invalid code such as:
+      //   struct S { int n; };
+      //   int f() { goto x; S &&s = S(); x: return x.n; }
       const MaterializeTemporaryExpr *M = NULL;
       Init = Init->findMaterializedTemporary(M);
 
+      SmallVector<const Expr *, 2> CommaLHSs;
       SmallVector<SubobjectAdjustment, 2> Adjustments;
-      Init = Init->skipRValueSubobjectAdjustments(Adjustments);
+      Init = Init->skipRValueSubobjectAdjustments(CommaLHSs, Adjustments);
 
       QualType QT = Init->getType();
-      if (QT.isNull())
+      if (QT.isNull() || !CommaLHSs.empty())
         return ScopePair(diag::note_protected_by_variable_init, 0);
 
       const Type *T = QT.getTypePtr();

diff --git a/test/CodeGenCXX/temporaries.cpp b/test/CodeGenCXX/temporaries.cpp
index a369c2e36976eb5e411e78260b32aefb61ab5770..ea87e7f8e384bdeb89470d97abee4adf9f9be7f9 100644 (file)
--- a/test/CodeGenCXX/temporaries.cpp
+++ b/test/CodeGenCXX/temporaries.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-apple-darwin9 | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-apple-darwin9 -std=c++11 | FileCheck %s
 struct A {
   A();
   ~A();
@@ -558,3 +558,49 @@ namespace AssignmentOp {
   // CHECK: call {{.*}} @_ZN12AssignmentOp1BaSERKS
   // CHECK: call {{.*}} @_ZN12AssignmentOp1AD1Ev(
 }
+
+namespace BindToSubobject {
+  struct A {
+    A();
+    ~A();
+    int a;
+  };
+
+  void f(), g();
+
+  // CHECK: call void @_ZN15BindToSubobject1AC1Ev({{.*}} @_ZGRN15BindToSubobject1aE)
+  // CHECK: call i32 @__cxa_atexit({{.*}} bitcast ({{.*}} @_ZN15BindToSubobject1AD1Ev to void (i8*)*), i8* bitcast ({{.*}} @_ZGRN15BindToSubobject1aE to i8*), i8* @__dso_handle)
+  // CHECK: store i32* getelementptr inbounds ({{.*}} @_ZGRN15BindToSubobject1aE, i32 0, i32 0), i32** @_ZN15BindToSubobject1aE, align 8
+  int &&a = A().a;
+
+  // CHECK: call void @_ZN15BindToSubobject1fEv()
+  // CHECK: call void @_ZN15BindToSubobject1AC1Ev({{.*}} @_ZGRN15BindToSubobject1bE)
+  // CHECK: call i32 @__cxa_atexit({{.*}} bitcast ({{.*}} @_ZN15BindToSubobject1AD1Ev to void (i8*)*), i8* bitcast ({{.*}} @_ZGRN15BindToSubobject1bE to i8*), i8* @__dso_handle)
+  // CHECK: store i32* getelementptr inbounds ({{.*}} @_ZGRN15BindToSubobject1bE, i32 0, i32 0), i32** @_ZN15BindToSubobject1bE, align 8
+  int &&b = (f(), A().a);
+
+  int A::*h();
+
+  // CHECK: call void @_ZN15BindToSubobject1fEv()
+  // CHECK: call void @_ZN15BindToSubobject1gEv()
+  // CHECK: call void @_ZN15BindToSubobject1AC1Ev({{.*}} @_ZGRN15BindToSubobject1cE)
+  // FIXME: This is wrong. We should emit the call to __cxa_atexit prior to
+  // calling h(), in case h() throws.
+  // CHECK: call {{.*}} @_ZN15BindToSubobject1hE
+  // CHECK: getelementptr
+  // CHECK: call i32 @__cxa_atexit({{.*}} bitcast ({{.*}} @_ZN15BindToSubobject1AD1Ev to void (i8*)*), i8* bitcast ({{.*}} @_ZGRN15BindToSubobject1cE to i8*), i8* @__dso_handle)
+  // CHECK: store i32* {{.*}}, i32** @_ZN15BindToSubobject1cE, align 8
+  int &&c = (f(), (g(), A().*h()));
+
+  struct B {
+    int padding;
+    A a;
+  };
+
+  // CHECK: call void @_ZN15BindToSubobject1BC1Ev({{.*}} @_ZGRN15BindToSubobject1dE)
+  // CHECK: call {{.*}} @_ZN15BindToSubobject1hE
+  // CHECK: getelementptr {{.*}} getelementptr
+  // CHECK: call i32 @__cxa_atexit({{.*}} bitcast ({{.*}} @_ZN15BindToSubobject1BD1Ev to void (i8*)*), i8* bitcast ({{.*}} @_ZGRN15BindToSubobject1dE to i8*), i8* @__dso_handle)
+  // CHECK: store i32* {{.*}}, i32** @_ZN15BindToSubobject1dE, align 8
+  int &&d = (B().a).*h();
+}