Skip to content

Commit

Permalink
Revert "[SemaCXX] Implement CWG2137 (list-initialization from objects…
Browse files Browse the repository at this point in the history 
… of the same type) (#77768)"

This reverts commit 9247013. Causes compilation
errors on valid code, see
#77768 (comment).
  • Loading branch information
@alexfh
alexfh committed Jan 24, 2024
1 parent f6ca6ed commit 6e4930c
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 210 deletions.
40 changes: 10 additions & 30 deletions clang/lib/Sema/SemaInit.cpp
View file
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
/// aggregate type as the entity?
/// type as the entity?
static void TryConstructorInitialization(Sema &S,
const InitializedEntity &Entity,
const InitializationKind &Kind,
Expand Down Expand Up @@ -4230,14 +4230,6 @@ 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 @@ -4250,17 +4242,11 @@ static void TryConstructorInitialization(Sema &S,
if (S.getLangOpts().CPlusPlus17 && !RequireActualConstructor &&
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;
}
// Convert qualifications if necessary.
Sequence.AddQualificationConversionStep(DestType, VK_PRValue);
if (ILE)
Sequence.RewrapReferenceInitList(DestType, ILE);
return;
}

const RecordType *DestRecordType = DestType->getAs<RecordType>();
Expand Down Expand Up @@ -4305,12 +4291,6 @@ 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 @@ -4592,9 +4572,9 @@ static void TryListInitialization(Sema &S,
return;
}

// 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
// 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
// initialized from that element (by copy-initialization for
// copy-list-initialization, or by direct-initialization for
// direct-list-initialization).
Expand All @@ -4605,7 +4585,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() && DestType->isAggregateType()) {
if (DestType->isRecordType()) {
QualType InitType = InitList->getInit(0)->getType();
if (S.Context.hasSameUnqualifiedType(InitType, DestType) ||
S.IsDerivedFrom(InitList->getBeginLoc(), InitType, DestType)) {
Expand Down
38 changes: 10 additions & 28 deletions clang/lib/Sema/SemaOverload.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1568,37 +1568,19 @@ TryUserDefinedConversion(Sema &S, Expr *From, QualType ToType,
// called for those cases.
if (CXXConstructorDecl *Constructor
= dyn_cast<CXXConstructorDecl>(ICS.UserDefined.ConversionFunction)) {
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 FromCanon
= S.Context.getCanonicalType(From->getType().getUnqualifiedType());
QualType ToCanon
= S.Context.getCanonicalType(ToType).getUnqualifiedType();
if (Constructor->isCopyConstructor() &&
(FromCanon == ToCanon ||
S.IsDerivedFrom(FromLoc, FromCanon, ToCanon))) {
S.IsDerivedFrom(From->getBeginLoc(), 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(FromType);
ICS.Standard.setFromType(From->getType());
ICS.Standard.setAllToTypes(ToType);
ICS.Standard.CopyConstructor = Constructor;
ICS.Standard.FoundCopyConstructor = Found;
Expand Down Expand Up @@ -5324,18 +5306,18 @@ TryListConversion(Sema &S, InitListExpr *From, QualType ToType,
IsDesignatedInit)
return Result;

// 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.
// 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.
//
// 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() && ToType->isAggregateType()) {
if (ToType->isRecordType()) {
QualType InitType = From->getInit(0)->getType();
if (S.Context.hasSameUnqualifiedType(InitType, ToType) ||
S.IsDerivedFrom(From->getBeginLoc(), InitType, ToType))
Expand Down
10 changes: 10 additions & 0 deletions clang/test/CXX/drs/dr14xx.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,16 @@ 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: 0 additions & 45 deletions clang/test/CXX/drs/dr21xx.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,6 @@
// 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 @@ -142,41 +132,6 @@ 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: 0 additions & 85 deletions clang/test/CXX/drs/dr23xx.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,6 @@
// 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 @@ -57,81 +47,6 @@ 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
View file
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="unreleased" align="center">Clang 18</td>
<td class="unknown" align="center">Unknown</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 class="unreleased" align="center">Clang 18</td>
<td align="center">Not resolved</td>
</tr>
<tr id="2312">
<td><a href="https://cplusplus.github.io/CWG/issues/2312.html">2312</a></td>
Expand Down
21 changes: 1 addition & 20 deletions libcxx/test/std/utilities/utility/pairs/pairs.pair/ctor.pair_U_V_move.pass.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,26 +121,7 @@ int main(int, char**)
test_pair_rv<CopyOnly, CopyOnly&>();
test_pair_rv<CopyOnly, 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>();
test_pair_rv<ExplicitTypes::CopyOnly, ExplicitTypes::CopyOnly&, true, false>();
test_pair_rv<ExplicitTypes::CopyOnly, ExplicitTypes::CopyOnly&&, true, false>();

Expand Down

0 comments on commit 6e4930c

Please sign in to comment.