r218520 - Fix an assertion failure trying to emit a trivial destructor in ObjC++

[Ben Langmuir]

Author: benlangmuir
Date: Fri Sep 26 10:27:29 2014
New Revision: 218520

URL: http://llvm.org/viewvc/llvm-project?rev=218520&view=rev
Log:
Fix an assertion failure trying to emit a trivial destructor in ObjC++

If a base class declares a destructor, we will add the implicit
destructor for the subclass in
ActOnFields -> AddImplicitlyDeclaredMembersToClass

But in Objective C++, we did not compute whether we have a trivial
destructor until after that in
CXXRecordDecl::completeDefinition()

This was leading to a mismatch between the class, which thought it had
no trivial destructor, and the CXXDestructorDecl, which considered
itself trivial. It turns out the reason we delayed setting this until
completeDefinition() was for a warning that has since been removed as
part of -Warc-abi, so we just do it eagerly now.

Added:
    cfe/trunk/test/CodeGenObjCXX/destroy.mm
Modified:
    cfe/trunk/lib/AST/DeclCXX.cpp

Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=218520&r1=218519&r2=218520&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Fri Sep 26 10:27:29 2014
@@ -677,17 +677,24 @@ void CXXRecordDecl::addedMember(Decl *D)
     //
     // Automatic Reference Counting: the presence of a member of Objective-C pointer type
     // that does not explicitly have no lifetime makes the class a non-POD.
-    // However, we delay setting PlainOldData to false in this case so that
-    // Sema has a chance to diagnostic causes where the same class will be
-    // non-POD with Automatic Reference Counting but a POD without ARC.
-    // In this case, the class will become a non-POD class when we complete
-    // the definition.
     ASTContext &Context = getASTContext();
     QualType T = Context.getBaseElementType(Field->getType());
     if (T->isObjCRetainableType() || T.isObjCGCStrong()) {
-      if (!Context.getLangOpts().ObjCAutoRefCount ||
-          T.getObjCLifetime() != Qualifiers::OCL_ExplicitNone)
+      if (!Context.getLangOpts().ObjCAutoRefCount) {
         setHasObjectMember(true);
+      } else if (T.getObjCLifetime() != Qualifiers::OCL_ExplicitNone) {
+        // Objective-C Automatic Reference Counting:
+        //   If a class has a non-static data member of Objective-C pointer
+        //   type (or array thereof), it is a non-POD type and its
+        //   default constructor (if any), copy constructor, move constructor,
+        //   copy assignment operator, move assignment operator, and destructor are
+        //   non-trivial.
+        setHasObjectMember(true);
+        struct DefinitionData &Data = data();
+        Data.PlainOldData = false;
+        Data.HasTrivialSpecialMembers = 0;
+        Data.HasIrrelevantDestructor = false;
+      }
     } else if (!T.isCXX98PODType(Context))
       data().PlainOldData = false;
     
@@ -1277,19 +1284,6 @@ void CXXRecordDecl::completeDefinition()
 void CXXRecordDecl::completeDefinition(CXXFinalOverriderMap *FinalOverriders) {
   RecordDecl::completeDefinition();
   
-  if (hasObjectMember() && getASTContext().getLangOpts().ObjCAutoRefCount) {
-    // Objective-C Automatic Reference Counting:
-    //   If a class has a non-static data member of Objective-C pointer
-    //   type (or array thereof), it is a non-POD type and its
-    //   default constructor (if any), copy constructor, move constructor,
-    //   copy assignment operator, move assignment operator, and destructor are
-    //   non-trivial.
-    struct DefinitionData &Data = data();
-    Data.PlainOldData = false;
-    Data.HasTrivialSpecialMembers = 0;
-    Data.HasIrrelevantDestructor = false;
-  }
-  
   // If the class may be abstract (but hasn't been marked as such), check for
   // any pure final overriders.
   if (mayBeAbstract()) {

Added: cfe/trunk/test/CodeGenObjCXX/destroy.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/destroy.mm?rev=218520&view=auto
==============================================================================
--- cfe/trunk/test/CodeGenObjCXX/destroy.mm (added)
+++ cfe/trunk/test/CodeGenObjCXX/destroy.mm Fri Sep 26 10:27:29 2014
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -disable-llvm-optzns -o - %s | FileCheck %s
+// rdar://18249673
+
+ at class MyObject;
+struct base {
+  ~base() = default;
+};
+struct derived : public base {
+  MyObject *myobject;
+};
+
+void test1() {
+  derived d1;
+}
+// CHECK-LABEL: define void @_Z5test1v()
+// CHECK: call void @_ZN7derivedC1Ev
+// CHECK: call void @_ZN7derivedD1Ev
+
+void test2() {
+  derived *d2 = new derived;
+  delete d2;
+}
+// CHECK-LABEL: define void @_Z5test2v()
+// CHECK:   call void @_ZN7derivedC1Ev
+// CHECK:   call void @_ZN7derivedD1Ev
+
+template <typename T>
+struct tderived : public base {
+  MyObject *myobject;
+};
+void test3() {
+  tderived<int> d1;
+}
+// CHECK-LABEL: define void @_Z5test3v()
+// CHECK: call void @_ZN8tderivedIiEC1Ev
+// CHECK: call void @_ZN8tderivedIiED1Ev
+
+void test4() {
+  tderived<int> *d2 = new tderived<int>;
+  delete d2;
+}
+// CHECK-LABEL: define void @_Z5test4v()
+// CHECK: call void @_ZN8tderivedIiEC1Ev
+// CHECK: call void @_ZN8tderivedIiED1Ev
+
+// CHECK-LABEL: define linkonce_odr void @_ZN8tderivedIiED2Ev
+// CHECK: call void @objc_storeStrong(i8** {{.*}}, i8* null)
+
+// CHECK-LABEL: define linkonce_odr void @_ZN7derivedD2Ev
+// CHECK: call void @objc_storeStrong(i8** {{.*}}, i8* null)