[cfe-commits] r104219 - in /cfe/trunk: lib/CodeGen/CGExpr.cpp test/CodeGenCXX/references.cpp

Douglas Gregor dgregor at apple.com
Thu May 20 01:36:29 PDT 2010


Author: dgregor
Date: Thu May 20 03:36:28 2010
New Revision: 104219

URL: http://llvm.org/viewvc/llvm-project?rev=104219&view=rev
Log:
Rework our handling of binding a reference to a temporary
subobject. Previously, we could only properly bind to a base class
subobject while extending the lifetime of the complete object (of a
derived type); for non-static data member subobjects, we could memcpy
(!) the result and bind to that, which is rather broken.

Now, we pull apart the expression that we're binding to, to figure out
which subobject we're accessing, then construct the temporary object
(adding a destruction if needed) and, finally, dig out the subobject
we actually meant to access.

This fixes yet another instance where we were memcpy'ing rather than
doing the right thing. However, note the FIXME in references.cpp:
there's more work to be done for binding to subobjects, since the AST
is incorrectly modeling some member accesses in base classes as
lvalues when they are really rvalues.

Modified:
    cfe/trunk/lib/CodeGen/CGExpr.cpp
    cfe/trunk/test/CodeGenCXX/references.cpp

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=104219&r1=104218&r2=104219&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Thu May 20 03:36:28 2010
@@ -135,6 +135,39 @@
   }
 }
 
+/// \brief An adjustment to be made to the temporary created when emitting a
+/// reference binding, which accesses a particular subobject of that temporary.
+struct SubobjectAdjustment {
+  enum { DerivedToBaseAdjustment, FieldAdjustment } Kind;
+  
+  union {
+    struct {
+      const CXXBaseSpecifierArray *BasePath;
+      const CXXRecordDecl *DerivedClass;
+    } DerivedToBase;
+    
+    struct {
+      FieldDecl *Field;
+      unsigned CVRQualifiers;
+    } Field;
+  };
+  
+  SubobjectAdjustment(const CXXBaseSpecifierArray *BasePath, 
+                      const CXXRecordDecl *DerivedClass)
+    : Kind(DerivedToBaseAdjustment) 
+  {
+    DerivedToBase.BasePath = BasePath;
+    DerivedToBase.DerivedClass = DerivedClass;
+  }
+  
+  SubobjectAdjustment(FieldDecl *Field, unsigned CVRQualifiers)
+    : Kind(FieldAdjustment) 
+  { 
+    this->Field.Field = Field;
+    this->Field.CVRQualifiers = CVRQualifiers;
+  }
+};
+
 RValue CodeGenFunction::EmitReferenceBindingToExpr(const Expr* E,
                                                    bool IsInitializer) {
   bool ShouldDestroyTemporaries = false;
@@ -174,19 +207,34 @@
         PopCXXTemporary();
     }      
   } else {
-    const CXXBaseSpecifierArray *BasePath = 0;
-    const CXXRecordDecl *DerivedClassDecl = 0;
+    QualType ResultTy = E->getType();
     
-    if (const CastExpr *CE = 
-          dyn_cast<CastExpr>(E->IgnoreParenNoopCasts(getContext()))) {
-      if (CE->getCastKind() == CastExpr::CK_DerivedToBase) {
-        E = CE->getSubExpr();
-
-        BasePath = &CE->getBasePath();
-        DerivedClassDecl = 
-          cast<CXXRecordDecl>(E->getType()->getAs<RecordType>()->getDecl());
+    llvm::SmallVector<SubobjectAdjustment, 2> Adjustments;
+    do {
+      if (const CastExpr *CE 
+                 = dyn_cast<CastExpr>(E->IgnoreParenNoopCasts(getContext()))) {
+        if (CE->getCastKind() == CastExpr::CK_DerivedToBase) {
+          E = CE->getSubExpr();
+          CXXRecordDecl *Derived 
+            = cast<CXXRecordDecl>(E->getType()->getAs<RecordType>()->getDecl());
+          Adjustments.push_back(SubobjectAdjustment(&CE->getBasePath(), 
+                                                    Derived));
+          continue;
+        }
+      } else if (const MemberExpr *ME
+                      = dyn_cast<MemberExpr>(
+                                       E->IgnoreParenNoopCasts(getContext()))) {
+        if (ME->getBase()->isLvalue(getContext()) != Expr::LV_Valid &&
+            ME->getBase()->getType()->isRecordType()) {
+          if (FieldDecl *Field = dyn_cast<FieldDecl>(ME->getMemberDecl())) {
+            E = ME->getBase();
+            Adjustments.push_back(SubobjectAdjustment(Field,
+                                              E->getType().getCVRQualifiers()));
+            continue;
+          }
+        }
       }
-    }
+    } while (false);
       
     Val = EmitAnyExprToTemp(E, /*IsAggLocVolatile=*/false,
                             IsInitializer);
@@ -225,13 +273,47 @@
       }
     }
     
-    // Check if need to perform the derived-to-base cast.
-    if (BasePath) {
-      llvm::Value *Derived = Val.getAggregateAddr();
-      llvm::Value *Base = 
-        GetAddressOfBaseClass(Derived, DerivedClassDecl, *BasePath, 
-                              /*NullCheckValue=*/false);
-      return RValue::get(Base);
+    // 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.
+    if (!Adjustments.empty()) {
+      llvm::Value *Object = Val.getAggregateAddr();
+      for (unsigned I = Adjustments.size(); I != 0; --I) {
+        SubobjectAdjustment &Adjustment = Adjustments[I-1];
+        switch (Adjustment.Kind) {
+        case SubobjectAdjustment::DerivedToBaseAdjustment:
+          Object = GetAddressOfBaseClass(Object, 
+                                         Adjustment.DerivedToBase.DerivedClass, 
+                                         *Adjustment.DerivedToBase.BasePath, 
+                                         /*NullCheckValue=*/false);
+          break;
+            
+        case SubobjectAdjustment::FieldAdjustment: {
+          unsigned CVR = Adjustment.Field.CVRQualifiers;
+          LValue LV = EmitLValueForField(Object, Adjustment.Field.Field, CVR);
+          if (LV.isSimple()) {
+            Object = LV.getAddress();
+            break;
+          }
+          
+          // For non-simple lvalues, we actually have to create a copy of
+          // the object we're binding to.
+          QualType T = Adjustment.Field.Field->getType().getNonReferenceType()
+                                                        .getUnqualifiedType();
+          Object = CreateTempAlloca(ConvertType(T), "lv");
+          EmitStoreThroughLValue(EmitLoadOfLValue(LV, T), 
+                                 LValue::MakeAddr(Object, 
+                                                  Qualifiers::fromCVRMask(CVR)),
+                                 T);
+          break;
+        }
+        }
+      }
+      
+      const llvm::Type *ResultPtrTy
+        = llvm::PointerType::get(ConvertType(ResultTy), 0);
+      Object = Builder.CreateBitCast(Object, ResultPtrTy, "temp");
+      return RValue::get(Object);
     }
   }
 

Modified: cfe/trunk/test/CodeGenCXX/references.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/references.cpp?rev=104219&r1=104218&r2=104219&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/references.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/references.cpp Thu May 20 03:36:28 2010
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -verify -emit-llvm -o - %s | FileCheck %s
 void t1() {
   extern int& a;
   int b = a; 
@@ -168,3 +168,56 @@
     const int* const args_array[] = { &arg };
   }
 }
+
+// Bind to subobjects while extending the life of the complete object.
+namespace N2 {
+  class X {
+  public:
+    X(const X&);
+    X &operator=(const X&);
+    ~X();
+  };
+
+  struct P {
+    X first;
+  };
+
+  P getP();
+
+  // CHECK: define void @_ZN2N21fEi
+  // CHECK: call void @_ZN2N24getPEv
+  // CHECK: getelementptr inbounds
+  // CHECK: store i32 17
+  // CHECK: call void @_ZN2N21PD1Ev
+  void f(int i) {
+    const X& xr = getP().first;
+    i = 17;
+  }
+
+  struct SpaceWaster {
+    int i, j;
+  };
+
+  struct ReallyHasX {
+    X x;
+  };
+
+  struct HasX : ReallyHasX { };
+
+  struct HasXContainer {
+    HasX has;
+  };
+
+  struct Y : SpaceWaster, HasXContainer { };
+  struct Z : SpaceWaster, Y { };
+
+  Z getZ();
+
+  // CHECK: define void @_ZN2N21gEi
+  // CHECK: call void @_ZN2N24getZEv
+  // FIXME: Not treated as an lvalue!
+  void g(int i) {
+    const X &xr = getZ().has.x;
+    i = 19;    
+  }
+}





More information about the cfe-commits mailing list