A step towards sorting out handling of triviality of special members in C++11.

Separate out the notions of 'has a trivial special member' and 'has a
non-trivial special member', and use them appropriately. These are not
opposites of one another (there might be no special member, or in C++11 there
might be a trivial one and a non-trivial one). The CXXRecordDecl predicates
continue to produce incorrect results, but do so in fewer cases now, and
they document the cases where they might be wrong.

No functionality changes are intended here (they will come when the predicates
start producing the right answers...).


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

10 files changed, 126 insertions, 74 deletions

diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h
index 9cb56e2b3c..15e2e3b558 100644
--- a/include/clang/AST/DeclCXX.h
+++ b/include/clang/AST/DeclCXX.h
@@ -805,6 +805,12 @@ public:
     return data().FirstFriend != 0;
   }
 
+  /// \brief Determine whether this class has any default constructors.
+  bool hasDefaultConstructor() const {
+    return !data().UserDeclaredConstructor ||
+           data().DeclaredDefaultConstructor;
+  }
+
   /// \brief Determine if we need to declare a default constructor for
   /// this class.
   ///
@@ -814,8 +820,8 @@ public:
            !data().DeclaredDefaultConstructor;
   }
 
-  /// hasDeclaredDefaultConstructor - Whether this class's default constructor
-  /// has been declared (either explicitly or implicitly).
+  /// \brief Determine whether any default constructors have been declared for
+  /// this class (either explicitly or implicitly).
   bool hasDeclaredDefaultConstructor() const {
     return data().DeclaredDefaultConstructor;
   }
@@ -1079,65 +1085,111 @@ public:
   /// mutable field.
   bool hasMutableFields() const { return data().HasMutableFields; }
 
-  /// hasTrivialDefaultConstructor - Whether this class has a trivial default
-  /// constructor (C++11 [class.ctor]p5).
+  /// \brief Determine whether this class has a trivial default constructor
+  /// (C++11 [class.ctor]p5).
+  /// FIXME: This can be wrong when the class has multiple default constructors.
   bool hasTrivialDefaultConstructor() const {
-    return data().HasTrivialDefaultConstructor &&
-           (!data().UserDeclaredConstructor ||
-             data().DeclaredDefaultConstructor);
+    return hasDefaultConstructor() && data().HasTrivialDefaultConstructor;
+  }
+
+  /// \brief Determine whether this class has a non-trivial default constructor
+  /// (C++11 [class.ctor]p5).
+  bool hasNonTrivialDefaultConstructor() const {
+    return hasDefaultConstructor() && !data().HasTrivialDefaultConstructor;
   }
 
-  /// hasConstexprNonCopyMoveConstructor - Whether this class has at least one
-  /// constexpr constructor other than the copy or move constructors.
+  /// \brief Determine whether this class has at least one constexpr constructor
+  /// other than the copy or move constructors.
   bool hasConstexprNonCopyMoveConstructor() const {
     return data().HasConstexprNonCopyMoveConstructor ||
            (!hasUserDeclaredConstructor() &&
             defaultedDefaultConstructorIsConstexpr());
   }
 
-  /// defaultedDefaultConstructorIsConstexpr - Whether a defaulted default
-  /// constructor for this class would be constexpr.
+  /// \brief Determine whether a defaulted default constructor for this class
+  /// would be constexpr.
   bool defaultedDefaultConstructorIsConstexpr() const {
     return data().DefaultedDefaultConstructorIsConstexpr &&
            (!isUnion() || hasInClassInitializer());
   }
 
-  /// hasConstexprDefaultConstructor - Whether this class has a constexpr
-  /// default constructor.
+  /// \brief Determine whether this class has a constexpr default constructor.
   bool hasConstexprDefaultConstructor() const {
     return data().HasConstexprDefaultConstructor ||
            (!data().UserDeclaredConstructor &&
             defaultedDefaultConstructorIsConstexpr());
   }
 
-  // hasTrivialCopyConstructor - Whether this class has a trivial copy
-  // constructor (C++ [class.copy]p6, C++0x [class.copy]p13)
+  /// \brief Determine whether this class has a trivial copy constructor
+  /// (C++ [class.copy]p6, C++11 [class.copy]p12)
+  /// FIXME: This can be wrong if the class has multiple copy constructors.
   bool hasTrivialCopyConstructor() const {
     return data().HasTrivialCopyConstructor;
   }
 
-  // hasTrivialMoveConstructor - Whether this class has a trivial move
-  // constructor (C++0x [class.copy]p13)
+  /// \brief Determine whether this class has a non-trivial copy constructor
+  /// (C++ [class.copy]p6, C++11 [class.copy]p12)
+  bool hasNonTrivialCopyConstructor() const {
+    return !data().HasTrivialCopyConstructor;
+  }
+
+  /// \brief Determine whether this class has a trivial move constructor
+  /// (C++11 [class.copy]p12)
+  /// FIXME: This can be wrong if the class has multiple move constructors,
+  /// or if the implicit move constructor would be deleted.
   bool hasTrivialMoveConstructor() const {
-    return data().HasTrivialMoveConstructor;
+    return data().HasTrivialMoveConstructor &&
+           (hasDeclaredMoveConstructor() || needsImplicitMoveConstructor());
   }
 
-  // hasTrivialCopyAssignment - Whether this class has a trivial copy
-  // assignment operator (C++ [class.copy]p11, C++0x [class.copy]p27)
+  /// \brief Determine whether this class has a non-trivial move constructor
+  /// (C++11 [class.copy]p12)
+  /// FIXME: This can be wrong if the implicit move constructor would be
+  /// deleted.
+  bool hasNonTrivialMoveConstructor() const {
+    return !data().HasTrivialMoveConstructor &&
+           (hasDeclaredMoveConstructor() || needsImplicitMoveConstructor());
+  }
+
+  /// \brief Determine whether this class has a trivial copy assignment operator
+  /// (C++ [class.copy]p11, C++11 [class.copy]p25)
+  /// FIXME: This can be wrong if the class has multiple copy assignment
+  /// operators.
   bool hasTrivialCopyAssignment() const {
     return data().HasTrivialCopyAssignment;
   }
 
-  // hasTrivialMoveAssignment - Whether this class has a trivial move
-  // assignment operator (C++0x [class.copy]p27)
+  /// \brief Determine whether this class has a non-trivial copy assignment
+  /// operator (C++ [class.copy]p11, C++11 [class.copy]p25)
+  bool hasNonTrivialCopyAssignment() const {
+    return !data().HasTrivialCopyAssignment;
+  }
+
+  /// \brief Determine whether this class has a trivial move assignment operator
+  /// (C++11 [class.copy]p25)
+  /// FIXME: This can be wrong if the class has multiple move assignment
+  /// operators, or if the implicit move assignment operator would be deleted.
   bool hasTrivialMoveAssignment() const {
-    return data().HasTrivialMoveAssignment;
+    return data().HasTrivialMoveAssignment &&
+           (hasDeclaredMoveAssignment() || needsImplicitMoveAssignment());
   }
 
-  // hasTrivialDestructor - Whether this class has a trivial destructor
-  // (C++ [class.dtor]p3)
+  /// \brief Determine whether this class has a non-trivial move assignment
+  /// operator (C++11 [class.copy]p25)
+  /// FIXME: This can be wrong if the implicit move assignment would be deleted.
+  bool hasNonTrivialMoveAssignment() const {
+    return !data().HasTrivialMoveAssignment &&
+           (hasDeclaredMoveAssignment() || needsImplicitMoveAssignment());
+  }
+
+  /// \brief Determine whether this class has a trivial destructor
+  /// (C++ [class.dtor]p3)
   bool hasTrivialDestructor() const { return data().HasTrivialDestructor; }
 
+  /// \brief Determine whether this class has a non-trivial destructor
+  /// (C++ [class.dtor]p3)
+  bool hasNonTrivialDestructor() const { return !data().HasTrivialDestructor; }
+
   // hasIrrelevantDestructor - Whether this class has a destructor which has no
   // semantic effect. Any such destructor will be trivial, public, defaulted
   // and not deleted, and will call only irrelevant destructors.
diff --git a/lib/AST/DeclCXX.cpp b/lib/AST/DeclCXX.cpp
index 82e630acef..a0ff6508ce 100644
--- a/lib/AST/DeclCXX.cpp
+++ b/lib/AST/DeclCXX.cpp
@@ -244,9 +244,7 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases,
       // non-trivial if it calls anything other than a trivial move constructor.
       if (!BaseClassDecl->hasTrivialCopyConstructor())
         data().HasTrivialCopyConstructor = false;
-      if (!BaseClassDecl->hasTrivialMoveConstructor() ||
-          !(BaseClassDecl->hasDeclaredMoveConstructor() ||
-            BaseClassDecl->needsImplicitMoveConstructor()))
+      if (!BaseClassDecl->hasTrivialMoveConstructor())
         data().HasTrivialMoveConstructor = false;
 
       // C++0x [class.copy]p27:
@@ -258,9 +256,7 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases,
       // of all of them.
       if (!BaseClassDecl->hasTrivialCopyAssignment())
         data().HasTrivialCopyAssignment = false;
-      if (!BaseClassDecl->hasTrivialMoveAssignment() ||
-          !(BaseClassDecl->hasDeclaredMoveAssignment() ||
-            BaseClassDecl->needsImplicitMoveAssignment()))
+      if (!BaseClassDecl->hasTrivialMoveAssignment())
         data().HasTrivialMoveAssignment = false;
 
       // C++11 [class.ctor]p6:
@@ -321,13 +317,13 @@ bool CXXRecordDecl::isTriviallyCopyable() const {
   // C++0x [class]p5:
   //   A trivially copyable class is a class that:
   //   -- has no non-trivial copy constructors,
-  if (!hasTrivialCopyConstructor()) return false;
+  if (hasNonTrivialCopyConstructor()) return false;
   //   -- has no non-trivial move constructors,
-  if (!hasTrivialMoveConstructor()) return false;
+  if (hasNonTrivialMoveConstructor()) return false;
   //   -- has no non-trivial copy assignment operators,
-  if (!hasTrivialCopyAssignment()) return false;
+  if (hasNonTrivialCopyAssignment()) return false;
   //   -- has no non-trivial move assignment operators, and
-  if (!hasTrivialMoveAssignment()) return false;
+  if (hasNonTrivialMoveAssignment()) return false;
   //   -- has a trivial destructor.
   if (!hasTrivialDestructor()) return false;
 
@@ -835,9 +831,7 @@ NotASpecialMember:;
         // FIXME: C++0x: We don't correctly model 'selected' constructors.
         if (!FieldRec->hasTrivialCopyConstructor())
           data().HasTrivialCopyConstructor = false;
-        if (!FieldRec->hasTrivialMoveConstructor() ||
-            !(FieldRec->hasDeclaredMoveConstructor() ||
-              FieldRec->needsImplicitMoveConstructor()))
+        if (!FieldRec->hasTrivialMoveConstructor())
           data().HasTrivialMoveConstructor = false;
 
         // C++0x [class.copy]p27:
@@ -849,9 +843,7 @@ NotASpecialMember:;
         // FIXME: C++0x: We don't correctly model 'selected' operators.
         if (!FieldRec->hasTrivialCopyAssignment())
           data().HasTrivialCopyAssignment = false;
-        if (!FieldRec->hasTrivialMoveAssignment() ||
-            !(FieldRec->hasDeclaredMoveAssignment() ||
-              FieldRec->needsImplicitMoveAssignment()))
+        if (!FieldRec->hasTrivialMoveAssignment())
           data().HasTrivialMoveAssignment = false;
 
         if (!FieldRec->hasTrivialDestructor())
diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp
index 1bf1c1b4c4..0aab240a53 100644
--- a/lib/AST/Type.cpp
+++ b/lib/AST/Type.cpp
@@ -1052,11 +1052,13 @@ bool QualType::isTrivialType(ASTContext &Context) const {
   if (const RecordType *RT = CanonicalType->getAs<RecordType>()) {
     if (const CXXRecordDecl *ClassDecl =
         dyn_cast<CXXRecordDecl>(RT->getDecl())) {
-      // C++0x [class]p5:
-      //   A trivial class is a class that has a trivial default constructor
-      if (!ClassDecl->hasTrivialDefaultConstructor()) return false;
-      //   and is trivially copyable.
-      if (!ClassDecl->isTriviallyCopyable()) return false;
+      // C++11 [class]p6:
+      //   A trivial class is a class that has a default constructor,
+      //   has no non-trivial default constructors, and is trivially
+      //   copyable.
+      return ClassDecl->hasDefaultConstructor() &&
+             !ClassDecl->hasNonTrivialDefaultConstructor() &&
+             ClassDecl->isTriviallyCopyable();
     }
     
     return true;
diff --git a/lib/CodeGen/CGBlocks.cpp b/lib/CodeGen/CGBlocks.cpp
index 69449b1aa5..06fd624759 100644
--- a/lib/CodeGen/CGBlocks.cpp
+++ b/lib/CodeGen/CGBlocks.cpp
@@ -218,7 +218,7 @@ static bool isSafeForCXXConstantCapture(QualType type) {
 
   // Maintain semantics for classes with non-trivial dtors or copy ctors.
   if (!record->hasTrivialDestructor()) return false;
-  if (!record->hasTrivialCopyConstructor()) return false;
+  if (record->hasNonTrivialCopyConstructor()) return false;
 
   // Otherwise, we just have to make sure there aren't any mutable
   // fields that might have changed since initialization.
diff --git a/lib/CodeGen/CGDebugInfo.cpp b/lib/CodeGen/CGDebugInfo.cpp
index 756af3fce7..8a66dff3d2 100644
--- a/lib/CodeGen/CGDebugInfo.cpp
+++ b/lib/CodeGen/CGDebugInfo.cpp
@@ -2332,7 +2332,7 @@ void CGDebugInfo::EmitDeclare(const VarDecl *VD, unsigned Tag,
       // If an aggregate variable has non trivial destructor or non trivial copy
       // constructor than it is pass indirectly. Let debug info know about this
       // by using reference of the aggregate type as a argument type.
-      if (!Record->hasTrivialCopyConstructor() ||
+      if (Record->hasNonTrivialCopyConstructor() ||
           !Record->hasTrivialDestructor())
         Ty = DBuilder.createReferenceType(llvm::dwarf::DW_TAG_reference_type, Ty);
     }
diff --git a/lib/CodeGen/CGExprAgg.cpp b/lib/CodeGen/CGExprAgg.cpp
index 718e8f999c..c9e43e9e33 100644
--- a/lib/CodeGen/CGExprAgg.cpp
+++ b/lib/CodeGen/CGExprAgg.cpp
@@ -213,7 +213,7 @@ bool AggExprEmitter::TypeRequiresGCollection(QualType T) {
   // Don't mess with non-trivial C++ types.
   RecordDecl *Record = RecordTy->getDecl();
   if (isa<CXXRecordDecl>(Record) &&
-      (!cast<CXXRecordDecl>(Record)->hasTrivialCopyConstructor() ||
+      (cast<CXXRecordDecl>(Record)->hasNonTrivialCopyConstructor() ||
        !cast<CXXRecordDecl>(Record)->hasTrivialDestructor()))
     return false;
 
@@ -1285,7 +1285,7 @@ void CodeGenFunction::EmitAggregateCopy(llvm::Value *DestPtr,
               Record->hasTrivialCopyAssignment() ||
               Record->hasTrivialMoveConstructor() ||
               Record->hasTrivialMoveAssignment()) &&
-             "Trying to aggregate-copy a type without a trivial copy "
+             "Trying to aggregate-copy a type without a trivial copy/move "
              "constructor or assignment operator");
       // Ignore empty classes in C++.
       if (Record->isEmpty())
diff --git a/lib/CodeGen/TargetInfo.cpp b/lib/CodeGen/TargetInfo.cpp
index ffff0d0a1b..1814e1f28a 100644
--- a/lib/CodeGen/TargetInfo.cpp
+++ b/lib/CodeGen/TargetInfo.cpp
@@ -173,7 +173,7 @@ static bool hasNonTrivialDestructorOrCopyConstructor(const RecordType *RT) {
   if (!RD)
     return false;
 
-  return !RD->hasTrivialDestructor() || !RD->hasTrivialCopyConstructor();
+  return !RD->hasTrivialDestructor() || RD->hasNonTrivialCopyConstructor();
 }
 
 /// isRecordWithNonTrivialDestructorOrCopyConstructor - Determine if a type is
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 72516fda80..cc19ce9959 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -9735,13 +9735,18 @@ bool Sema::CheckNontrivialField(FieldDecl *FD) {
       // copy constructors.
 
       CXXSpecialMember member = CXXInvalid;
-      if (!RDecl->hasTrivialCopyConstructor())
+      // We're required to check for any non-trivial constructors. Since the
+      // implicit default constructor is suppressed if there are any
+      // user-declared constructors, we just need to check that there is a
+      // trivial default constructor and a trivial copy constructor. (We don't
+      // worry about move constructors here, since this is a C++98 check.)
+      if (RDecl->hasNonTrivialCopyConstructor())
         member = CXXCopyConstructor;
       else if (!RDecl->hasTrivialDefaultConstructor())
         member = CXXDefaultConstructor;
-      else if (!RDecl->hasTrivialCopyAssignment())
+      else if (RDecl->hasNonTrivialCopyAssignment())
         member = CXXCopyAssignment;
-      else if (!RDecl->hasTrivialDestructor())
+      else if (RDecl->hasNonTrivialDestructor())
         member = CXXDestructor;
 
       if (member != CXXInvalid) {
@@ -9789,6 +9794,8 @@ static bool diagnoseNonTrivialUserDeclaredCtor(Sema &S, QualType QT,
 
 /// DiagnoseNontrivial - Given that a class has a non-trivial
 /// special member, figure out why.
+/// FIXME: These checks are not correct in C++11 mode. Currently, this is OK
+/// since we only use this in C++11 for a -Wc++98-compat warning.
 void Sema::DiagnoseNontrivial(const RecordType* T, CXXSpecialMember member) {
   QualType QT(T, 0U);
   CXXRecordDecl* RD = cast<CXXRecordDecl>(T->getDecl());
@@ -9887,17 +9894,21 @@ void Sema::DiagnoseNontrivial(const RecordType* T, CXXSpecialMember member) {
     }
   }
 
-  bool (CXXRecordDecl::*hasTrivial)() const;
+  bool (CXXRecordDecl::*hasNonTrivial)() const;
   switch (member) {
   case CXXDefaultConstructor:
-    hasTrivial = &CXXRecordDecl::hasTrivialDefaultConstructor; break;
+    hasNonTrivial = &CXXRecordDecl::hasNonTrivialDefaultConstructor; break;
   case CXXCopyConstructor:
-    hasTrivial = &CXXRecordDecl::hasTrivialCopyConstructor; break;
+    hasNonTrivial = &CXXRecordDecl::hasNonTrivialCopyConstructor; break;
   case CXXCopyAssignment:
-    hasTrivial = &CXXRecordDecl::hasTrivialCopyAssignment; break;
+    hasNonTrivial = &CXXRecordDecl::hasNonTrivialCopyAssignment; break;
+  case CXXMoveConstructor:
+    hasNonTrivial = &CXXRecordDecl::hasNonTrivialMoveConstructor; break;
+  case CXXMoveAssignment:
+    hasNonTrivial = &CXXRecordDecl::hasNonTrivialMoveAssignment; break;
   case CXXDestructor:
-    hasTrivial = &CXXRecordDecl::hasTrivialDestructor; break;
-  default:
+    hasNonTrivial = &CXXRecordDecl::hasNonTrivialDestructor; break;
+  case CXXInvalid:
     llvm_unreachable("unexpected special member");
   }
 
@@ -9906,7 +9917,7 @@ void Sema::DiagnoseNontrivial(const RecordType* T, CXXSpecialMember member) {
     const RecordType *BaseRT = bi->getType()->getAs<RecordType>();
     assert(BaseRT && "Don't know how to handle dependent bases");
     CXXRecordDecl *BaseRecTy = cast<CXXRecordDecl>(BaseRT->getDecl());
-    if (!(BaseRecTy->*hasTrivial)()) {
+    if ((BaseRecTy->*hasNonTrivial)()) {
       SourceLocation BaseLoc = bi->getLocStart();
       Diag(BaseLoc, diag::note_nontrivial_has_nontrivial) << QT << 1 << member;
       DiagnoseNontrivial(BaseRT, member);
@@ -9922,7 +9933,7 @@ void Sema::DiagnoseNontrivial(const RecordType* T, CXXSpecialMember member) {
     if (const RecordType *EltRT = EltTy->getAs<RecordType>()) {
       CXXRecordDecl* EltRD = cast<CXXRecordDecl>(EltRT->getDecl());
 
-      if (!(EltRD->*hasTrivial)()) {
+      if ((EltRD->*hasNonTrivial)()) {
         SourceLocation FLoc = fi->getLocation();
         Diag(FLoc, diag::note_nontrivial_has_nontrivial) << QT << 0 << member;
         DiagnoseNontrivial(EltRT, member);
@@ -9945,8 +9956,6 @@ void Sema::DiagnoseNontrivial(const RecordType* T, CXXSpecialMember member) {
       }
     }
   }
-
-  llvm_unreachable("found no explanation for non-trivial member");
 }
 
 /// TranslateIvarVisibility - Translate visibility from a token ID to an
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index a8ddc51f3f..e98f75ebe6 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -8201,10 +8201,7 @@ hasVirtualBaseWithNonTrivialMoveAssignment(Sema &S, CXXRecordDecl *ClassDecl) {
     if (BaseClass->needsImplicitMoveAssignment())
       S.DeclareImplicitMoveAssignment(BaseClass);
 
-    // If the class has both a trivial move assignment and a non-trivial move
-    // assignment, hasTrivialMoveAssignment() is false.
-    if (BaseClass->hasDeclaredMoveAssignment() &&
-        !BaseClass->hasTrivialMoveAssignment())
+    if (BaseClass->hasNonTrivialMoveAssignment())
       return true;
   }
 
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index e419ba5a3f..840f04f0da 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -635,16 +635,16 @@ Sema::VarArgKind Sema::isValidVarArgType(const QualType &Ty) {
   if (Ty.isCXX98PODType(Context))
     return VAK_Valid;
 
-  // C++0x [expr.call]p7:
-  //   Passing a potentially-evaluated argument of class type (Clause 9) 
+  // C++11 [expr.call]p7:
+  //   Passing a potentially-evaluated argument of class type (Clause 9)
   //   having a non-trivial copy constructor, a non-trivial move constructor,
-  //   or a non-trivial destructor, with no corresponding parameter, 
+  //   or a non-trivial destructor, with no corresponding parameter,
   //   is conditionally-supported with implementation-defined semantics.
   if (getLangOpts().CPlusPlus0x && !Ty->isDependentType())
     if (CXXRecordDecl *Record = Ty->getAsCXXRecordDecl())
-      if (Record->hasTrivialCopyConstructor() &&
-          Record->hasTrivialMoveConstructor() &&
-          Record->hasTrivialDestructor())
+      if (!Record->hasNonTrivialCopyConstructor() &&
+          !Record->hasNonTrivialMoveConstructor() &&
+          !Record->hasNonTrivialDestructor())
         return VAK_ValidInCXX11;
 
   if (getLangOpts().ObjCAutoRefCount && Ty->isObjCLifetimeType())