[Sema][C++] Adopt DR2171 relaxed triviality requirements

[comex]

Summary

When defining a copy constructor or assignment operator as either explicitly defaulted (= default) or deleted (= delete), it's possible to give it a different parameter list from the normal (const Foo &).

For = default, the only allowed change [1] is to use a non-const reference instead of a const one:

struct X {

X(X &) = default;

};

For = delete, it's also possible to add additional parameters with default values, as well as variadic parameters (...).

(Besides the parameter list, it's also possible to change the exception specification and ref-qualifiers, but those never affected triviality.)

Originally, C++11 (which introduced explicit defaulting) had a rule that any modification of the parameter list would prevent the function from being considered trivial, so e.g. is_trivially_copyable_v<X> would be false. However, Defect Report 2171 [2] (2016) removed that rule entirely. Up until now, that change hasn't been implemented in Clang; this patch implements it.

In addition, Clang currently applies a similar rule to deleted *default* constructors, which AFAICT is not in compliance with any published version of the spec. So this fails when it should pass:

struct X {

X(...) = delete;

};
static_assert(__has_trivial_constructor(X));

This patch also fixes that.

This is an ABI-breaking change, since the existence of trivial constructors affects whether a type is "trivial for the purposes of calls" [3]. I checked other compilers to see whether they implement the DR:

• GCC has never treated such functions as nontrivial, even before the DR. [4]
• MSVC currently doesn't treat them as nontrivial [5]; I haven't checked old versions since they're not on Godbolt.

Thus the change would improve Clang's compatibility with those compilers.

Implementation-wise, this patch is simple enough: it just removes the code in Sema::SpecialMemberIsTrivial that checked the parameter list. In terms of tests, there were already a number of tests verifying the old behavior, so the patch merely updates them for the new behavior.

[1] https://eel.is/c++draft/dcl.fct.def.default#2
[2] http://www.open-std.org/Jtc1/sc22/wg21/docs/cwg_defects.html#2171
[3] https://quuxplusone.github.io/blog/2018/05/02/trivial-abi-101/
[4] https://gcc.godbolt.org/z/GU_wyz (note GCC version)
[5] https://gcc.godbolt.org/z/VYiMn3

[rsmith]

This change should be guarded by -fclang-abi-compat= so that users can opt to using the old ABI.

clang/lib/Sema/SemaDeclCXX.cpp

9200–9202

There is corresponding code in lib/AST/DeclCXX.cpp that attempts to determine whether special members are trivial "on the fly" (without performing overload resolution etc). Please check whether that needs to be updated too.

clang/test/CXX/class/class.union/p1.cpp

94–96

Do we still have test coverage for this somewhere? It'd generally be good to keep around the old tests but with updated diagnostic expectations.

clang/include/clang/Basic/DiagnosticSemaKinds.td

def note_nontrivial_no_copy : Note<
   "assignment operator|<<ERROR>>}2 can be used to "
   "%select{<<ERROR>>|copy|move|copy|move|<<ERROR>>}2 "
   "%select{base class|field|an object}0 of type %3">;
 def note_nontrivial_user_provided : Note<
   "because %select{base class of |field of |}0type %1 has a user-provided "
   "%sub{select_special_member_kind}2">;
 def note_nontrivial_in_class_init : Note<
   "because field %0 has an initializer">;
-def note_nontrivial_param_type : Note<
-  "because its parameter is %diff{of type $, not $|of the wrong type}2,3">;
-def note_nontrivial_default_arg : Note<"because it has a default argument">;
-def note_nontrivial_variadic : Note<"because it is a variadic function">;
 def note_nontrivial_subobject : Note<
   "because the function selected to %select{construct|copy|move|copy|move|"
   "destroy}2 %select{base class|field}0 of type %1 is not trivial">;
 def note_nontrivial_objc_ownership : Note<
   "because type %0 has a member with %select{no|no|__strong|__weak|"
   "__autoreleasing}1 ownership">;
 
 /// Selector for a TagTypeKind value.

clang/lib/Sema/SemaDeclCXX.cpp

 /// Determine whether a defaulted or deleted special member function is trivial,
 /// as specified in C++11 [class.ctor]p5, C++11 [class.copy]p12,
 /// C++11 [class.copy]p25, and C++11 [class.dtor]p5.
 bool Sema::SpecialMemberIsTrivial(CXXMethodDecl *MD, CXXSpecialMember CSM,
                                   TrivialABIHandling TAH, bool Diagnose) {
   assert(!MD->isUserProvided() && CSM != CXXInvalid && "not special enough");
 
   CXXRecordDecl *RD = MD->getParent();
-
-  bool ConstArg = false;
-
-  // C++11 [class.copy]p12, p25: [DR1593]
-  //   A [special member] is trivial if [...] its parameter-type-list is
-  //   equivalent to the parameter-type-list of an implicit declaration [...]
-  switch (CSM) {
-  case CXXDefaultConstructor:
-  case CXXDestructor:
-    // Trivial default constructors and destructors cannot have parameters.
-    break;
-
-  case CXXCopyConstructor:
-  case CXXCopyAssignment: {
-    // Trivial copy operations always have const, non-volatile parameter types.
-    ConstArg = true;
-    const ParmVarDecl *Param0 = MD->getParamDecl(0);
-    const ReferenceType *RT = Param0->getType()->getAs<ReferenceType>();
-    if (!RT || RT->getPointeeType().getCVRQualifiers() != Qualifiers::Const) {
-      if (Diagnose)
-        Diag(Param0->getLocation(), diag::note_nontrivial_param_type)
-          << Param0->getSourceRange() << Param0->getType()
-          << Context.getLValueReferenceType(
-               Context.getRecordType(RD).withConst());
-      return false;
-    }
-    break;
-  }
-
-  case CXXMoveConstructor:
-  case CXXMoveAssignment: {
-    // Trivial move operations always have non-cv-qualified parameters.
-    const ParmVarDecl *Param0 = MD->getParamDecl(0);
-    const RValueReferenceType *RT =
-      Param0->getType()->getAs<RValueReferenceType>();
-    if (!RT || RT->getPointeeType().getCVRQualifiers()) {
-      if (Diagnose)
-        Diag(Param0->getLocation(), diag::note_nontrivial_param_type)
-          << Param0->getSourceRange() << Param0->getType()
-          << Context.getRValueReferenceType(Context.getRecordType(RD));
-      return false;
-    }
-    break;
-  }
-
-  case CXXInvalid:
-    llvm_unreachable("not a special member");
-  }
-
-  if (MD->getMinRequiredArguments() < MD->getNumParams()) {
-    if (Diagnose)
-      Diag(MD->getParamDecl(MD->getMinRequiredArguments())->getLocation(),
-           diag::note_nontrivial_default_arg)
-        << MD->getParamDecl(MD->getMinRequiredArguments())->getSourceRange();
-    return false;
-  }
-  if (MD->isVariadic()) {
-    if (Diagnose)
-      Diag(MD->getLocation(), diag::note_nontrivial_variadic);
-    return false;
-  }
+  bool ConstArg = CSM == CXXCopyConstructor || CSM == CXXCopyAssignment;
 
   // C++11 [class.ctor]p5, C++11 [class.dtor]p5:
   //   A copy/move [constructor or assignment operator] is trivial if
   //    -- the [member] selected to copy/move each direct base class subobject
   //       is trivial
   //
   // C++11 [class.copy]p12, C++11 [class.copy]p25:
   //   A [default constructor or destructor] is trivial if

clang/test/CXX/class/class.union/p1.cpp

union U3 {
   struct s4 : CopyCtor { // expected-note {{because no constructor can be used to copy an object of type 'const U3::s4'}}
   } m4; // expected-error {{union member 'm4' has a non-trivial copy constructor}}
   struct s5 : CopyAssign { // expected-note {{because no assignment operator can be used to copy an object of type 'const U3::s5'}}
   } m5; // expected-error {{union member 'm5' has a non-trivial copy assignment operator}}
   struct s6 : Dtor { // expected-note {{because base class of type 'Dtor' has a user-provided destructor}}
   } m6; // expected-error {{union member 'm6' has a non-trivial destructor}}
   struct s7 : Okay {
   } m7;
-  struct s8 {
-    s8(...) = delete; // expected-note {{because it is a variadic function}} expected-warning {{C++11}}
-  } m8; // expected-error {{union member 'm8' has a non-trivial default constructor}}
 };
 
 union U4 {
   static int i1; // expected-warning {{static data member 'i1' in union is a C++11 extension}}
 };
 int U4::i1 = 10;
 
 union U5 {

clang/test/CXX/special/class.copy/p12-0x.cpp

-// RUN: %clang_cc1 -std=c++11 -verify %s -Wno-defaulted-function-deleted
+// RUN: %clang_cc1 -std=c++17 -verify %s -Wno-defaulted-function-deleted
 
 // expected-no-diagnostics
 
-template<typename T, bool B> struct trivially_copyable_check {
-  static_assert(B == __has_trivial_copy(T), "");
+enum TrivialCtorKind {
+  None,
+  NonConst,
+  Const,
+  ConstWithNonTrivialNonConstOverload,
+};
+
+template<typename T, TrivialCtorKind Kind> struct trivially_copyable_check {
+  static constexpr bool B = (Kind == Const || Kind == ConstWithNonTrivialNonConstOverload);
+  static_assert((Kind != None) == __has_trivial_copy(T), "");
   static_assert(B == __is_trivially_constructible(T, T), "");
   static_assert(B == __is_trivially_constructible(T, const T &), "");
   static_assert(B == __is_trivially_constructible(T, T &&), "");
+  static_assert((Kind == NonConst || Kind == Const) == __is_trivially_constructible(T, T &), "");
   typedef void type;
 };
-template<typename T> using trivially_copyable =
-  typename trivially_copyable_check<T, true>::type;
+template<typename T, TrivialCtorKind Kind = Const> using trivially_copyable =
+  typename trivially_copyable_check<T, Kind>::type;
 template<typename T> using not_trivially_copyable =
-  typename trivially_copyable_check<T, false>::type;
+  typename trivially_copyable_check<T, None>::type;
 
 struct Trivial {};
 using _ = trivially_copyable<Trivial>;
 
 // A copy/move constructor for class X is trivial if it is not user-provided,
 struct UserProvided {
   UserProvided(const UserProvided &);
 };
 using _ = not_trivially_copyable<UserProvided>;
 
-// its declared parameter type is the same as if it had been implicitly
-// declared,
+// [its declared parameter type is the same as if it had been implicitly
+// declared, - removed by DR2171]
 struct NonConstCopy {
   NonConstCopy(NonConstCopy &) = default;
 };
-using _ = not_trivially_copyable<NonConstCopy>;
+using _ = trivially_copyable<NonConstCopy, NonConst>;
 
 // class X has no virtual functions
 struct VFn {
   virtual void f();
 };
 using _ = not_trivially_copyable<VFn>;
 
 // and no virtual base classes
 struct VBase : virtual Trivial {};
 using _ = not_trivially_copyable<VBase>;
 
 // and the constructor selected to copy/move each [direct subobject] is trivial
 struct TemplateCtor {
   template<typename T> TemplateCtor(T &);
 };
-using _ = trivially_copyable<TemplateCtor>;
+using _ = trivially_copyable<TemplateCtor, ConstWithNonTrivialNonConstOverload>;
 struct TemplateCtorMember {
   TemplateCtor tc;
 };
 using _ = trivially_copyable<TemplateCtorMember>;
 
 // We can select a non-trivial copy ctor even if there is a trivial one.
 struct MutableTemplateCtorMember {
   mutable TemplateCtor mtc;

clang/test/CXX/special/class.copy/p25-0x.cpp

-// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++17 -verify %s
 
 // expected-no-diagnostics
 
-template<typename T, bool B> struct trivially_assignable_check {
-  static_assert(B == __has_trivial_assign(T), "");
+enum TrivialCopyKind {
+  None,
+  NonConst,
+  Const,
+  ConstWithNonTrivialNonConstOverload,
+};
+
+template<typename T, TrivialCopyKind Kind> struct trivially_assignable_check {
+  static constexpr bool B = (Kind == Const || Kind == ConstWithNonTrivialNonConstOverload);
+  static_assert((Kind != None) == __has_trivial_assign(T), "");
   static_assert(B == __is_trivially_assignable(T&, T), "");
-  static_assert(B == __is_trivially_assignable(T&, const T &), "");
-  static_assert(B == __is_trivially_assignable(T&, T &&), "");
+  static_assert(B == __is_trivially_assignable(T&, const T&), "");
+  static_assert(B == __is_trivially_assignable(T&, T&&), "");
   static_assert(B == __is_trivially_assignable(T&&, T), "");
-  static_assert(B == __is_trivially_assignable(T&&, const T &), "");
-  static_assert(B == __is_trivially_assignable(T&&, T &&), "");
+  static_assert(B == __is_trivially_assignable(T&&, const T&), "");
+  static_assert(B == __is_trivially_assignable(T&&, T&&), "");
+  static_assert((Kind == NonConst || Kind == Const) == __is_trivially_assignable(T&, T&), "");
+  static_assert((Kind == NonConst || Kind == Const) == __is_trivially_assignable(T&&, T&), "");
   typedef void type;
 };
-template<typename T> using trivially_assignable =
-  typename trivially_assignable_check<T, true>::type;
+template<typename T, TrivialCopyKind Kind = Const> using trivially_assignable =
+  typename trivially_assignable_check<T, Kind>::type;
 template<typename T> using not_trivially_assignable =
-  typename trivially_assignable_check<T, false>::type;
+  typename trivially_assignable_check<T, None>::type;
 
 struct Trivial {};
 using _ = trivially_assignable<Trivial>;
 
 // A copy/move assignment operator for class X is trivial if it is not user-provided,
 struct UserProvided {
   UserProvided &operator=(const UserProvided &);
 };
 using _ = not_trivially_assignable<UserProvided>;
 
-// its declared parameter type is the same as if it had been implicitly
-// declared,
+// [its declared parameter type is the same as if it had been implicitly
+// declared, - removed by DR2171]
 struct NonConstCopy {
   NonConstCopy &operator=(NonConstCopy &) = default;
 };
-using _ = not_trivially_assignable<NonConstCopy>;
+using _ = trivially_assignable<NonConstCopy, NonConst>;
 
 // class X has no virtual functions
 struct VFn {
   virtual void f();
 };
 using _ = not_trivially_assignable<VFn>;
 
 // and no virtual base classes
 struct VBase : virtual Trivial {};
 using _ = not_trivially_assignable<VBase>;
 
 // and the assignment operator selected to copy/move each [direct subobject] is trivial
 struct TemplateCtor {
   template<typename T> TemplateCtor operator=(T &);
 };
-using _ = trivially_assignable<TemplateCtor>;
+using _ = trivially_assignable<TemplateCtor, ConstWithNonTrivialNonConstOverload>;
 struct TemplateCtorMember {
   TemplateCtor tc;
 };
 using _ = trivially_assignable<TemplateCtorMember>;
 struct MutableTemplateCtorMember {
   mutable TemplateCtor mtc;
 };
 static_assert(!__is_trivially_assignable(MutableTemplateCtorMember, const MutableTemplateCtorMember &), "");

 static_assert(__is_trivially_assignable(TNTMember, const TNTMember &), "");
 static_assert(!__is_trivially_assignable(TNTMember, volatile TNTMember &), "");
 static_assert(__is_trivially_assignable(TNTMember, TNTMember &&), "");
 static_assert(__is_trivially_assignable(TNTMember, const TNTMember &&), "");
 static_assert(!__is_trivially_assignable(TNTMember, volatile TNTMember &&), "");
 
 struct NCCTNT : NonConstCopy, TNT {};
 
-static_assert(!__has_trivial_assign(NCCTNT), "");
+static_assert(__has_trivial_assign(NCCTNT), "");
 static_assert(!__is_trivially_assignable(NCCTNT, NCCTNT), "");
-static_assert(!__is_trivially_assignable(NCCTNT, NCCTNT &), "");
+static_assert(__is_trivially_assignable(NCCTNT, NCCTNT &), "");
 static_assert(!__is_trivially_assignable(NCCTNT, const NCCTNT &), "");
 static_assert(!__is_trivially_assignable(NCCTNT, volatile NCCTNT &), "");
 static_assert(!__is_trivially_assignable(NCCTNT, NCCTNT &&), "");
 static_assert(!__is_trivially_assignable(NCCTNT, const NCCTNT &&), "");
 static_assert(!__is_trivially_assignable(NCCTNT, volatile NCCTNT &&), "");
 
 struct MultipleTrivial {
   // All four of these are trivial.

clang/test/CXX/special/class.ctor/p5-0x.cpp

 
 // - all the direct base classes of its class have trivial default constructors, and
 ASSERT_NONTRIVIAL(NonTrivialDefCtor5, : NonTrivialDefCtor1, )
 
 // - for all the non-static data members of its class that are of class type (or array thereof), each such class
 // has a trivial default constructor.
 ASSERT_NONTRIVIAL(NonTrivialDefCtor6, , NonTrivialDefCtor1 t;)
 
-// FIXME: No core issue number yet.
-// - its parameter-declaration-clause is equivalent to that of an implicit declaration.
-struct NonTrivialDefCtor7 {
-  NonTrivialDefCtor7(...) = delete;
-};
-static_assert(!__has_trivial_constructor(NonTrivialDefCtor7), "");
-struct NonTrivialDefCtor8 {
-  NonTrivialDefCtor8(int = 0) = delete;
-};
-static_assert(!__has_trivial_constructor(NonTrivialDefCtor8), "");
-
 // Otherwise, the default constructor is non-trivial.
-
-class Trivial2 { Trivial2() = delete; };
+struct Trivial2 {
+  Trivial2(...) = delete;
+};
 static_assert(__has_trivial_constructor(Trivial2), "Trivial2 is trivial");
-
-class Trivial3 { Trivial3() = default; };
+struct Trivial3 {
+  Trivial3(int = 0) = delete;
+};
 static_assert(__has_trivial_constructor(Trivial3), "Trivial3 is trivial");
 
-template<typename T> class Trivial4 { Trivial4() = default; };
-static_assert(__has_trivial_constructor(Trivial4<int>), "Trivial4 is trivial");
+class Trivial4 { Trivial4() = delete; };
+static_assert(__has_trivial_constructor(Trivial4), "Trivial4 is trivial");
+
+class Trivial5 { Trivial5() = default; };
+static_assert(__has_trivial_constructor(Trivial5), "Trivial5 is trivial");
+
+template<typename T> class Trivial6 { Trivial6() = default; };
+static_assert(__has_trivial_constructor(Trivial6<int>), "Trivial6 is trivial");
 
-template<typename T> class Trivial5 { Trivial5() = delete; };
-static_assert(__has_trivial_constructor(Trivial5<int>), "Trivial5 is trivial");
+template<typename T> class Trivial7 { Trivial7() = delete; };
+static_assert(__has_trivial_constructor(Trivial7<int>), "Trivial7 is trivial");
 
 namespace PR14558 {
   // Ensure we determine whether an explicitly-defaulted or deleted special
   // member is trivial before we return to parsing the containing class.
   struct A {
     struct B { B() = default; } b;
     struct C { C() = delete; } c;
   };
 
   static_assert(__has_trivial_constructor(A), "");
   static_assert(__has_trivial_constructor(A::B), "");
 }