Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) #77768

Merged
merged 2 commits into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,11 @@ 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.
- 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)``.

- Implemented `CWG2598 <https://wg21.link/CWG2598>`_ and `CWG2096 <https://wg21.link/CWG2096>`_,
making unions (that have either no members or at least one literal member) literal types.
Expand Down
40 changes: 30 additions & 10 deletions clang/lib/Sema/SemaInit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4200,7 +4200,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,
Expand Down Expand Up @@ -4230,6 +4230,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
Expand All @@ -4242,11 +4250,17 @@ static void TryConstructorInitialization(Sema &S,
if (S.getLangOpts().CPlusPlus17 && !RequireActualConstructor &&
UnwrappedArgs.size() == 1 && UnwrappedArgs[0]->isPRValue() &&
S.Context.hasSameUnqualifiedType(UnwrappedArgs[0]->getType(), DestType)) {
// Convert qualifications if necessary.
Sequence.AddQualificationConversionStep(DestType, VK_PRValue);
if (ILE)
Sequence.RewrapReferenceInitList(DestType, ILE);
return;
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>();
Expand Down Expand Up @@ -4291,6 +4305,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:
Expand Down Expand Up @@ -4572,9 +4592,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).
Expand All @@ -4585,7 +4605,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)) {
Expand Down
38 changes: 28 additions & 10 deletions clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1568,19 +1568,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());
Comment on lines +1579 to +1590
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should only do that if we find a single overload, ie, should we check that Conversions.size() == 1 ?

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;
Expand Down Expand Up @@ -5306,18 +5324,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))
Expand Down
10 changes: 0 additions & 10 deletions clang/test/CXX/drs/dr14xx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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}}};

Expand Down
45 changes: 45 additions & 0 deletions clang/test/CXX/drs/dr21xx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,16 @@
// cxx98-error@-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 {
Expand Down Expand Up @@ -132,6 +142,41 @@ namespace dr2126 { // dr2126: 12
#endif
}

namespace dr2137 { // dr2137: 18
#if __cplusplus >= 201103L
struct Q {
Q();
Q(Q&&);
Q(std::initializer_list<Q>) = delete; // #dr2137-Qcons
};

Q x = Q { Q() };
// since-cxx11-error@-1 {{call to deleted constructor of 'Q'}}
// since-cxx11-note@#dr2137-Qcons {{'Q' has been explicitly marked deleted here}}

int f(Q); // #dr2137-f
int y = f({ Q() });
// since-cxx11-error@-1 {{call to deleted constructor of 'Q'}}
// since-cxx11-note@#dr2137-Qcons {{'Q' has been explicitly marked deleted here}}
// since-cxx11-note@#dr2137-f {{passing argument to parameter here}}

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; };
Expand Down
85 changes: 85 additions & 0 deletions clang/test/CXX/drs/dr23xx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -47,6 +57,81 @@ void g() {
} //namespace dr2303
#endif

namespace dr2311 { // dr2311: 18 open
#if __cplusplus >= 201707L
template<typename T>
void test() {
// Ensure none of these 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; // #dr2311-X
};

X<std::initializer_list<any>> x{ X<std::initializer_list<any>>() };
// since-cxx17-error@-1 {{call to deleted constructor of 'X<std::initializer_list<any>>'}}
// since-cxx17-note@#dr2311-X {{'X' has been explicitly marked deleted here}}
#endif
}

// dr2331: na
// dr2335 is in dr2335.cxx

Expand Down
4 changes: 2 additions & 2 deletions clang/www/cxx_dr_status.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>();

Expand Down