comex created this revision.
comex added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D74684
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/class/class.union/p1.cpp
clang/test/CXX/special/class.copy/p12-0x.cpp
clang/test/CXX/special/class.copy/p25-0x.cpp
clang/test/CXX/special/class.ctor/p5-0x.cpp
Index: clang/test/CXX/special/class.ctor/p5-0x.cpp
===================================================================
--- clang/test/CXX/special/class.ctor/p5-0x.cpp
+++ clang/test/CXX/special/class.ctor/p5-0x.cpp
@@ -175,30 +175,27 @@
// 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;
+// Otherwise, the default constructor is non-trivial.
+struct Trivial2 {
+ Trivial2(...) = delete;
};
-static_assert(!__has_trivial_constructor(NonTrivialDefCtor7), "");
-struct NonTrivialDefCtor8 {
- NonTrivialDefCtor8(int = 0) = delete;
+static_assert(__has_trivial_constructor(Trivial2), "Trivial2 is trivial");
+struct Trivial3 {
+ Trivial3(int = 0) = delete;
};
-static_assert(!__has_trivial_constructor(NonTrivialDefCtor8), "");
-
-// Otherwise, the default constructor is non-trivial.
+static_assert(__has_trivial_constructor(Trivial3), "Trivial3 is trivial");
-class Trivial2 { Trivial2() = delete; };
-static_assert(__has_trivial_constructor(Trivial2), "Trivial2 is trivial");
+class Trivial4 { Trivial4() = delete; };
+static_assert(__has_trivial_constructor(Trivial4), "Trivial4 is trivial");
-class Trivial3 { Trivial3() = default; };
-static_assert(__has_trivial_constructor(Trivial3), "Trivial3 is trivial");
+class Trivial5 { Trivial5() = default; };
+static_assert(__has_trivial_constructor(Trivial5), "Trivial5 is trivial");
-template<typename T> class Trivial4 { Trivial4() = default; };
-static_assert(__has_trivial_constructor(Trivial4<int>), "Trivial4 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
Index: clang/test/CXX/special/class.copy/p25-0x.cpp
===================================================================
--- clang/test/CXX/special/class.copy/p25-0x.cpp
+++ clang/test/CXX/special/class.copy/p25-0x.cpp
@@ -1,21 +1,31 @@
-// 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>;
@@ -26,12 +36,12 @@
};
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 {
@@ -47,7 +57,7 @@
struct TemplateCtor {
template<typename T> TemplateCtor operator=(T &);
};
-using _ = trivially_assignable<TemplateCtor>;
+using _ = trivially_assignable<TemplateCtor, ConstWithNonTrivialNonConstOverload>;
struct TemplateCtorMember {
TemplateCtor tc;
};
@@ -104,9 +114,9 @@
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 &&), "");
Index: clang/test/CXX/special/class.copy/p12-0x.cpp
===================================================================
--- clang/test/CXX/special/class.copy/p12-0x.cpp
+++ clang/test/CXX/special/class.copy/p12-0x.cpp
@@ -1,18 +1,27 @@
-// 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>;
@@ -23,12 +32,12 @@
};
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 {
@@ -44,7 +53,7 @@
struct TemplateCtor {
template<typename T> TemplateCtor(T &);
};
-using _ = trivially_copyable<TemplateCtor>;
+using _ = trivially_copyable<TemplateCtor, ConstWithNonTrivialNonConstOverload>;
struct TemplateCtorMember {
TemplateCtor tc;
};
Index: clang/test/CXX/class/class.union/p1.cpp
===================================================================
--- clang/test/CXX/class/class.union/p1.cpp
+++ clang/test/CXX/class/class.union/p1.cpp
@@ -91,9 +91,6 @@
} 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 {
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -9194,67 +9194,7 @@
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
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1802,10 +1802,6 @@
"%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">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
