[llvm-branch-commits] [cfe-branch] r168324 - in /cfe/branches/release_32: ./ lib/AST/DeclCXX.cpp lib/Sema/SemaPseudoObject.cpp test/CodeGenCXX/cxx11-special-members.cpp test/CodeGenObjCXX/property-objects.mm test/SemaObjCXX/properties.mm

Pawel Wodnicki pawel at 32bitmicro.com
Mon Nov 19 13:01:40 PST 2012


Author: pawel
Date: Mon Nov 19 15:01:40 2012
New Revision: 168324

URL: http://llvm.org/viewvc/llvm-project?rev=168324&view=rev
Log:
Merging r167884,r167920 from trunk into 3.2 release branch 

r167884

Don't try to save the assigned value in a Objective-C property assignment
if the type of the value is a non-trivial class type.  Fixes PR14318.

(There's a minor ObjC++ language change here: given that we can't save the
value, the type of the assignment expression is void in such cases.)

r167920

PR14279: Work around this major miscompilation by treating move operations as
non-trivial if they would not call a move operation, even if they would in fact
call a trivial copy operation. A proper fix is to follow, but this small
directed fix is intended for porting to the 3.2 release branch.

Added:
    cfe/branches/release_32/test/CodeGenCXX/cxx11-special-members.cpp
      - copied unchanged from r167920, cfe/trunk/test/CodeGenCXX/cxx11-special-members.cpp
Modified:
    cfe/branches/release_32/   (props changed)
    cfe/branches/release_32/lib/AST/DeclCXX.cpp
    cfe/branches/release_32/lib/Sema/SemaPseudoObject.cpp
    cfe/branches/release_32/test/CodeGenObjCXX/property-objects.mm
    cfe/branches/release_32/test/SemaObjCXX/properties.mm

Propchange: cfe/branches/release_32/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Mon Nov 19 15:01:40 2012
@@ -1,3 +1,3 @@
 /cfe/branches/type-system-rewrite:134693-134817
-/cfe/trunk:167749,167762,167780,167788,167790,167813-167814,167868
+/cfe/trunk:167749,167762,167780,167788,167790,167813-167814,167868,167884,167920
 /cfe/trunk/test/SemaTemplate:126920

Modified: cfe/branches/release_32/lib/AST/DeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_32/lib/AST/DeclCXX.cpp?rev=168324&r1=168323&r2=168324&view=diff
==============================================================================
--- cfe/branches/release_32/lib/AST/DeclCXX.cpp (original)
+++ cfe/branches/release_32/lib/AST/DeclCXX.cpp Mon Nov 19 15:01:40 2012
@@ -240,10 +240,13 @@
       //    -- the constructor selected to copy/move each direct base class
       //       subobject is trivial, and
       // FIXME: C++0x: We need to only consider the selected constructor
-      // instead of all of them.
+      // instead of all of them. For now, we treat a move constructor as being
+      // non-trivial if it calls anything other than a trivial move constructor.
       if (!BaseClassDecl->hasTrivialCopyConstructor())
         data().HasTrivialCopyConstructor = false;
-      if (!BaseClassDecl->hasTrivialMoveConstructor())
+      if (!BaseClassDecl->hasTrivialMoveConstructor() ||
+          !(BaseClassDecl->hasDeclaredMoveConstructor() ||
+            BaseClassDecl->needsImplicitMoveConstructor()))
         data().HasTrivialMoveConstructor = false;
 
       // C++0x [class.copy]p27:
@@ -255,7 +258,9 @@
       // of all of them.
       if (!BaseClassDecl->hasTrivialCopyAssignment())
         data().HasTrivialCopyAssignment = false;
-      if (!BaseClassDecl->hasTrivialMoveAssignment())
+      if (!BaseClassDecl->hasTrivialMoveAssignment() ||
+          !(BaseClassDecl->hasDeclaredMoveAssignment() ||
+            BaseClassDecl->needsImplicitMoveAssignment()))
         data().HasTrivialMoveAssignment = false;
 
       // C++11 [class.ctor]p6:
@@ -830,7 +835,9 @@
         // FIXME: C++0x: We don't correctly model 'selected' constructors.
         if (!FieldRec->hasTrivialCopyConstructor())
           data().HasTrivialCopyConstructor = false;
-        if (!FieldRec->hasTrivialMoveConstructor())
+        if (!FieldRec->hasTrivialMoveConstructor() ||
+            !(FieldRec->hasDeclaredMoveConstructor() ||
+              FieldRec->needsImplicitMoveConstructor()))
           data().HasTrivialMoveConstructor = false;
 
         // C++0x [class.copy]p27:
@@ -842,7 +849,9 @@
         // FIXME: C++0x: We don't correctly model 'selected' operators.
         if (!FieldRec->hasTrivialCopyAssignment())
           data().HasTrivialCopyAssignment = false;
-        if (!FieldRec->hasTrivialMoveAssignment())
+        if (!FieldRec->hasTrivialMoveAssignment() ||
+            !(FieldRec->hasDeclaredMoveAssignment() ||
+              FieldRec->needsImplicitMoveAssignment()))
           data().HasTrivialMoveAssignment = false;
 
         if (!FieldRec->hasTrivialDestructor())

Modified: cfe/branches/release_32/lib/Sema/SemaPseudoObject.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_32/lib/Sema/SemaPseudoObject.cpp?rev=168324&r1=168323&r2=168324&view=diff
==============================================================================
--- cfe/branches/release_32/lib/Sema/SemaPseudoObject.cpp (original)
+++ cfe/branches/release_32/lib/Sema/SemaPseudoObject.cpp Mon Nov 19 15:01:40 2012
@@ -198,7 +198,14 @@
     }
 
     /// Return true if assignments have a non-void result.
-    virtual bool assignmentsHaveResult() { return true; }
+    bool CanCaptureValueOfType(QualType ty) {
+      assert(!ty->isIncompleteType());
+      assert(!ty->isDependentType());
+
+      if (const CXXRecordDecl *ClassDecl = ty->getAsCXXRecordDecl())
+        return ClassDecl->isTriviallyCopyable();
+      return true;
+    }
 
     virtual Expr *rebuildAndCaptureObject(Expr *) = 0;
     virtual ExprResult buildGet() = 0;
@@ -380,7 +387,7 @@
 
   // The result of the assignment, if not void, is the value set into
   // the l-value.
-  result = buildSet(result.take(), opcLoc, assignmentsHaveResult());
+  result = buildSet(result.take(), opcLoc, /*captureSetValueAsResult*/ true);
   if (result.isInvalid()) return ExprError();
   addSemanticExpr(result.take());
 
@@ -404,7 +411,7 @@
   QualType resultType = result.get()->getType();
 
   // That's the postfix result.
-  if (UnaryOperator::isPostfix(opcode) && assignmentsHaveResult()) {
+  if (UnaryOperator::isPostfix(opcode) && CanCaptureValueOfType(resultType)) {
     result = capture(result.take());
     setResultToLastSemantic();
   }
@@ -423,8 +430,7 @@
 
   // Store that back into the result.  The value stored is the result
   // of a prefix operation.
-  result = buildSet(result.take(), opcLoc,
-             UnaryOperator::isPrefix(opcode) && assignmentsHaveResult());
+  result = buildSet(result.take(), opcLoc, UnaryOperator::isPrefix(opcode));
   if (result.isInvalid()) return ExprError();
   addSemanticExpr(result.take());
 
@@ -696,7 +702,8 @@
     ObjCMessageExpr *msgExpr =
       cast<ObjCMessageExpr>(msg.get()->IgnoreImplicit());
     Expr *arg = msgExpr->getArg(0);
-    msgExpr->setArg(0, captureValueAsResult(arg));
+    if (CanCaptureValueOfType(arg->getType()))
+      msgExpr->setArg(0, captureValueAsResult(arg));
   }
 
   return msg;
@@ -1312,7 +1319,8 @@
     ObjCMessageExpr *msgExpr =
       cast<ObjCMessageExpr>(msg.get()->IgnoreImplicit());
     Expr *arg = msgExpr->getArg(0);
-    msgExpr->setArg(0, captureValueAsResult(arg));
+    if (CanCaptureValueOfType(arg->getType()))
+      msgExpr->setArg(0, captureValueAsResult(arg));
   }
   
   return msg;

Modified: cfe/branches/release_32/test/CodeGenObjCXX/property-objects.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_32/test/CodeGenObjCXX/property-objects.mm?rev=168324&r1=168323&r2=168324&view=diff
==============================================================================
--- cfe/branches/release_32/test/CodeGenObjCXX/property-objects.mm (original)
+++ cfe/branches/release_32/test/CodeGenObjCXX/property-objects.mm Mon Nov 19 15:01:40 2012
@@ -1,8 +1,4 @@
 // RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -o - | FileCheck %s
-// CHECK-NOT: callq	_objc_msgSend_stret
-// CHECK: call void @_ZN1SC1ERKS_
-// CHECK: call %class.S* @_ZN1SaSERKS_
-// CHECK: call %struct.CGRect* @_ZN6CGRectaSERKS_
 
 class S {
 public:
@@ -19,13 +15,14 @@
   S position;
   CGRect bounds;
 }
+
 @property(assign, nonatomic) S position;
 @property CGRect bounds;
 @property CGRect frame;
 - (void)setFrame:(CGRect)frameRect;
 - (CGRect)frame;
 - (void) initWithOwner;
-- (struct CGRect)extent;
+- (CGRect)extent;
 - (void)dealloc;
 @end
 
@@ -33,6 +30,11 @@
 @synthesize position;
 @synthesize bounds;
 @synthesize frame;
+
+// CHECK: define internal void @"\01-[I setPosition:]"
+// CHECK: call %class.S* @_ZN1SaSERKS_
+// CHECK-NEXT: ret void
+
 - (void)setFrame:(CGRect)frameRect {}
 - (CGRect)frame {return bounds;}
 
@@ -42,14 +44,20 @@
   labelLayerFrame = self.bounds;
   _labelLayer.frame = labelLayerFrame;
 }
+
 // rdar://8366604
 - (void)dealloc
   {
       CGRect cgrect = self.extent;
   }
 - (struct CGRect)extent {return bounds;}
+
 @end
 
+// CHECK: define i32 @main
+// CHECK: call void @_ZN1SC1ERKS_(%class.S* [[AGGTMP:%[a-zA-Z0-9\.]+]], %class.S* {{%[a-zA-Z0-9\.]+}})
+// CHECK: call void bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to void (i8*, i8*, %class.S*)*)(i8* {{%[a-zA-Z0-9\.]+}}, i8* {{%[a-zA-Z0-9\.]+}}, %class.S* [[AGGTMP]])
+// CHECK-NEXT: ret i32 0
 int main() {
   I *i;
   S s1;
@@ -59,7 +67,9 @@
 
 // rdar://8379892
 // CHECK: define void @_Z1fP1A
-// CHECK: @objc_msgSend to void
+// CHECK: call void @_ZN1XC1Ev(%struct.X* [[LVTEMP:%[a-zA-Z0-9\.]+]])
+// CHECK: call void @_ZN1XC1ERKS_(%struct.X* [[AGGTMP:%[a-zA-Z0-9\.]+]], %struct.X* [[LVTEMP]])
+// CHECK: call void bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to void (i8*, i8*, %struct.X*)*)({{.*}} %struct.X* [[AGGTMP]])
 struct X {
   X();
   X(const X&);

Modified: cfe/branches/release_32/test/SemaObjCXX/properties.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_32/test/SemaObjCXX/properties.mm?rev=168324&r1=168323&r2=168324&view=diff
==============================================================================
--- cfe/branches/release_32/test/SemaObjCXX/properties.mm (original)
+++ cfe/branches/release_32/test/SemaObjCXX/properties.mm Mon Nov 19 15:01:40 2012
@@ -106,3 +106,26 @@
   delete ptr.implicit_struct_property;
   delete ptr.explicit_struct_property;
 }
+
+// Make sure the returned value from property assignment is void,
+// because there isn't any other viable way to handle it for
+// non-trivial classes.
+class NonTrivial1 {
+public:
+	~NonTrivial1();
+};
+class NonTrivial2 {
+public:
+	NonTrivial2();
+	NonTrivial2(const NonTrivial2&);
+};
+ at interface TestNonTrivial
+ at property(assign, nonatomic) NonTrivial1 p1;
+ at property(assign, nonatomic) NonTrivial2 p2;
+ at end
+TestNonTrivial *TestNonTrivialObj;
+
+extern void* VoidType;
+extern decltype(TestNonTrivialObj.p1 = NonTrivial1())* VoidType;
+extern decltype(TestNonTrivialObj.p2 = NonTrivial2())* VoidType;
+





More information about the llvm-branch-commits mailing list