Implement CWG2137 (list-initialization from objects of the same type)

[MitalAshok]

Summary

CWG2137 summary: A non-aggregate class being initialized with an initializer list with a single element (previously changed by CWG1467) should prefer initializer-list constructors over copy constructors, like everywhere else.

[MitalAshok]

clang-format

[MitalAshok]

clang/lib/Sema/SemaInit.cpp

4258–4259

This change unfortunately exposes the still-open CWG2311 but allows T{ object_of_type_T } to consider user declared constructors.

I am working on a separate fix for CWG2311 (Consider constructors as below, but then if the chosen constructor is not an initializer-list constructor, elide it).

clang/lib/Sema/SemaOverload.cpp

1470–1480

This allows with struct T { T(const T&); } t; void f(T); for f({ t }) is an exact match standard conversion instead of a user-defined conversion

[MitalAshok]

Removing "SelfInitIsNotListInit" test case: copy-list-init for non-aggregate classes remains as copy-list-inits, only aggregates copies are converted into copy-inits (which I don't think is observable)

[MitalAshok]

clang/test/CXX/drs/dr14xx.cpp

433

This relies on the old wording. "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)".

"If T is a class type" -> "If T is an aggregate class type", so this copy-list-initialization remains copy-list-initialization (and should fail for picking the explicit constructor)

[MitalAshok]

clang/lib/Sema/SemaInit.cpp

4258–4259

Fix will be here: https://reviews.llvm.org/D156062

auto{ prvalue } will elide a copy for aggregate types again. It will do so after checking constructors for non-aggregate classes now.

[MitalAshok]

Add CWG2311 fix too

Too much code and tests rely on elision of T{ T_prvalue } in C++17, so add change to allow that to be elided when this does not pick a initializer-list constructor.

[MitalAshok]

Target clang 18 instead; add entry to release notes

[aaron.ballman]

Adding some more reviewers to hopefully get this unstuck.

[aaron.ballman]

Btw, it looks like precommit CI may have found issues in libc++ related to your changes.

[cor3ntin]

clang/lib/Sema/SemaOverload.cpp

1470

Can you specify an exact C++ version in the comment? - I am assuming C++23

clang/test/CXX/drs/dr23xx.cpp

9

50

the dr status html page is generated automatically by leaving a magic comment here - and then running clang/www/make_cxx_dr_status

54

[cor3ntin]

clang/lib/Sema/SemaInit.cpp

4329–4330

I think this needs rephrasing. What ambiguity are you referring to?

4329–4330

[MitalAshok]

Fix broken libc++ pair constructor test

[MitalAshok]

It seems there were two places which relied on T x = { xvalue_of_type_T } being a copy-initialization: The test at https://reviews.llvm.org/D156032#inline-1509544 and that one pair constructor test.

GCC seems to allow it but I don't think that's standard. I also have a pretty short patch to implement this behaviour if we decide that it needs to be supported (maybe this is a standard defect?)

clang/test/CXX/drs/dr23xx.cpp

9

Wouldn't work in c++98 mode this file is being run in. I've copied this from other tests in this directory. I guess I can put a #if __cplusplus >= 201103L instead?

50

CWG2311 is still open, but clang did support list-initialization elision before this patch. The committee still needs to decide how exactly this elision is going to work, which might be different from how this patch implements it, so hesitant to mark this as done.

[Mordante]

I only looked at the libc++ changes. The libc++ AIX CI failures are unrelated to your patch. LGTM modulo one nit.

libcxx/test/std/utilities/utility/pairs/pairs.pair/ctor.pair_U_V_move.pass.cpp

134 ↗

(On Diff #549145)

Please remove this line.

[aaron.ballman]

clang/lib/Sema/SemaInit.cpp

4258–4259

As this was abandoned in favor of doing the fix here, the patch title, summary, and release notes should all be updated to talk about CWG2311.

4261–4262

4332

4335

4340

clang/test/CXX/drs/dr23xx.cpp

9

The current form seems fine to me.

50

CC @Endill -- do we have a special marking for "issue still open in WG21, but we implement the current suggested resolution?"

54

[cor3ntin]

clang/test/CXX/drs/dr23xx.cpp

50

18 open sould work

[MitalAshok]

Address comments; better implementation for elision (check after considering only initializer list constructors)

[MitalAshok]

rebased onto fast-forwarded main branch

(trying to fix seemingly unrelated CI build failure)

[MitalAshok]

Remove accidental unrelated changelog

[cor3ntin]

LGTM modulo one nit.
@hubert.reinterpretcast If you want to make sure this is conforming

clang/docs/ReleaseNotes.rst

73

clang/docs/ReleaseNotes.rst

 C++23 Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 
 C++2c Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 
 Resolutions to C++ Defect Reports
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Implemented `DR2137 <https://wg21.link/CWG2137>`_ which allows
+  list-initialization from objects of the same type.
 
 C Language Changes
 ------------------
 
 C2x Feature Support
 ^^^^^^^^^^^^^^^^^^^
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------

clang/lib/Sema/SemaInit.cpp

MultiExprArg UnwrappedArgs =
       ILE ? MultiExprArg(ILE->getInits(), ILE->getNumInits()) : Args;
 
   // The type we're constructing needs to be complete.
   if (!S.isCompleteType(Kind.getLocation(), DestType)) {
     Sequence.setIncompleteTypeFailure(DestType);
     return;
   }
 
+  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
   //       destination, the initializer expression is used to initialize the
   //       destination object.
   // Per DR (no number yet), this does not apply when initializing a base
   // class or delegating to another constructor from a mem-initializer.
   // ObjC++: Lambda captured by the block in the lambda to block conversion
   // should avoid copy elision.
   if (S.getLangOpts().CPlusPlus17 &&
       Entity.getKind() != InitializedEntity::EK_Base &&
       Entity.getKind() != InitializedEntity::EK_Delegating &&
       Entity.getKind() !=
           InitializedEntity::EK_LambdaToBlockConversionBlockElement &&
       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);
+    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)
+      CopyElisionPossible = true;
+    } else {
+      ElideConstructor();
       return;
     }
+  }
 
   const RecordType *DestRecordType = DestType->getAs<RecordType>();
   assert(DestRecordType && "Constructor initialization requires record type");
   CXXRecordDecl *DestRecordDecl
     = cast<CXXRecordDecl>(DestRecordType->getDecl());
 
   // Build the candidate set directly in the initialization sequence
   // structure, so that it will persist if we fail.
if (Result == OR_No_Viable_Function) {
     AsInitializerList = false;
     Result = ResolveConstructorOverload(S, Kind.getLocation(), UnwrappedArgs,
                                         CandidateSet, DestType, Ctors, Best,
                                         CopyInitialization, AllowExplicit,
                                         /*OnlyListConstructors=*/false,
                                         IsListInit);
   }
   if (Result) {
+    // Ambiguity means an initializer list constructor would not be called
+    // Or a deleted non-initializer-list constructor was to be called
+    if (CopyElisionPossible &&
+        (Result != OR_Deleted || !S.isInitListConstructor(Best->Function))) {
+      ElideConstructor();
+      return;
+    }
+
     Sequence.SetOverloadFailure(
         IsListInit ? InitializationSequence::FK_ListConstructorOverloadFailed
                    : InitializationSequence::FK_ConstructorOverloadFailed,
         Result);
 
     if (Result != OR_Deleted)
       return;
   }
 
   bool HadMultipleCandidates = (CandidateSet.size() > 1);
 
   // In C++17, ResolveConstructorOverload can select a conversion function
   // instead of a constructor.
   if (auto *CD = dyn_cast<CXXConversionDecl>(Best->Function)) {
     // Add the user-defined conversion step that calls the conversion function.
     QualType ConvType = CD->getConversionType();
     assert(S.Context.hasSameUnqualifiedType(ConvType, DestType) &&
            "should not have selected this conversion function");
+    assert(!CopyElisionPossible &&
+           "conversion function picked converting to the same type?");
     Sequence.AddUserConversionStep(CD, Best->FoundDecl, ConvType,
                                    HadMultipleCandidates);
     if (!S.Context.hasSameType(ConvType, DestType))
       Sequence.AddQualificationConversionStep(DestType, VK_PRValue);
     if (IsListInit)
       Sequence.RewrapReferenceInitList(Entity.getType(), ILE);
     return;
   }
 
   CXXConstructorDecl *CtorDecl = cast<CXXConstructorDecl>(Best->Function);
+  if (CopyElisionPossible && !S.isInitListConstructor(CtorDecl)) {
+    ElideConstructor();
+    return;
+  }
   if (Result != OR_Deleted) {
     // C++11 [dcl.init]p6:
     //   If a program calls for the default initialization of an object
     //   of a const-qualified type T, T shall be a class type with a
     //   user-provided default constructor.
     // C++ core issue 253 proposal:
     //   If the implicit default constructor initializes all subobjects, no
     //   initializer should be required.
static void TryListInitialization(Sema &S,
   // as a tentative DR resolution.
   bool IsDesignatedInit = InitList->hasDesignatedInit();
   if (!DestType->isAggregateType() && IsDesignatedInit) {
     Sequence.SetFailed(
         InitializationSequence::FK_DesignatedInitForNonAggregate);
     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).
   // - Otherwise, if T 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]), initialization is performed as described
   //   in that section.
   // - 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)) {
         Expr *InitListAsExpr = InitList;
         TryConstructorInitialization(S, Entity, Kind, InitListAsExpr, DestType,
                                      DestType, Sequence,
                                      /*InitListSyntax*/false,
                                      /*IsInitListCopy*/true);

clang/lib/Sema/SemaOverload.cpp

case OR_Deleted:
     //   A conversion of an expression of class type to the same class
     //   type is given Exact Match rank, and a conversion of an
     //   expression of class type to a base class of that type is
     //   given Conversion rank, in spite of the fact that a copy
     //   constructor (i.e., a user-defined conversion function) is
     //   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++ [over.ics.list]p6, per DR2137:
+      //   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;
         if (ToCanon != FromCanon)
           ICS.Standard.Second = ICK_Derived_To_Base;
       }
     }
     break;
TryListConversion(Sema &S, InitListExpr *From, QualType ToType,
   // FIXME: The exception for reference initialization here is not part of the
   // language rules, but follow other compilers in adding it as a tentative DR
   // resolution.
   bool IsDesignatedInit = From->hasDesignatedInit();
   if (!ToType->isAggregateType() && !ToType->isReferenceType() &&
       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))
         return TryCopyInitialization(S, From->getInit(0), ToType,
                                      SuppressUserConversions,
                                      InOverloadResolution,
                                      AllowObjCWritebackConversion);
     }

clang/test/CXX/drs/dr14xx.cpp

namespace nonaggregate {
 
     void test2() {
       NonAggregate x;
       NonAggregate y{x};
       NonAggregate z{{x}};
     }
   } // 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}}};
 
   namespace NestedInit2 {
     struct Pair { int a, b; };
     struct TwoPairs { TwoPairs(Pair, Pair); };
     struct Value { Value(Pair); Value(TwoPairs); };
     void f() { Value{{{1,2},{3,4}}}; }

clang/test/CXX/drs/dr21xx.cpp

 // RUN: %clang_cc1 -std=c++98 -triple x86_64-unknown-unknown %s -verify -fexceptions -Wno-deprecated-builtins -fcxx-exceptions -pedantic-errors
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown %s -verify -fexceptions -Wno-deprecated-builtins -fcxx-exceptions -pedantic-errors
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown %s -verify -fexceptions -Wno-deprecated-builtins -fcxx-exceptions -pedantic-errors
 // RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-unknown %s -verify -fexceptions -Wno-deprecated-builtins -fcxx-exceptions -pedantic-errors
 // RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown %s -verify -fexceptions -Wno-deprecated-builtins -fcxx-exceptions -pedantic-errors
 // RUN: %clang_cc1 -std=c++23 -triple x86_64-unknown-unknown %s -verify -fexceptions -Wno-deprecated-builtins -fcxx-exceptions -pedantic-errors
 
 #if __cplusplus < 201103L
 // expected-error@+1 {{variadic macro}}
 #define static_assert(...) __extension__ _Static_assert(__VA_ARGS__)
 #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 {
     static const int n = 1;
     int f() {
       return X<&n>::n; // ok, value-dependent
     }
     int g() {
#if __cplusplus >= 201103L
   static_assert(g.n == 1, "");
   static_assert(h.n == 1, ""); // expected-error {{constant}} expected-note {{read of temporary}}
   static_assert(i.a.n == 1, ""); // expected-error {{constant}} expected-note {{read of non-constexpr variable}}
   static_assert(j.a.n == 1, ""); // expected-error {{constant}} expected-note {{read of temporary}}
   static_assert(k.a.n == 1, "");
 #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; };
   constexpr int *test(U u) {
     return u.b;
   }
   static_assert(!test({123}), "u.b should be valid even when b is inactive");
 #endif

clang/test/CXX/drs/dr23xx.cpp

 // RUN: %clang_cc1 -std=c++98 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors 2>&1
 // RUN: %clang_cc1 -std=c++11 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors 2>&1 | FileCheck %s
 // RUN: %clang_cc1 -std=c++14 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors 2>&1 | FileCheck %s
 // RUN: %clang_cc1 -std=c++17 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors 2>&1 | FileCheck %s
 // RUN: %clang_cc1 -std=c++20 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors 2>&1 | FileCheck %s
 // RUN: %clang_cc1 -std=c++23 %s -verify -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>
 struct A;
 template <>
 struct A<> {};
 template <typename T, typename... Ts>
 struct A<T, Ts...> : A<Ts...> {};
void g() {
   f2(&b);
   f(C{});
   f(D{});
   f(F{}); // expected-error {{ambiguous conversion from derived class}}
 }
 } //namespace dr2303
 #endif
 
+namespace dr2311 {
+#if __cplusplus >= 201707L
+template<typename T>
+void test() {
+  // These should all be elided (no move)
+  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
+}
+
+struct NonMovable {
+  NonMovable(int);
+  NonMovable(NonMovable&&) = delete;
+};
+
+template void test<NonMovable>();
+
+struct NonMovableNonApplicableIList {
+  NonMovableNonApplicableIList(int);
+  NonMovableNonApplicableIList(NonMovableNonApplicableIList&&) = delete;
+  NonMovableNonApplicableIList(std::initializer_list<int>);
+};
+
+template void test<NonMovableNonApplicableIList>();
+#endif
+}
+
 // dr2331: na
 
 #if __cplusplus >= 201103L
 namespace dr2338 { // dr2338: 12
 namespace B {
 enum E : bool { Zero, One };
 static_assert((int)(E)2 == 1, "");
 } // namespace B

clang/www/cxx_dr_status.html

<tr id="2136">
     <td>NAD</td>
     <td>Argument-dependent lookup and initializer lists</td>
     <td class="none" align="center">Unknown</td>
   </tr>
   <tr id="2137">
     <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="none" 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>
     <td>NAD</td>
     <td>Explicit member specialization vs implicit instantiation</td>
     <td class="none" align="center">Unknown</td>
   </tr>
   <tr id="2139">