[libcxx-commits] [clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)
Mital Ashok via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Jan 11 05:35:46 PST 2024
https://github.com/MitalAshok created https://github.com/llvm/llvm-project/pull/77768
Closes #77638, #24186
Rebased from <https://reviews.llvm.org/D156032>, see there for more information.
Implements wording change in [CWG2137](https://wg21.link/CWG2137) in the first commit.
This also implements an approach to [CWG2311](https://wg21.link/CWG2311) in the second commit, because too much code that relies on `T{ T_prvalue }` being an elision would break. Because that issue is still open and the CWG issue doesn't provide wording to fix the issue, there may be different behaviours on other compilers.
>From 08e2db75625ff4494f352df70922174275d7c2dd Mon Sep 17 00:00:00 2001
From: Mital Ashok <mital at mitalashok.co.uk>
Date: Sat, 22 Jul 2023 20:07:00 +0100
Subject: [PATCH 1/2] [SemaCXX] Implement CWG2137 (list-initialization from
objects of the same type)
Differential Revision: https://reviews.llvm.org/D156032
---
clang/docs/ReleaseNotes.rst | 2 +
clang/lib/Sema/SemaInit.cpp | 14 +++----
clang/lib/Sema/SemaOverload.cpp | 38 +++++++++++++-----
clang/test/CXX/drs/dr14xx.cpp | 10 -----
clang/test/CXX/drs/dr21xx.cpp | 40 +++++++++++++++++++
clang/www/cxx_dr_status.html | 2 +-
.../pairs.pair/ctor.pair_U_V_move.pass.cpp | 21 +++++++++-
7 files changed, 98 insertions(+), 29 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 59732962caac65..a52fb805497756 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -199,6 +199,8 @@ C++2c Feature Support
Resolutions to C++ Defect Reports
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Implemented `CWG2137 <https://wg21.link/CWG2137>`_ which allows
+ list-initialization from objects of the same type.
C Language Changes
------------------
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 408ee5f775804b..a3a7486af093fb 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -4199,7 +4199,7 @@ static OverloadingResult ResolveConstructorOverload(
/// \param IsListInit Is this list-initialization?
/// \param IsInitListCopy Is this non-list-initialization resulting from a
/// list-initialization from {x} where x is the same
-/// type as the entity?
+/// aggregate type as the entity?
static void TryConstructorInitialization(Sema &S,
const InitializedEntity &Entity,
const InitializationKind &Kind,
@@ -4239,8 +4239,8 @@ static void TryConstructorInitialization(Sema &S,
// ObjC++: Lambda captured by the block in the lambda to block conversion
// should avoid copy elision.
if (S.getLangOpts().CPlusPlus17 && !RequireActualConstructor &&
- UnwrappedArgs.size() == 1 && UnwrappedArgs[0]->isPRValue() &&
- S.Context.hasSameUnqualifiedType(UnwrappedArgs[0]->getType(), DestType)) {
+ Args.size() == 1 && Args[0]->isPRValue() &&
+ S.Context.hasSameUnqualifiedType(Args[0]->getType(), DestType)) {
// Convert qualifications if necessary.
Sequence.AddQualificationConversionStep(DestType, VK_PRValue);
if (ILE)
@@ -4571,9 +4571,9 @@ static void TryListInitialization(Sema &S,
return;
}
- // C++11 [dcl.init.list]p3, per DR1467:
- // - If T is a class type and the initializer list has a single element of
- // type cv U, where U is T or a class derived from T, the object is
+ // C++11 [dcl.init.list]p3, per DR1467 and DR2137:
+ // - If T is an aggregate class and the initializer list has a single element
+ // of type cv U, where U is T or a class derived from T, the object is
// initialized from that element (by copy-initialization for
// copy-list-initialization, or by direct-initialization for
// direct-list-initialization).
@@ -4584,7 +4584,7 @@ static void TryListInitialization(Sema &S,
// - Otherwise, if T is an aggregate, [...] (continue below).
if (S.getLangOpts().CPlusPlus11 && InitList->getNumInits() == 1 &&
!IsDesignatedInit) {
- if (DestType->isRecordType()) {
+ if (DestType->isRecordType() && DestType->isAggregateType()) {
QualType InitType = InitList->getInit(0)->getType();
if (S.Context.hasSameUnqualifiedType(InitType, DestType) ||
S.IsDerivedFrom(InitList->getBeginLoc(), InitType, DestType)) {
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 64bc3851980272..30bc2036cf781c 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -1562,19 +1562,37 @@ TryUserDefinedConversion(Sema &S, Expr *From, QualType ToType,
// called for those cases.
if (CXXConstructorDecl *Constructor
= dyn_cast<CXXConstructorDecl>(ICS.UserDefined.ConversionFunction)) {
- QualType FromCanon
- = S.Context.getCanonicalType(From->getType().getUnqualifiedType());
+ QualType FromType;
+ SourceLocation FromLoc;
+ // C++11 [over.ics.list]p6, per DR2137:
+ // C++17 [over.ics.list]p6:
+ // If C is not an initializer-list constructor and the initializer list
+ // has a single element of type cv U, where U is X or a class derived
+ // from X, the implicit conversion sequence has Exact Match rank if U is
+ // X, or Conversion rank if U is derived from X.
+ if (const auto *InitList = dyn_cast<InitListExpr>(From);
+ InitList && InitList->getNumInits() == 1 &&
+ !S.isInitListConstructor(Constructor)) {
+ const Expr *SingleInit = InitList->getInit(0);
+ FromType = SingleInit->getType();
+ FromLoc = SingleInit->getBeginLoc();
+ } else {
+ FromType = From->getType();
+ FromLoc = From->getBeginLoc();
+ }
+ QualType FromCanon =
+ S.Context.getCanonicalType(FromType.getUnqualifiedType());
QualType ToCanon
= S.Context.getCanonicalType(ToType).getUnqualifiedType();
if (Constructor->isCopyConstructor() &&
(FromCanon == ToCanon ||
- S.IsDerivedFrom(From->getBeginLoc(), FromCanon, ToCanon))) {
+ S.IsDerivedFrom(FromLoc, FromCanon, ToCanon))) {
// Turn this into a "standard" conversion sequence, so that it
// gets ranked with standard conversion sequences.
DeclAccessPair Found = ICS.UserDefined.FoundConversionFunction;
ICS.setStandard();
ICS.Standard.setAsIdentityConversion();
- ICS.Standard.setFromType(From->getType());
+ ICS.Standard.setFromType(FromType);
ICS.Standard.setAllToTypes(ToType);
ICS.Standard.CopyConstructor = Constructor;
ICS.Standard.FoundCopyConstructor = Found;
@@ -5320,18 +5338,18 @@ TryListConversion(Sema &S, InitListExpr *From, QualType ToType,
IsDesignatedInit)
return Result;
- // Per DR1467:
- // If the parameter type is a class X and the initializer list has a single
- // element of type cv U, where U is X or a class derived from X, the
- // implicit conversion sequence is the one required to convert the element
- // to the parameter type.
+ // Per DR1467 and DR2137:
+ // If the parameter type is an aggregate class X and the initializer list
+ // has a single element of type cv U, where U is X or a class derived from
+ // X, the implicit conversion sequence is the one required to convert the
+ // element to the parameter type.
//
// Otherwise, if the parameter type is a character array [... ]
// and the initializer list has a single element that is an
// appropriately-typed string literal (8.5.2 [dcl.init.string]), the
// implicit conversion sequence is the identity conversion.
if (From->getNumInits() == 1 && !IsDesignatedInit) {
- if (ToType->isRecordType()) {
+ if (ToType->isRecordType() && ToType->isAggregateType()) {
QualType InitType = From->getInit(0)->getType();
if (S.Context.hasSameUnqualifiedType(InitType, ToType) ||
S.IsDerivedFrom(From->getBeginLoc(), InitType, ToType))
diff --git a/clang/test/CXX/drs/dr14xx.cpp b/clang/test/CXX/drs/dr14xx.cpp
index d262f6f9dcab79..4c29d03a6e117a 100644
--- a/clang/test/CXX/drs/dr14xx.cpp
+++ b/clang/test/CXX/drs/dr14xx.cpp
@@ -488,16 +488,6 @@ namespace dr1467 { // dr1467: 3.7 c++11
}
} // nonaggregate
- namespace SelfInitIsNotListInit {
- struct S {
- S();
- explicit S(S &);
- S(const S &);
- };
- S s1;
- S s2 = {s1}; // ok, not list-initialization so we pick the non-explicit constructor
- }
-
struct NestedInit { int a, b, c; };
NestedInit ni[1] = {{NestedInit{1, 2, 3}}};
diff --git a/clang/test/CXX/drs/dr21xx.cpp b/clang/test/CXX/drs/dr21xx.cpp
index a7e50df3f374be..9d2016262712d1 100644
--- a/clang/test/CXX/drs/dr21xx.cpp
+++ b/clang/test/CXX/drs/dr21xx.cpp
@@ -11,6 +11,16 @@
// cxx98-error at -1 {{variadic macros are a C99 feature}}
#endif
+namespace std {
+ __extension__ typedef __SIZE_TYPE__ size_t;
+
+ template<typename E> struct initializer_list {
+ const E *p; size_t n;
+ initializer_list(const E *p, size_t n);
+ initializer_list();
+ };
+}
+
namespace dr2100 { // dr2100: 12
template<const int *P, bool = true> struct X {};
template<typename T> struct A {
@@ -132,6 +142,36 @@ namespace dr2126 { // dr2126: 12
#endif
}
+namespace dr2137 { // dr2137: 18
+#if __cplusplus >= 201103L
+ struct Q {
+ Q();
+ Q(Q&&);
+ Q(std::initializer_list<Q>) = delete; // expected-note 2 {{has been explicitly marked deleted here}}
+ };
+
+ Q x = Q { Q() }; // expected-error {{call to deleted constructor}}
+
+ int f(Q); // expected-note {{passing argument to parameter here}}
+ int y = f({ Q() }); // expected-error {{call to deleted constructor}}
+
+ struct U {
+ U();
+ U(const U&);
+ };
+
+ struct Derived : U {
+ Derived();
+ Derived(const Derived&);
+ } d;
+
+ int g(Derived);
+ int g(U(&&)[1]) = delete;
+
+ int z = g({ d });
+#endif
+}
+
namespace dr2140 { // dr2140: 9
#if __cplusplus >= 201103L
union U { int a; decltype(nullptr) b; };
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 4a3ed19161f9a5..b336049ec464a3 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -12630,7 +12630,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/2137.html">2137</a></td>
<td>CD4</td>
<td>List-initialization from object of same type</td>
- <td class="unknown" align="center">Unknown</td>
+ <td class="unreleased" align="center">Clang 18</td>
</tr>
<tr id="2138">
<td><a href="https://cplusplus.github.io/CWG/issues/2138.html">2138</a></td>
diff --git a/libcxx/test/std/utilities/utility/pairs/pairs.pair/ctor.pair_U_V_move.pass.cpp b/libcxx/test/std/utilities/utility/pairs/pairs.pair/ctor.pair_U_V_move.pass.cpp
index 3b2d093eb34d49..30fdb19fd3aebe 100644
--- a/libcxx/test/std/utilities/utility/pairs/pairs.pair/ctor.pair_U_V_move.pass.cpp
+++ b/libcxx/test/std/utilities/utility/pairs/pairs.pair/ctor.pair_U_V_move.pass.cpp
@@ -121,7 +121,26 @@ int main(int, char**)
test_pair_rv<CopyOnly, CopyOnly&>();
test_pair_rv<CopyOnly, CopyOnly&&>();
- test_pair_rv<ExplicitTypes::CopyOnly, ExplicitTypes::CopyOnly>();
+ /* For ExplicitTypes::CopyOnly, two of the viable candidates for initializing from a non-const xvalue are:
+ * pair(const pair&); // (defaulted copy constructor)
+ * template<class U1, class U2> explicit pair(const pair<U1, U2>&&); [U1 = ExplicitTypes::CopyOnly, U2 = int]
+ * This results in diverging behavior for test_convertible which uses copy-list-initialization
+ * Prior to CWG2137, this would have selected the first (non-explicit) ctor as explicit ctors would not be considered
+ * Afterwards, it should select the second since it is a better match, and then failed because it is explicit
+ *
+ * This may change with future defect reports, and some compilers only have partial support for CWG2137,
+ * so use std::is_convertible directly to avoid a copy-list-initialization
+ */
+ {
+ using P1 = std::pair<ExplicitTypes::CopyOnly, int>;
+ using P2 = std::pair<int, ExplicitTypes::CopyOnly>;
+ using UP1 = std::pair<ExplicitTypes::CopyOnly, int>&&;
+ using UP2 = std::pair<int, ExplicitTypes::CopyOnly>&&;
+ static_assert(std::is_constructible<P1, UP1>::value, "");
+ static_assert(std::is_convertible<P1, UP1>::value, "");
+ static_assert(std::is_constructible<P2, UP2>::value, "");
+ static_assert(std::is_convertible<P2, UP2>::value, "");
+ }
test_pair_rv<ExplicitTypes::CopyOnly, ExplicitTypes::CopyOnly&, true, false>();
test_pair_rv<ExplicitTypes::CopyOnly, ExplicitTypes::CopyOnly&&, true, false>();
>From 01ea12bdbb5155240fd9106322011af38dafc329 Mon Sep 17 00:00:00 2001
From: Mital Ashok <mital at mitalashok.co.uk>
Date: Sun, 23 Jul 2023 18:17:18 +0100
Subject: [PATCH 2/2] [SemaCXX] Allow some copy elision of T{ T_prvalue } if an
initializer-list constructor isn't used (CWG2311)
Differential Revision: https://reviews.llvm.org/D156032
---
clang/docs/ReleaseNotes.rst | 3 ++
clang/lib/Sema/SemaInit.cpp | 34 +++++++++++---
clang/test/CXX/drs/dr23xx.cpp | 84 +++++++++++++++++++++++++++++++++++
clang/www/cxx_dr_status.html | 2 +-
4 files changed, 115 insertions(+), 8 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a52fb805497756..66d388bb954bda 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -201,6 +201,9 @@ Resolutions to C++ Defect Reports
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
- Implemented `CWG2137 <https://wg21.link/CWG2137>`_ which allows
list-initialization from objects of the same type.
+- Implemented `CWG2311 <https://wg21.link/CWG2311>`_: given a prvalue ``e`` of object type
+ ``T``, ``T{e}`` will try to resolve an initializer list constructor and will use it if successful (CWG2137).
+ Otherwise, if there is no initializer list constructor, the copy will be elided as if it was ``T(e)``.
C Language Changes
------------------
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index a3a7486af093fb..2822a8d5d78b3e 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -4229,6 +4229,14 @@ static void TryConstructorInitialization(Sema &S,
Entity.getKind() !=
InitializedEntity::EK_LambdaToBlockConversionBlockElement);
+ bool CopyElisionPossible = false;
+ auto ElideConstructor = [&] {
+ // Convert qualifications if necessary.
+ Sequence.AddQualificationConversionStep(DestType, VK_PRValue);
+ if (ILE)
+ Sequence.RewrapReferenceInitList(DestType, ILE);
+ };
+
// C++17 [dcl.init]p17:
// - If the initializer expression is a prvalue and the cv-unqualified
// version of the source type is the same class as the class of the
@@ -4239,13 +4247,19 @@ static void TryConstructorInitialization(Sema &S,
// ObjC++: Lambda captured by the block in the lambda to block conversion
// should avoid copy elision.
if (S.getLangOpts().CPlusPlus17 && !RequireActualConstructor &&
- Args.size() == 1 && Args[0]->isPRValue() &&
- S.Context.hasSameUnqualifiedType(Args[0]->getType(), DestType)) {
- // Convert qualifications if necessary.
- Sequence.AddQualificationConversionStep(DestType, VK_PRValue);
- if (ILE)
- Sequence.RewrapReferenceInitList(DestType, ILE);
- return;
+ UnwrappedArgs.size() == 1 && UnwrappedArgs[0]->isPRValue() &&
+ S.Context.hasSameUnqualifiedType(UnwrappedArgs[0]->getType(), DestType)) {
+ if (ILE && !DestType->isAggregateType()) {
+ // CWG2311: T{ prvalue_of_type_T } is not eligible for copy elision
+ // Make this an elision if this won't call an initializer-list
+ // constructor. (Always on an aggregate type or check constructors first.)
+ assert(!IsInitListCopy &&
+ "IsInitListCopy only possible with aggregate types");
+ CopyElisionPossible = true;
+ } else {
+ ElideConstructor();
+ return;
+ }
}
const RecordType *DestRecordType = DestType->getAs<RecordType>();
@@ -4290,6 +4304,12 @@ static void TryConstructorInitialization(Sema &S,
S, Kind.getLocation(), Args, CandidateSet, DestType, Ctors, Best,
CopyInitialization, AllowExplicit,
/*OnlyListConstructors=*/true, IsListInit, RequireActualConstructor);
+
+ if (CopyElisionPossible && Result == OR_No_Viable_Function) {
+ // No initializer list candidate
+ ElideConstructor();
+ return;
+ }
}
// C++11 [over.match.list]p1:
diff --git a/clang/test/CXX/drs/dr23xx.cpp b/clang/test/CXX/drs/dr23xx.cpp
index d2f4e7652ab568..8aa04c917846fa 100644
--- a/clang/test/CXX/drs/dr23xx.cpp
+++ b/clang/test/CXX/drs/dr23xx.cpp
@@ -6,6 +6,16 @@
// RUN: %clang_cc1 -std=c++23 %s -verify=expected,since-cxx11,since-cxx14,since-cxx17,since-cxx20 -fexceptions -fcxx-exceptions -pedantic-errors 2>&1 | FileCheck %s
// RUN: %clang_cc1 -std=c++2c %s -verify=expected,since-cxx11,since-cxx14,since-cxx17,since-cxx20 -fexceptions -fcxx-exceptions -pedantic-errors 2>&1 | FileCheck %s
+namespace std {
+ __extension__ typedef __SIZE_TYPE__ size_t;
+
+ template<typename E> struct initializer_list {
+ const E *p; size_t n;
+ initializer_list(const E *p, size_t n);
+ initializer_list();
+ };
+}
+
#if __cplusplus >= 201103L
namespace dr2303 { // dr2303: 12
template <typename... T>
@@ -47,6 +57,80 @@ void g() {
} //namespace dr2303
#endif
+namespace dr2311 { // dr2311: 18 open
+#if __cplusplus >= 201707L
+template<typename T>
+void test() {
+ // Ensure none of these expressions try to call a move constructor.
+ T a = T{T(0)};
+ T b{T(0)};
+ auto c{T(0)};
+ T d = {T(0)};
+ auto e = {T(0)};
+#if __cplusplus >= 202302L
+ auto f = auto{T(0)};
+#endif
+ void(*fn)(T);
+ fn({T(0)});
+}
+
+struct NonMovable {
+ NonMovable(int);
+ NonMovable(NonMovable&&) = delete;
+};
+struct NonMovableNonApplicableIList {
+ NonMovableNonApplicableIList(int);
+ NonMovableNonApplicableIList(NonMovableNonApplicableIList&&) = delete;
+ NonMovableNonApplicableIList(std::initializer_list<int>);
+};
+struct ExplicitMovable {
+ ExplicitMovable(int);
+ explicit ExplicitMovable(ExplicitMovable&&);
+};
+struct ExplicitNonMovable {
+ ExplicitNonMovable(int);
+ explicit ExplicitNonMovable(ExplicitNonMovable&&) = delete;
+};
+struct ExplicitNonMovableNonApplicableIList {
+ ExplicitNonMovableNonApplicableIList(int);
+ explicit ExplicitNonMovableNonApplicableIList(ExplicitNonMovableNonApplicableIList&&) = delete;
+ ExplicitNonMovableNonApplicableIList(std::initializer_list<int>);
+};
+struct CopyOnly {
+ CopyOnly(int);
+ CopyOnly(const CopyOnly&);
+ CopyOnly(CopyOnly&&) = delete;
+};
+struct ExplicitCopyOnly {
+ ExplicitCopyOnly(int);
+ explicit ExplicitCopyOnly(const ExplicitCopyOnly&);
+ explicit ExplicitCopyOnly(ExplicitCopyOnly&&) = delete;
+};
+
+template void test<NonMovable>();
+template void test<NonMovableNonApplicableIList>();
+template void test<ExplicitMovable>();
+template void test<ExplicitNonMovable>();
+template void test<ExplicitNonMovableNonApplicableIList>();
+template void test<CopyOnly>();
+template void test<ExplicitCopyOnly>();
+
+struct any {
+ template<typename T>
+ any(T&&);
+};
+
+template<typename T>
+struct X {
+ X();
+ X(T) = delete; // expected-note {{'X' has been explicitly marked deleted here}}
+};
+
+X<std::initializer_list<any>> x{ X<std::initializer_list<any>>() }; // expected-error {{call to deleted constructor of 'X<std::initializer_list<any>>'}}
+
+#endif
+}
+
// dr2331: na
#if __cplusplus >= 201103L
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index b336049ec464a3..391c611480820c 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -13674,7 +13674,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/2311.html">2311</a></td>
<td>open</td>
<td>Missed case for guaranteed copy elision</td>
- <td align="center">Not resolved</td>
+ <td class="unreleased" align="center">Clang 18</td>
</tr>
<tr id="2312">
<td><a href="https://cplusplus.github.io/CWG/issues/2312.html">2312</a></td>
More information about the libcxx-commits
mailing list