Consolidate copy constructor deletion into ShouldDeleteSpecialMember.

llvm-svn: 141645

clang/include/clang/Sema/Sema.h

@@ -2834,10 +2834,6 @@ public:
   /// definition when it is defaulted.
   bool ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM);
 
-  /// \brief Determine if a defaulted copy constructor ought to be
-  /// deleted.
-  bool ShouldDeleteCopyConstructor(CXXConstructorDecl *CD);
-
   /// \brief Determine if a defaulted copy assignment operator ought to be
   /// deleted.
   bool ShouldDeleteCopyAssignmentOperator(CXXMethodDecl *MD);

clang/lib/Sema/SemaDeclCXX.cpp

@@ -3812,7 +3812,7 @@ void Sema::CheckExplicitlyDefaultedCopyConstructor(CXXConstructorDecl *CD) {
     return;
   }
 
-  if (ShouldDeleteCopyConstructor(CD)) {
+  if (ShouldDeleteSpecialMember(CD, CXXCopyConstructor)) {
     if (First) {
       CD->setDeletedAsWritten();
     } else {
@@ -4101,6 +4101,7 @@ void Sema::CheckExplicitlyDefaultedDestructor(CXXDestructorDecl *DD) {
 
 /// This function implements the following C++0x paragraphs:
 ///  - [class.ctor]/5
+///  - [class.copy]/11
 bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) {
   assert(!MD->isInvalidDecl());
   CXXRecordDecl *RD = MD->getParent();
@@ -4119,13 +4120,17 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) {
   case CXXDefaultConstructor:
     IsConstructor = true;
     break;
+  case CXXCopyConstructor:
+    IsConstructor = true;
+    ConstArg = MD->getParamDecl(0)->getType().isConstQualified();
+    break;
   default:
     llvm_unreachable("function only currently implemented for default ctors");
   }
 
   SourceLocation Loc = MD->getLocation();
 
-  // Do access control from the constructor
+  // Do access control from the special member function
   ContextRAII MethodContext(*this, MD);
 
   bool AllConst = true;
@@ -4159,7 +4164,8 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) {
     }
 
     // Finding the corresponding member in the base should lead to a
-    // unique, accessible, non-deleted function.
+    // unique, accessible, non-deleted function. If we are doing
+    // a destructor, we have already checked this case.
     if (CSM != CXXDestructor) {
       SpecialMemberOverloadResult *SMOR =
         LookupSpecialMember(BaseDecl, CSM, ConstArg, false, IsMove, false,
@@ -4228,20 +4234,14 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) {
 
       if (IsUnion && !FieldType.isConstQualified())
         AllConst = false;
+    // For a copy constructor, data members must not be of rvalue reference
+    // type.
+    } else if (CSM == CXXCopyConstructor) {
+      if (FieldType->isRValueReferenceType())
+        return true;
     }
 
     if (FieldRecord) {
-      // Unless we're doing assignment, the field's destructor must be
-      // accessible and not deleted.
-      if (!IsAssignment) {
-        CXXDestructorDecl *FieldDtor = LookupDestructor(FieldRecord);
-        if (FieldDtor->isDeleted())
-          return true;
-        if (CheckDestructorAccess(Loc, FieldDtor, PDiag()) !=
-            AR_accessible)
-          return true;
-      }
-
       // For a default constructor, a const member must have a user-provided
       // default constructor or else be explicitly initialized.
       if (CSM == CXXDefaultConstructor && FieldType.isConstQualified() &&
@@ -4249,9 +4249,8 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) {
           !FieldRecord->hasUserProvidedDefaultConstructor())
         return true;
  
-      // For a default constructor, additional restrictions exist on the
+      // Some additional restrictions exist on the variant members.
-      // variant members.
+      if (!IsUnion && FieldRecord->isUnion() &&
-      if (CSM == CXXDefaultConstructor && !IsUnion && FieldRecord->isUnion() &&
           FieldRecord->isAnonymousStructOrUnion()) {
         // We're okay to reuse AllConst here since we only care about the
         // value otherwise if we're in a union.
@@ -4267,12 +4266,49 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) {
           if (!UnionFieldType.isConstQualified())
             AllConst = false;
 
-          if (UnionFieldRecord &&
+          if (UnionFieldRecord) {
-              !UnionFieldRecord->hasTrivialDefaultConstructor())
+            // FIXME: Checking for accessibility and validity of this
-            return true;
+            //        destructor is technically going beyond the
+            //        standard, but this is believed to be a defect.
+            if (!IsAssignment) {
+              CXXDestructorDecl *FieldDtor = LookupDestructor(UnionFieldRecord);
+              if (FieldDtor->isDeleted())
+                return true;
+              if (CheckDestructorAccess(Loc, FieldDtor, PDiag()) !=
+                  AR_accessible)
+                return true;
+              if (!FieldDtor->isTrivial())
+                return true;
+            }
+
+            if (CSM != CXXDestructor) {
+              SpecialMemberOverloadResult *SMOR =
+                LookupSpecialMember(UnionFieldRecord, CSM, ConstArg, false,
+                                    IsMove, false, false);
+              // FIXME: Checking for accessibility and validity of this
+              //        corresponding member is technically going beyond the
+              //        standard, but this is believed to be a defect.
+              if (!SMOR->hasSuccess())
+                return true;
+
+              CXXMethodDecl *FieldMember = SMOR->getMethod();
+              // A member of a union must have a trivial corresponding
+              // constructor.
+              if (!FieldMember->isTrivial())
+                return true;
+
+              if (IsConstructor) {
+                CXXConstructorDecl *FieldCtor = cast<CXXConstructorDecl>(FieldMember);
+                if (CheckConstructorAccess(Loc, FieldCtor, FieldCtor->getAccess(),
+                                           PDiag()) != AR_accessible)
+                return true;
+              }
+            }
+          }
         }
 
-        if (AllConst)
+        // At least one member in each anonymous union must be non-const
+        if (CSM == CXXDefaultConstructor && AllConst)
           return true;
 
         // Don't try to initialize the anonymous union
@@ -4280,6 +4316,17 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) {
         continue;
       }
 
+      // Unless we're doing assignment, the field's destructor must be
+      // accessible and not deleted.
+      if (!IsAssignment) {
+        CXXDestructorDecl *FieldDtor = LookupDestructor(FieldRecord);
+        if (FieldDtor->isDeleted())
+          return true;
+        if (CheckDestructorAccess(Loc, FieldDtor, PDiag()) !=
+            AR_accessible)
+          return true;
+      }
+
       // Check that the corresponding member of the field is accessible,
       // unique, and non-deleted. We don't do this if it has an explicit
       // initialization when default-constructing.
@@ -4322,165 +4369,6 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) {
   return false;
 }
 
-bool Sema::ShouldDeleteCopyConstructor(CXXConstructorDecl *CD) {
-  CXXRecordDecl *RD = CD->getParent();
-  assert(!RD->isDependentType() && "do deletion after instantiation");
-  if (!LangOpts.CPlusPlus0x || RD->isInvalidDecl())
-    return false;
-
-  SourceLocation Loc = CD->getLocation();
-
-  // Do access control from the constructor
-  ContextRAII CtorContext(*this, CD);
-
-  bool Union = RD->isUnion();
-
-  assert(!CD->getParamDecl(0)->getType()->getPointeeType().isNull() &&
-         "copy assignment arg has no pointee type");
-  unsigned ArgQuals =
-    CD->getParamDecl(0)->getType()->getPointeeType().isConstQualified() ?
-      Qualifiers::Const : 0;
-
-  // We do this because we should never actually use an anonymous
-  // union's constructor.
-  if (Union && RD->isAnonymousStructOrUnion())
-    return false;
-
-  // FIXME: We should put some diagnostic logic right into this function.
-
-  // C++0x [class.copy]/11
-  //    A defaulted [copy] constructor for class X is defined as delete if X has:
-
-  for (CXXRecordDecl::base_class_iterator BI = RD->bases_begin(),
-                                          BE = RD->bases_end();
-       BI != BE; ++BI) {
-    // We'll handle this one later
-    if (BI->isVirtual())
-      continue;
-
-    QualType BaseType = BI->getType();
-    CXXRecordDecl *BaseDecl = BaseType->getAsCXXRecordDecl();
-    assert(BaseDecl && "base isn't a CXXRecordDecl");
-
-    // -- any [direct base class] of a type with a destructor that is deleted or
-    //    inaccessible from the defaulted constructor
-    CXXDestructorDecl *BaseDtor = LookupDestructor(BaseDecl);
-    if (BaseDtor->isDeleted())
-      return true;
-    if (CheckDestructorAccess(Loc, BaseDtor, PDiag()) !=
-        AR_accessible)
-      return true;
-
-    // -- a [direct base class] B that cannot be [copied] because overload
-    //    resolution, as applied to B's [copy] constructor, results in an
-    //    ambiguity or a function that is deleted or inaccessible from the
-    //    defaulted constructor
-    CXXConstructorDecl *BaseCtor = LookupCopyingConstructor(BaseDecl, ArgQuals);
-    if (!BaseCtor || BaseCtor->isDeleted())
-      return true;
-    if (CheckConstructorAccess(Loc, BaseCtor, BaseCtor->getAccess(), PDiag()) !=
-        AR_accessible)
-      return true;
-  }
-
-  for (CXXRecordDecl::base_class_iterator BI = RD->vbases_begin(),
-                                          BE = RD->vbases_end();
-       BI != BE; ++BI) {
-    QualType BaseType = BI->getType();
-    CXXRecordDecl *BaseDecl = BaseType->getAsCXXRecordDecl();
-    assert(BaseDecl && "base isn't a CXXRecordDecl");
-
-    // -- any [virtual base class] of a type with a destructor that is deleted or
-    //    inaccessible from the defaulted constructor
-    CXXDestructorDecl *BaseDtor = LookupDestructor(BaseDecl);
-    if (BaseDtor->isDeleted())
-      return true;
-    if (CheckDestructorAccess(Loc, BaseDtor, PDiag()) !=
-        AR_accessible)
-      return true;
-
-    // -- a [virtual base class] B that cannot be [copied] because overload
-    //    resolution, as applied to B's [copy] constructor, results in an
-    //    ambiguity or a function that is deleted or inaccessible from the
-    //    defaulted constructor
-    CXXConstructorDecl *BaseCtor = LookupCopyingConstructor(BaseDecl, ArgQuals);
-    if (!BaseCtor || BaseCtor->isDeleted())
-      return true;
-    if (CheckConstructorAccess(Loc, BaseCtor, BaseCtor->getAccess(), PDiag()) !=
-        AR_accessible)
-      return true;
-  }
-
-  for (CXXRecordDecl::field_iterator FI = RD->field_begin(),
-                                     FE = RD->field_end();
-       FI != FE; ++FI) {
-    if (FI->isUnnamedBitfield())
-      continue;
-    
-    QualType FieldType = Context.getBaseElementType(FI->getType());
-    
-    // -- for a copy constructor, a non-static data member of rvalue reference
-    //    type
-    if (FieldType->isRValueReferenceType())
-      return true;
- 
-    CXXRecordDecl *FieldRecord = FieldType->getAsCXXRecordDecl();
-
-    if (FieldRecord) {
-      // This is an anonymous union
-      if (FieldRecord->isUnion() && FieldRecord->isAnonymousStructOrUnion()) {
-        // Anonymous unions inside unions do not variant members create
-        if (!Union) {
-          for (CXXRecordDecl::field_iterator UI = FieldRecord->field_begin(),
-                                             UE = FieldRecord->field_end();
-               UI != UE; ++UI) {
-            QualType UnionFieldType = Context.getBaseElementType(UI->getType());
-            CXXRecordDecl *UnionFieldRecord =
-              UnionFieldType->getAsCXXRecordDecl();
-
-            // -- a variant member with a non-trivial [copy] constructor and X
-            //    is a union-like class
-            if (UnionFieldRecord &&
-                !UnionFieldRecord->hasTrivialCopyConstructor())
-              return true;
-          }
-        }
-
-        // Don't try to initalize an anonymous union
-        continue;
-      } else {
-         // -- a variant member with a non-trivial [copy] constructor and X is a
-         //    union-like class
-        if (Union && !FieldRecord->hasTrivialCopyConstructor())
-          return true;
-
-        // -- any [non-static data member] of a type with a destructor that is
-        //    deleted or inaccessible from the defaulted constructor
-        CXXDestructorDecl *FieldDtor = LookupDestructor(FieldRecord);
-        if (FieldDtor->isDeleted())
-          return true;
-        if (CheckDestructorAccess(Loc, FieldDtor, PDiag()) !=
-            AR_accessible)
-          return true;
-      }
-
-    // -- a [non-static data member of class type (or array thereof)] B that
-    //    cannot be [copied] because overload resolution, as applied to B's
-    //    [copy] constructor, results in an ambiguity or a function that is
-    //    deleted or inaccessible from the defaulted constructor
-      CXXConstructorDecl *FieldCtor = LookupCopyingConstructor(FieldRecord,
-                                                               ArgQuals);
-      if (!FieldCtor || FieldCtor->isDeleted())
-        return true;
-      if (CheckConstructorAccess(Loc, FieldCtor, FieldCtor->getAccess(),
-                                 PDiag()) != AR_accessible)
-        return true;
-    }
-  }
-
-  return false;
-}
-
 bool Sema::ShouldDeleteCopyAssignmentOperator(CXXMethodDecl *MD) {
   CXXRecordDecl *RD = MD->getParent();
   assert(!RD->isDependentType() && "do deletion after instantiation");
@@ -8735,7 +8623,7 @@ CXXConstructorDecl *Sema::DeclareImplicitCopyConstructor(
   //   deleted; ...
   if (ClassDecl->hasUserDeclaredMoveConstructor() ||
       ClassDecl->hasUserDeclaredMoveAssignment() ||
-      ShouldDeleteCopyConstructor(CopyConstructor))
+      ShouldDeleteSpecialMember(CopyConstructor, CXXCopyConstructor))
     CopyConstructor->setDeletedAsWritten();
   
   return CopyConstructor;

clang/test/CXX/special/class.copy/p11.0x.copy.cpp

@@ -0,0 +1,90 @@
+// RUN: %clang_cc1 -std=c++0x -fsyntax-only -verify %s
+
+struct NonTrivial {
+  NonTrivial(const NonTrivial&);
+};
+
+union DeletedNTVariant { // expected-note{{here}}
+  NonTrivial NT;
+  DeletedNTVariant();
+};
+DeletedNTVariant DVa;
+DeletedNTVariant DVb(DVa); // expected-error{{call to deleted constructor}}
+
+struct DeletedNTVariant2 { // expected-note{{here}}
+  union {
+    NonTrivial NT;
+  };
+  DeletedNTVariant2();
+};
+DeletedNTVariant2 DV2a;
+DeletedNTVariant2 DV2b(DV2a); // expected-error{{call to deleted constructor}}
+
+struct NoAccess {
+  NoAccess() = default;
+private:
+  NoAccess(const NoAccess&);
+
+  friend struct HasAccess;
+};
+
+struct HasNoAccess { // expected-note{{here}}
+  NoAccess NA;
+};
+HasNoAccess HNAa;
+HasNoAccess HNAb(HNAa); // expected-error{{call to deleted constructor}}
+
+struct HasAccess {
+  NoAccess NA;
+};
+
+HasAccess HAa;
+HasAccess HAb(HAa);
+
+struct NonConst {
+  NonConst(NonConst&);
+};
+struct Ambiguity {
+  Ambiguity(const Ambiguity&);
+  Ambiguity(volatile Ambiguity&);
+};
+
+struct IsAmbiguous { // expected-note{{here}}
+  NonConst NC;
+  Ambiguity A;
+  IsAmbiguous();
+};
+IsAmbiguous IAa;
+IsAmbiguous IAb(IAa); // expected-error{{call to deleted constructor}}
+
+struct Deleted { // expected-note{{here}}
+  IsAmbiguous IA;
+};
+Deleted Da;
+Deleted Db(Da); // expected-error{{call to deleted constructor}}
+
+struct NoAccessDtor {
+private:
+  ~NoAccessDtor();
+  friend struct HasAccessDtor;
+};
+
+struct HasNoAccessDtor { // expected-note{{here}}
+  NoAccessDtor NAD;
+  HasNoAccessDtor();
+  ~HasNoAccessDtor();
+};
+HasNoAccessDtor HNADa;
+HasNoAccessDtor HNADb(HNADa); // expected-error{{call to deleted constructor}}
+
+struct HasAccessDtor {
+  NoAccessDtor NAD;
+};
+HasAccessDtor HADa;
+HasAccessDtor HADb(HADa);
+
+struct RValue { // expected-note{{here}}
+  int && ri = 1;
+};
+RValue RVa;
+RValue RVb(RVa); // expected-error{{call to deleted constructor}}