Skip to content

Commit 5ea341d

Browse files
Javier-varezroyjacobson
authored andcommitted
[clang] Fix trivially copyable for copy constructor and copy assignment operator
From [class.copy.ctor]: ``` A non-template constructor for class X is a copy constructor if its first parameter is of type X&, const X&, volatile X& or const volatile X&, and either there are no other parameters or else all other parameters have default arguments (9.3.4.7). A copy/move constructor for class X is trivial if it is not user-provided and if: - class X has no virtual functions (11.7.3) and no virtual base classes (11.7.2), and - the constructor selected to copy/move each direct base class subobject is trivial, and - or each non-static data member of X that is of class type (or array thereof), the constructor selected to copy/move that member is trivial; otherwise the copy/move constructor is non-trivial. ``` So `T(T&) = default`; should be trivial assuming that the previous provisions are met. This works in GCC, but not in Clang at the moment: https://godbolt.org/z/fTGe71b6P Reviewed By: royjacobson Differential Revision: https://reviews.llvm.org/D127593
1 parent 880ac51 commit 5ea341d

File tree

8 files changed

+101
-10
lines changed

8 files changed

+101
-10
lines changed

clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-const.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,12 @@ struct Str {
1010
};
1111

1212
// This class is non-trivially copyable because the copy-constructor and copy
13-
// assignment take non-const references.
13+
// assignment take non-const references and are user-provided.
1414
struct ModifiesRightSide {
1515
ModifiesRightSide() = default;
16-
ModifiesRightSide(ModifiesRightSide &) = default;
16+
ModifiesRightSide(ModifiesRightSide &);
1717
bool operator<(ModifiesRightSide &) const;
18-
ModifiesRightSide &operator=(ModifiesRightSide &) = default;
18+
ModifiesRightSide &operator=(ModifiesRightSide &);
1919
};
2020

2121
template <typename T>

clang/docs/ReleaseNotes.rst

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,6 @@ AIX Support
391391
``-mignore-xcoff-visibility`` option can be manually specified on the
392392
command-line to recover the previous behavior if desired.
393393

394-
395394
C Language Changes in Clang
396395
---------------------------
397396

@@ -478,6 +477,11 @@ ABI Changes in Clang
478477
(e.g. ``int : 0``) no longer prevents the structure from being considered a
479478
homogeneous floating-point or vector aggregate. The new behavior agrees with
480479
the AAPCS specification, and matches the similar bug fix in GCC 12.1.
480+
- All copy constructors can now be trivial if they are not user-provided,
481+
regardless of the type qualifiers of the argument of the defaulted constructor,
482+
fixing dr2171.
483+
You can switch back to the old ABI behavior with the flag:
484+
``-fclang-abi-compat=14.0``.
481485

482486
OpenMP Support in Clang
483487
-----------------------

clang/include/clang/Basic/LangOptions.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,12 @@ class LangOptions : public LangOptionsBase {
215215
Ver12,
216216

217217
/// Attempt to be ABI-compatible with code generated by Clang 14.0.x.
218-
/// This causes clang to mangle dependent nested names incorrectly.
219-
/// This causes clang to pack non-POD members of packed structs.
218+
/// This causes clang to:
219+
/// - mangle dependent nested names incorrectly.
220+
/// - pack non-POD members of packed structs.
221+
/// - make trivial only those defaulted copy constructors with a
222+
/// parameter-type-list equivalent to the parameter-type-list of an
223+
/// implicit declaration.
220224
Ver14,
221225

222226
/// Conform to the underlying platform's C and C++ ABIs as closely

clang/lib/Sema/SemaDeclCXX.cpp

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9772,18 +9772,31 @@ bool Sema::SpecialMemberIsTrivial(CXXMethodDecl *MD, CXXSpecialMember CSM,
97729772

97739773
case CXXCopyConstructor:
97749774
case CXXCopyAssignment: {
9775-
// Trivial copy operations always have const, non-volatile parameter types.
9776-
ConstArg = true;
97779775
const ParmVarDecl *Param0 = MD->getParamDecl(0);
97789776
const ReferenceType *RT = Param0->getType()->getAs<ReferenceType>();
9779-
if (!RT || RT->getPointeeType().getCVRQualifiers() != Qualifiers::Const) {
9777+
9778+
// When ClangABICompat14 is true, CXX copy constructors will only be trivial
9779+
// if they are not user-provided and their parameter-type-list is equivalent
9780+
// to the parameter-type-list of an implicit declaration. This maintains the
9781+
// behavior before dr2171 was implemented.
9782+
//
9783+
// Otherwise, if ClangABICompat14 is false, All copy constructors can be
9784+
// trivial, if they are not user-provided, regardless of the qualifiers on
9785+
// the reference type.
9786+
const bool ClangABICompat14 = Context.getLangOpts().getClangABICompat() <=
9787+
LangOptions::ClangABI::Ver14;
9788+
if (!RT ||
9789+
((RT->getPointeeType().getCVRQualifiers() != Qualifiers::Const) &&
9790+
ClangABICompat14)) {
97809791
if (Diagnose)
97819792
Diag(Param0->getLocation(), diag::note_nontrivial_param_type)
97829793
<< Param0->getSourceRange() << Param0->getType()
97839794
<< Context.getLValueReferenceType(
97849795
Context.getRecordType(RD).withConst());
97859796
return false;
97869797
}
9798+
9799+
ConstArg = RT->getPointeeType().isConstQualified();
97879800
break;
97889801
}
97899802

clang/test/CXX/drs/dr21xx.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,33 @@ namespace dr2170 { // dr2170: 9
140140
#endif
141141
}
142142

143+
namespace dr2171 { // dr2171: 15
144+
#if __cplusplus >= 201103L
145+
146+
struct NonConstCopy {
147+
NonConstCopy(NonConstCopy &) = default;
148+
NonConstCopy &operator=(NonConstCopy &) = default;
149+
};
150+
151+
static_assert(__has_trivial_copy(NonConstCopy), "");
152+
static_assert(__is_trivially_constructible(NonConstCopy, NonConstCopy &), "");
153+
static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy), "");
154+
static_assert(!__is_trivially_constructible(NonConstCopy, const NonConstCopy &), "");
155+
static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy &&), "");
156+
157+
static_assert(__has_trivial_assign(NonConstCopy), "");
158+
static_assert(__is_trivially_assignable(NonConstCopy &, NonConstCopy &), "");
159+
static_assert(!__is_trivially_assignable(NonConstCopy &, const NonConstCopy &), "");
160+
static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy), "");
161+
static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy &&), "");
162+
static_assert(__is_trivially_assignable(NonConstCopy &&, NonConstCopy &), "");
163+
static_assert(!__is_trivially_assignable(NonConstCopy &&, const NonConstCopy &), "");
164+
static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy), "");
165+
static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy &&), "");
166+
167+
#endif
168+
} // namespace dr2171
169+
143170
namespace dr2180 { // dr2180: yes
144171
class A {
145172
A &operator=(const A &); // expected-note 0-2{{here}}

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// RUN: %clang_cc1 -std=c++11 -verify %s -Wno-defaulted-function-deleted
2+
// RUN: %clang_cc1 -std=c++11 -verify %s -Wno-defaulted-function-deleted -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14
23

34
// expected-no-diagnostics
45

@@ -28,7 +29,25 @@ using _ = not_trivially_copyable<UserProvided>;
2829
struct NonConstCopy {
2930
NonConstCopy(NonConstCopy &) = default;
3031
};
32+
#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 14
33+
// Up until (and including) Clang 14, non-const copy constructors were not trivial because of dr2171
3134
using _ = not_trivially_copyable<NonConstCopy>;
35+
#else
36+
// In the latest Clang version, all defaulted constructors are trivial, even if non-const, because
37+
// dr2171 is fixed.
38+
static_assert(__has_trivial_copy(NonConstCopy), "");
39+
static_assert(__is_trivially_constructible(NonConstCopy, NonConstCopy &), "");
40+
static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy), "");
41+
static_assert(!__is_trivially_constructible(NonConstCopy, const NonConstCopy &), "");
42+
static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy &&), "");
43+
44+
struct DefaultedSpecialMembers {
45+
DefaultedSpecialMembers(const DefaultedSpecialMembers &) = default;
46+
DefaultedSpecialMembers(DefaultedSpecialMembers &) = default;
47+
DefaultedSpecialMembers(DefaultedSpecialMembers &&) = default;
48+
};
49+
using _ = trivially_copyable<DefaultedSpecialMembers>;
50+
#endif
3251

3352
// class X has no virtual functions
3453
struct VFn {

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// RUN: %clang_cc1 -std=c++11 -verify %s
2+
// RUN: %clang_cc1 -std=c++11 -verify %s -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14
23

34
// expected-no-diagnostics
45

@@ -31,7 +32,30 @@ using _ = not_trivially_assignable<UserProvided>;
3132
struct NonConstCopy {
3233
NonConstCopy &operator=(NonConstCopy &) = default;
3334
};
35+
#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 14
36+
// Up until (and including) Clang 14, non-const copy assignment operators were not trivial because
37+
// of dr2171
3438
using _ = not_trivially_assignable<NonConstCopy>;
39+
#else
40+
// In the latest Clang version, all defaulted assignment operators are trivial, even if non-const,
41+
// because dr2171 is fixed
42+
static_assert(__has_trivial_assign(NonConstCopy), "");
43+
static_assert(__is_trivially_assignable(NonConstCopy &, NonConstCopy &), "");
44+
static_assert(!__is_trivially_assignable(NonConstCopy &, const NonConstCopy &), "");
45+
static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy), "");
46+
static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy &&), "");
47+
static_assert(__is_trivially_assignable(NonConstCopy &&, NonConstCopy &), "");
48+
static_assert(!__is_trivially_assignable(NonConstCopy &&, const NonConstCopy &), "");
49+
static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy), "");
50+
static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy &&), "");
51+
52+
struct DefaultedSpecialMembers {
53+
DefaultedSpecialMembers &operator=(const DefaultedSpecialMembers &) = default;
54+
DefaultedSpecialMembers &operator=(DefaultedSpecialMembers &) = default;
55+
DefaultedSpecialMembers &operator=(DefaultedSpecialMembers &&) = default;
56+
};
57+
using _ = trivially_assignable<DefaultedSpecialMembers>;
58+
#endif
3559

3660
// class X has no virtual functions
3761
struct VFn {

clang/www/cxx_dr_status.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12840,7 +12840,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
1284012840
<td><a href="https://wg21.link/cwg2171">2171</a></td>
1284112841
<td>CD4</td>
1284212842
<td>Triviality of copy constructor with less-qualified parameter</td>
12843-
<td class="none" align="center">Unknown</td>
12843+
<td class="unreleased" align="center">Clang 15</td>
1284412844
</tr>
1284512845
<tr class="open" id="2172">
1284612846
<td><a href="https://wg21.link/cwg2172">2172</a></td>

0 commit comments

Comments
 (0)