[clang-tools-extra] 5ea341d - [clang] Fix trivially copyable for copy constructor and copy assignment operator
Roy Jacobson via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 17 00:36:25 PDT 2022
Author: Javier Alvarez
Date: 2022-06-17T10:35:01+03:00
New Revision: 5ea341d7c4f9cc4933adc04ff74d59e26ad2f306
URL: https://github.com/llvm/llvm-project/commit/5ea341d7c4f9cc4933adc04ff74d59e26ad2f306
DIFF: https://github.com/llvm/llvm-project/commit/5ea341d7c4f9cc4933adc04ff74d59e26ad2f306.diff
LOG: [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
Added:
Modified:
clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-const.cpp
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/LangOptions.h
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/drs/dr21xx.cpp
clang/test/CXX/special/class.copy/p12-0x.cpp
clang/test/CXX/special/class.copy/p25-0x.cpp
clang/www/cxx_dr_status.html
Removed:
################################################################################
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-const.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-const.cpp
index fd3bddedd5675..beec7fae961a9 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-const.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-const.cpp
@@ -10,12 +10,12 @@ struct Str {
};
// This class is non-trivially copyable because the copy-constructor and copy
-// assignment take non-const references.
+// assignment take non-const references and are user-provided.
struct ModifiesRightSide {
ModifiesRightSide() = default;
- ModifiesRightSide(ModifiesRightSide &) = default;
+ ModifiesRightSide(ModifiesRightSide &);
bool operator<(ModifiesRightSide &) const;
- ModifiesRightSide &operator=(ModifiesRightSide &) = default;
+ ModifiesRightSide &operator=(ModifiesRightSide &);
};
template <typename T>
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ecf574b3b0f8d..e9ea54d591dad 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -391,7 +391,6 @@ AIX Support
``-mignore-xcoff-visibility`` option can be manually specified on the
command-line to recover the previous behavior if desired.
-
C Language Changes in Clang
---------------------------
@@ -478,6 +477,11 @@ ABI Changes in Clang
(e.g. ``int : 0``) no longer prevents the structure from being considered a
homogeneous floating-point or vector aggregate. The new behavior agrees with
the AAPCS specification, and matches the similar bug fix in GCC 12.1.
+- All copy constructors can now be trivial if they are not user-provided,
+ regardless of the type qualifiers of the argument of the defaulted constructor,
+ fixing dr2171.
+ You can switch back to the old ABI behavior with the flag:
+ ``-fclang-abi-compat=14.0``.
OpenMP Support in Clang
-----------------------
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index cf121de6699c8..f1b6cb60ae855 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -215,8 +215,12 @@ class LangOptions : public LangOptionsBase {
Ver12,
/// Attempt to be ABI-compatible with code generated by Clang 14.0.x.
- /// This causes clang to mangle dependent nested names incorrectly.
- /// This causes clang to pack non-POD members of packed structs.
+ /// This causes clang to:
+ /// - mangle dependent nested names incorrectly.
+ /// - pack non-POD members of packed structs.
+ /// - make trivial only those defaulted copy constructors with a
+ /// parameter-type-list equivalent to the parameter-type-list of an
+ /// implicit declaration.
Ver14,
/// Conform to the underlying platform's C and C++ ABIs as closely
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 6c8704cd0b789..ebeb98b2f701c 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -9772,11 +9772,22 @@ bool Sema::SpecialMemberIsTrivial(CXXMethodDecl *MD, CXXSpecialMember CSM,
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) {
+
+ // When ClangABICompat14 is true, CXX copy constructors will only be trivial
+ // if they are not user-provided and their parameter-type-list is equivalent
+ // to the parameter-type-list of an implicit declaration. This maintains the
+ // behavior before dr2171 was implemented.
+ //
+ // Otherwise, if ClangABICompat14 is false, All copy constructors can be
+ // trivial, if they are not user-provided, regardless of the qualifiers on
+ // the reference type.
+ const bool ClangABICompat14 = Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver14;
+ if (!RT ||
+ ((RT->getPointeeType().getCVRQualifiers() != Qualifiers::Const) &&
+ ClangABICompat14)) {
if (Diagnose)
Diag(Param0->getLocation(), diag::note_nontrivial_param_type)
<< Param0->getSourceRange() << Param0->getType()
@@ -9784,6 +9795,8 @@ bool Sema::SpecialMemberIsTrivial(CXXMethodDecl *MD, CXXSpecialMember CSM,
Context.getRecordType(RD).withConst());
return false;
}
+
+ ConstArg = RT->getPointeeType().isConstQualified();
break;
}
diff --git a/clang/test/CXX/drs/dr21xx.cpp b/clang/test/CXX/drs/dr21xx.cpp
index 62c56703195bc..a6277f799f695 100644
--- a/clang/test/CXX/drs/dr21xx.cpp
+++ b/clang/test/CXX/drs/dr21xx.cpp
@@ -140,6 +140,33 @@ namespace dr2170 { // dr2170: 9
#endif
}
+namespace dr2171 { // dr2171: 15
+#if __cplusplus >= 201103L
+
+struct NonConstCopy {
+ NonConstCopy(NonConstCopy &) = default;
+ NonConstCopy &operator=(NonConstCopy &) = default;
+};
+
+static_assert(__has_trivial_copy(NonConstCopy), "");
+static_assert(__is_trivially_constructible(NonConstCopy, NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, const NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy &&), "");
+
+static_assert(__has_trivial_assign(NonConstCopy), "");
+static_assert(__is_trivially_assignable(NonConstCopy &, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy &&), "");
+static_assert(__is_trivially_assignable(NonConstCopy &&, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy &&), "");
+
+#endif
+} // namespace dr2171
+
namespace dr2180 { // dr2180: yes
class A {
A &operator=(const A &); // expected-note 0-2{{here}}
diff --git a/clang/test/CXX/special/class.copy/p12-0x.cpp b/clang/test/CXX/special/class.copy/p12-0x.cpp
index a0ef49d9b64db..283e945ffc0da 100644
--- a/clang/test/CXX/special/class.copy/p12-0x.cpp
+++ b/clang/test/CXX/special/class.copy/p12-0x.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -std=c++11 -verify %s -Wno-defaulted-function-deleted
+// RUN: %clang_cc1 -std=c++11 -verify %s -Wno-defaulted-function-deleted -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14
// expected-no-diagnostics
@@ -28,7 +29,25 @@ using _ = not_trivially_copyable<UserProvided>;
struct NonConstCopy {
NonConstCopy(NonConstCopy &) = default;
};
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 14
+// Up until (and including) Clang 14, non-const copy constructors were not trivial because of dr2171
using _ = not_trivially_copyable<NonConstCopy>;
+#else
+// In the latest Clang version, all defaulted constructors are trivial, even if non-const, because
+// dr2171 is fixed.
+static_assert(__has_trivial_copy(NonConstCopy), "");
+static_assert(__is_trivially_constructible(NonConstCopy, NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, const NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy &&), "");
+
+struct DefaultedSpecialMembers {
+ DefaultedSpecialMembers(const DefaultedSpecialMembers &) = default;
+ DefaultedSpecialMembers(DefaultedSpecialMembers &) = default;
+ DefaultedSpecialMembers(DefaultedSpecialMembers &&) = default;
+};
+using _ = trivially_copyable<DefaultedSpecialMembers>;
+#endif
// class X has no virtual functions
struct VFn {
diff --git a/clang/test/CXX/special/class.copy/p25-0x.cpp b/clang/test/CXX/special/class.copy/p25-0x.cpp
index c7224aea95909..a95993229d1b1 100644
--- a/clang/test/CXX/special/class.copy/p25-0x.cpp
+++ b/clang/test/CXX/special/class.copy/p25-0x.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++11 -verify %s -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14
// expected-no-diagnostics
@@ -31,7 +32,30 @@ using _ = not_trivially_assignable<UserProvided>;
struct NonConstCopy {
NonConstCopy &operator=(NonConstCopy &) = default;
};
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 14
+// Up until (and including) Clang 14, non-const copy assignment operators were not trivial because
+// of dr2171
using _ = not_trivially_assignable<NonConstCopy>;
+#else
+// In the latest Clang version, all defaulted assignment operators are trivial, even if non-const,
+// because dr2171 is fixed
+static_assert(__has_trivial_assign(NonConstCopy), "");
+static_assert(__is_trivially_assignable(NonConstCopy &, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy &&), "");
+static_assert(__is_trivially_assignable(NonConstCopy &&, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy &&), "");
+
+struct DefaultedSpecialMembers {
+ DefaultedSpecialMembers &operator=(const DefaultedSpecialMembers &) = default;
+ DefaultedSpecialMembers &operator=(DefaultedSpecialMembers &) = default;
+ DefaultedSpecialMembers &operator=(DefaultedSpecialMembers &&) = default;
+};
+using _ = trivially_assignable<DefaultedSpecialMembers>;
+#endif
// class X has no virtual functions
struct VFn {
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index b9a38150063bc..e437da1bfda99 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -12840,7 +12840,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://wg21.link/cwg2171">2171</a></td>
<td>CD4</td>
<td>Triviality of copy constructor with less-qualified parameter</td>
- <td class="none" align="center">Unknown</td>
+ <td class="unreleased" align="center">Clang 15</td>
</tr>
<tr class="open" id="2172">
<td><a href="https://wg21.link/cwg2172">2172</a></td>
More information about the cfe-commits
mailing list