[clang] pre-0388 array parm list initialization

[urnathan]

Summary

in continuing on p0388 (unbounded array conversions), I discovered we don't deal with list initialization of bounded array parameters. Indeed there's a smoking gun:

// FIXME: We're missing a lot of these checks.

This patch implements those checks. I discovered that the existing implementation, when marking an implicit conversion sequence as being to std::initializer_list, is broken in that it'll think initializing 'std::initializer_list<T> parm[N]' is doing that. But it's not, it's initializing an array of std::initializer. There's a test for that, (clang/tests/CXX/drs/dr15xx.cpp, around line 490) but it's working due to the other missing checks! (It continues to work after this patch too.)

Anyway, rather than have a single bit flag 'StdInitializerListElement' we need to record the type being initialized by a list, so we can compare array bounds. Sadly this does grow ImplicitConversionSequence.

TryListConversion now checks the target array type has sufficient bounds, and if it has additional bounds, there is a default conversion from {} available. Note the FIXME there -- is there a better way to determine that?

CompareImplicitConversionSequence now also compares array size, for two conversions to array type.

The structure of the code changes in SemaOverload is ready for changes that p0388 introduces involving IncompleteArrayType -- the llvm_unreachables in CompareImplicitConversionSequence in particular.

... let's try adding some other reviewers who've had their fingers in SemaOverload ...

[urnathan]

ping?

[jlebar]

... let's try adding some other reviewers who've had their fingers in SemaOverload ...

Way beyond my ken, but fingers crossed you get some help with this one!

[urnathan]

This is changing Larisse's code of:
19d08672844ee (Larisse Voufo 2015-01-27 18:47:05 +0000 3801) // list-initialization sequence L2 if:
so adding as a reviewer

[rsmith]

clang/include/clang/Sema/Overload.h

542

Can we store this directly as a QualType instead? (I think the only reason we store types as void*s in *ConversionSequence is a historical artifact from a time when union members couldn't have non-trivial default constructors.)

It's also not clear to me whether we need to store an additional type here, in addition to the types we're already tracking. I think I would expect the type we need to inspect is the overall destination type of the implicit conversion sequence (the "to type" for the final standard conversion sequence). Is this redundant, or are there cases where the type we want isn't already tracked? Is the problem case a conversion to T (&&)[], where we invent an array bound?

678–681

I'm not sure how to parse this: is this "is this a conversion from an initializer list to a type", or is this "do we have a destination type to convert an initializer list from"? If the latter, hasInitializerListToType might be clearer.

clang/lib/Sema/SemaOverload.cpp

3819

Use Context.hasSameType or Context.hasSameUnqualifiedType here to ignore type sugar in this comparison.

3827

You're also effectively asserting that the array is not a VLA and doesn't have a value-dependent bound here. I think that's fine (we shouldn't have a list initialization conversion for those cases); can you use a broader assertion failure message?

It looks like we would also reach this if the array bounds are equal. I would imagine that it's probably possible for us to end up comparing two initializer list conversions to the same type.

5150–5152

It would be nice to have custom FailureKind for this so that we can diagnose that there were too few / too many inits in the initializer list instead of saying there was no conversion.

5160

Hm, conversion sequences aren't totally ordered; what happens if there are two incomparable conversion sequences (eg, using different conversion functions)?

I think that's a pre-existing issue that this patch is not making any worse, and the problem is the language rules just don't make sense here. I've posted an example on the core reflector.

5165–5167

"bad" is not the worst conversion sequence we support; we treat "disallowed by language rules" conversion sequences (string literal -> non-const char*) as worse than a bad sequence. With this change, we won't bail if we hit a bad conversion after a "disallowed by language rules" conversion any more. Eg:

#include <initializer_list>
struct any { template<typename T> any(T); };
void f(std::initializer_list<char*>); // #1
void f(std::initializer_list<any>); // #2
void g() {
  // Used to pick #2; I think this will now pick #1 and reject during initialization because the worst
  // conversion sequence is string literal -> char* and we never set `Result` to the bad conversion sequence.
  f({"hello", 3});
}

I think we should go back to checking ICS.isBad() before comparing implicit conversion sequences to avoid that.

[urnathan]

clang/include/clang/Sema/Overload.h

542

It is quite possible a QualType is fine -- I matched the void *'s already there (thanks for the explanation)

We need to store 4 pieces of information (eventually, when all of p0388 is landed)
0) whether we're initializing from an initializer-list

1. whether we're initializing to a std::initializer_list.
2. the array bound for the unbound case you mention
3. whether we determined a bound (an inferred bound loses to a fixed bound of the same value).

IIUC, array bounds are presumed to be within the host's 'unsigned' (inferred from the iterator in TryListConversion), so we could use 3 bool flags[*] and an unsigned, rather than a whole type -- would that be preferrable? It looks like there's already 32 bits of padding in the structure on an I32LP64 machine.

The existing bug I ran into is the lack of clearing the flag for #1, when initializing an array of std::initializer_list (we still think it's an initializer_list, not an array). But of course there are more ways to fix that than the one I chose.

• we could use a zero value of the inferred array bound to indicate 'not inferred', as we can never infer a zero-sized array, right? But then we'd need to re-extract the bound from fixed-sized arrays when comparing sequences. Seems more efficient to extract once and store in an unsigned.

678–681

ok.

clang/lib/Sema/SemaOverload.cpp

3827

Good catch, let's not try and assert here, given a later patch adds the c++20 behaviour. I'll add an equal-size testcase though.

5150–5152

I have added too_{few,many}_initializers. Is it ok to go directly poking into Result.Bad.Kind like that? There seem to be other cases of such peeking.

5160

Thanks, I'll amend the comment to point this out.

5165–5167

I'm confused. The ImplicitConversionSequence::Kind has BadConversion as the only non-conversion, AFAICT. isBad would cause us to bail on the attempted string-literal->char *, wouldn't it? Your example selects #2 with this patch.

[urnathan]

Updated diff addressing most of Richard's comments, but still open:
a) how to record the list conversion bound and kind (rather than as a type)
b) A FIXME about figuring out if there's a list conversion from empty {}

[urnathan]

clang/lib/Sema/SemaOverload.cpp

5155–5156

/Is/ this the simplest way of finding {}?

[rsmith]

clang/include/clang/Sema/Overload.h

542

The use of host unsigned for braced initializers is a longstanding bug; things like int arr[] = {[0x100000001] = 1}; tend to crash or miscompile because of this. (But even if those things worked, they'd use a huge amount of memory and compile extremely slowly as a result, so fixing this hasn't been a high priority.) If possible I'd prefer to not replicate it in more places, because I think we'll eventually want to fix it :)

I think storing a type here to represent the type of the actual entity being initialized, but it'd be useful to retain the part of the old comment that says the rest of the sequence (including its "to" type) represents only the worst conversion for any element of the list. (It wasn't obvious to me when reading through this how the InitializerListToType differs from the final ToType of the conversion sequence; I think this would have been clearer if there were a reminder that the conversion sequence describes only the worst element conversion rather than the overall conversion.)

clang/lib/Sema/SemaOverload.cpp

3815–3817

Is it possible to simplify this by using getAsConstantArrayType here and eliding the ConstantArrayType checks below?

5150–5152

I think it's OK to poke at Result.Bad.Kind directly here, given that this code basically owns that type anyway, but it might be more obvious to use Result.setBad(BadConversionSequence::too_many_initializers, From, ToType) so that we don't need a reader of the code to remember that we already had a bad conversion sequence (mostly) properly set up at the start of the function.

5155–5156

I think so. The checks below, especially for aggregate classes and user-defined conversions, are pretty involved; reusing them directly like this seems best.

5165–5167

The problem is that a string literal to char* conversion is not a bad conversion, but ranks worse than bad conversions, so in particular taking the worst conversion in a sequence containing both a string literal to char* conversion and a bad conversion will pick the string literal to char* conversion as the worst conversion in the sequence, and in so doing will lose the information that the conversion was not possible at all.

Concretely, what I'm worried about is that we treat "hello" -> char* as a valid conversion sequence X, and 3 -> char* as a bad conversion sequence Y, and we consider X to be worse than Y. We used to bail out if we ever saw a bad ICS, which meant that we'd treat a sequence containing X and Y as bad. But with the new approach, we don't check whether each ICS in isolation is bad, and only check if the worst sequence so far is bad, so I was expecting that we would:

• Start with Result being good
• Compare Result with X, find X is worse, and remember that as Result
• Not bail out because Result is not bad
• Compare Result with Y, find Result is worse, and keep Result as-is

... and then end up with a non-bad conversion sequence for f. So my worry is that we'd incorrectly treat #1 as being viable.

I think my example is not properly demonstrating this, because we still end up with X as the worst conversion sequence for #1, and X compares worse than the user-defined conversion sequence to any. A correct example might be more like this:

#include <initializer_list>
struct ugly { ugly(char*); ugly(int); };
void f(std::initializer_list<char*>); // #1
void f(std::initializer_list<ugly>); // #2
void g() {
  f({"hello", 3});
}

... for which I think this patch will prefer #1, because for #1 we'll pick conversion X and for #2 we'll pick a user-defined conversion sequence starting with conversion X. Whereas we should pick #2, because #1 is not viable.

I think we could address this either by checking for ICS.isBad() on each conversion, and bailing out if we find a bad conversion (even if it's not the worst one), or by changing CompareImplicitConversionSequences to consider bad conversions to be worse than the string literal -> char* conversion. The latter is probably the better approach; maybe there's a reason we don't already do that, but it seems worth trying. It doesn't make sense to me to treat a formally-bad-but-supported conversion as being *worse* than a formally-bad-and-unsupported conversion.

5173–5174

I wonder if it would be cleaner to move this call earlier and convert some of the breaks and if (!Result.isBad()) checks into early returns?

[urnathan]

clang/lib/Sema/SemaOverload.cpp

5165–5167

Thanks, your non-template example does exhibit the problem. The template one didn't.

5173–5174

I also realized that the order we chose to compare any implicit{} initialization to the explicit element initializations is significant, hence my email reply CWG. I've changed this patch to compare {} after the element conversions for the reasons described there.

[urnathan]

Thanks for your review. I think I've addressed all your comments, but let me know if I've missed something. I renamed 'hasInitializerListToType' and friends to 'hasInitializerListContainerType', because there are other conversion sequences from initializer lists to non-array non-std::initializer_list types. Container seemed the best shortest word that didn't just say 'Array'.

[urnathan]

Added comment about maybe keeping StdInitializerListElement flag

clang/include/clang/Sema/Overload.h

542

thinking about the path forward, perhaps it would be better to
(a) keep StdInitializerListElement bitflag
(b) store the (maybe deduced) array dimension, (and not the type)

For the last patch in the series, I needed to add a 'is unbound array' bit flag. Keeping the StdInitializerListElement flag means reducing duplicate calls to isStdInitializerList. Storing just the bound means we can use ToType to check the element types are the same, and we don't need to fish the array bound out of the type when comparing.

That would most likely be more efficient, and remove the weird name for 'InitializerListContainerType' -- 'ArrayBound' is probably more mnemonic?

[xbolva00]

Commited without approval?

clang/include/clang/Basic/DiagnosticSemaKinds.td

def note_ovl_candidate_bad_conv_incomplete : Note<
     "%diff{$ to $|to parameter type}3,4 for "
     "%select{%ordinal6 argument|object argument}5"
     "%select{|; dereference the argument with *|"
     "; take the address of the argument with &|"
     "; remove *|"
     "; remove &}7">;
 def note_ovl_candidate_bad_list_argument : Note<
     "candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "
-    "cannot convert initializer list argument to %4">;
+    "%select{cannot convert initializer list|too few initializers in list"
+    "|too many initializers in list}7 argument to %4">;
 def note_ovl_candidate_bad_overload : Note<
     "candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "
     "no overload of %4 matching %3 for %ordinal5 argument">;
 def note_ovl_candidate_bad_conv : Note<
     "candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "
     "no known conversion "
     "%diff{from $ to $|from argument type to parameter type}3,4 for "
     "%select{%ordinal6 argument|object argument}5"

clang/include/clang/Sema/Overload.h

class Sema;
   /// BadConversionSequence - Records information about an invalid
   /// conversion sequence.
   struct BadConversionSequence {
     enum FailureKind {
       no_conversion,
       unrelated_class,
       bad_qualifiers,
       lvalue_ref_to_rvalue,
-      rvalue_ref_to_lvalue
+      rvalue_ref_to_lvalue,
+      too_few_initializers,
+      too_many_initializers,
     };
 
     // This can be null, e.g. for implicit object arguments.
     Expr *FromExpr;
 
     FailureKind Kind;
 
   private:
public:
     };
 
   private:
     enum {
       Uninitialized = BadConversion + 1
     };
 
     /// ConversionKind - The kind of implicit conversion sequence.
-    unsigned ConversionKind : 30;
+    unsigned ConversionKind;
 
-    /// Whether the target is really a std::initializer_list, and the
-    /// sequence only represents the worst element conversion.
-    unsigned StdInitializerListElement : 1;
+    /// When initializing an array or std::initializer_list from an
+    /// initializer-list, this is the array or std::initializer_list type being
+    /// initialized. The remainder of the conversion sequence, including ToType,
+    /// describe the worst conversion of an initializer to an element of the
+    /// array or std::initializer_list. (Note, 'worst' is not well defined.)
+    QualType InitializerListContainerType;
 
     void setKind(Kind K) {
       destruct();
       ConversionKind = K;
     }
 
     void destruct() {
       if (ConversionKind == AmbiguousConversion) Ambiguous.destruct();
union {
       AmbiguousConversionSequence Ambiguous;
 
       /// When ConversionKind == BadConversion, provides the details
       /// of the bad conversion.
       BadConversionSequence Bad;
     };
 
     ImplicitConversionSequence()
-        : ConversionKind(Uninitialized), StdInitializerListElement(false) {
+        : ConversionKind(Uninitialized), InitializerListContainerType() {
       Standard.setAsIdentityConversion();
     }
 
     ImplicitConversionSequence(const ImplicitConversionSequence &Other)
         : ConversionKind(Other.ConversionKind),
-          StdInitializerListElement(Other.StdInitializerListElement) {
+          InitializerListContainerType(Other.InitializerListContainerType) {
       switch (ConversionKind) {
       case Uninitialized: break;
       case StandardConversion: Standard = Other.Standard; break;
       case UserDefinedConversion: UserDefined = Other.UserDefined; break;
       case AmbiguousConversion: Ambiguous.copyFrom(Other.Ambiguous); break;
       case EllipsisConversion: break;
       case BadConversion: Bad = Other.Bad; break;
       }
public:
 
     void setAsIdentityConversion(QualType T) {
       setStandard();
       Standard.setAsIdentityConversion();
       Standard.setFromType(T);
       Standard.setAllToTypes(T);
     }
 
-    /// Whether the target is really a std::initializer_list, and the
-    /// sequence only represents the worst element conversion.
-    bool isStdInitializerListElement() const {
-      return StdInitializerListElement;
-    }
-
-    void setStdInitializerListElement(bool V = true) {
-      StdInitializerListElement = V;
+    // True iff this is a conversion sequence from an initializer list to an
+    // array or std::initializer.
+    bool hasInitializerListContainerType() const {
+      return !InitializerListContainerType.isNull();
+    }
+    void setInitializerListContainerType(QualType T) {
+      InitializerListContainerType = T;
+    }
+    QualType getInitializerListContainerType() const {
+      assert(hasInitializerListContainerType() &&
+             "not initializer list container");
+      return InitializerListContainerType;
     }
 
     /// Form an "implicit" conversion sequence from nullptr_t to bool, for a
     /// direct-initialization of a bool object from nullptr_t.
     static ImplicitConversionSequence getNullptrToBool(QualType SourceType,
                                                        QualType DestType,
                                                        bool NeedLValToRVal) {
       ImplicitConversionSequence ICS;

clang/lib/Sema/SemaOverload.cpp

if (After.First || After.Second || After.Third) {
     After.dump();
   }
 }
 
 /// dump - Print this implicit conversion sequence to standard
 /// error. Useful for debugging overloading issues.
 void ImplicitConversionSequence::dump() const {
   raw_ostream &OS = llvm::errs();
-  if (isStdInitializerListElement())
-    OS << "Worst std::initializer_list element conversion: ";
+  if (hasInitializerListContainerType())
+    OS << "Worst list element conversion: ";
   switch (ConversionKind) {
   case StandardConversion:
     OS << "Standard conversion: ";
     Standard.dump();
     break;
   case UserDefinedConversion:
     OS << "User-defined conversion: ";
     UserDefined.dump();
CompareImplicitConversionSequences(Sema &S, SourceLocation Loc,
   // conversion is better than string-literal to char* conversion (since there
   // is no such conversion in C++11). If there was no #1 at all or #1 couldn't
   // convert arguments, #2 would be the best viable function in C++11.
   // If the best viable function has this conversion, a warning will be issued
   // in C++03, or an ExtWarn (+SFINAE failure) will be issued in C++11.
 
   if (S.getLangOpts().CPlusPlus11 && !S.getLangOpts().WritableStrings &&
       hasDeprecatedStringLiteralToCharPtrConversion(ICS1) !=
-      hasDeprecatedStringLiteralToCharPtrConversion(ICS2))
+          hasDeprecatedStringLiteralToCharPtrConversion(ICS2) &&
+      // Ill-formedness must not differ
+      ICS1.isBad() == ICS2.isBad())
     return hasDeprecatedStringLiteralToCharPtrConversion(ICS1)
                ? ImplicitConversionSequence::Worse
                : ImplicitConversionSequence::Better;
 
   if (ICS1.getKindRank() < ICS2.getKindRank())
     return ImplicitConversionSequence::Better;
   if (ICS2.getKindRank() < ICS1.getKindRank())
     return ImplicitConversionSequence::Worse;
CompareImplicitConversionSequences(Sema &S, SourceLocation Loc,
   // Two implicit conversion sequences of the same form are
   // indistinguishable conversion sequences unless one of the
   // following rules apply: (C++ 13.3.3.2p3):
 
   // List-initialization sequence L1 is a better conversion sequence than
   // list-initialization sequence L2 if:
   // - L1 converts to std::initializer_list<X> for some X and L2 does not, or,
   //   if not that,
-  // - L1 converts to type "array of N1 T", L2 converts to type "array of N2 T",
-  //   and N1 is smaller than N2.,
+  // — L1 and L2 convert to arrays of the same element type, and either the
+  //   number of elements n_1 initialized by L1 is less than the number of
+  //   elements n_2 initialized by L2, or (unimplemented:C++20) n_1 = n_2 and L2
+  //   converts to an array of unknown bound and L1 does not,
   // even if one of the other rules in this paragraph would otherwise apply.
   if (!ICS1.isBad()) {
-    if (ICS1.isStdInitializerListElement() &&
-        !ICS2.isStdInitializerListElement())
-      return ImplicitConversionSequence::Better;
-    if (!ICS1.isStdInitializerListElement() &&
-        ICS2.isStdInitializerListElement())
-      return ImplicitConversionSequence::Worse;
+    bool StdInit1 = false, StdInit2 = false;
+    if (ICS1.hasInitializerListContainerType())
+      StdInit1 = S.isStdInitializerList(ICS1.getInitializerListContainerType(),
+                                        nullptr);
+    if (ICS2.hasInitializerListContainerType())
+      StdInit2 = S.isStdInitializerList(ICS2.getInitializerListContainerType(),
+                                        nullptr);
+    if (StdInit1 != StdInit2)
+      return StdInit1 ? ImplicitConversionSequence::Better
+                      : ImplicitConversionSequence::Worse;
+
+    if (ICS1.hasInitializerListContainerType() &&
+        ICS2.hasInitializerListContainerType())
+      if (auto *CAT1 = S.Context.getAsConstantArrayType(
+              ICS1.getInitializerListContainerType()))
+        if (auto *CAT2 = S.Context.getAsConstantArrayType(
+                ICS2.getInitializerListContainerType()))
+          if (S.Context.hasSameUnqualifiedType(CAT1->getElementType(),
+                                               CAT2->getElementType()) &&
+              CAT1->getSize() != CAT2->getSize())
+            return CAT1->getSize().ult(CAT2->getSize())
+                       ? ImplicitConversionSequence::Better
+                       : ImplicitConversionSequence::Worse;
   }
 
   if (ICS1.isStandard())
     // Standard conversion sequence S1 is a better conversion sequence than
     // standard conversion sequence S2 if [...]
     Result = CompareStandardConversionSequences(S, Loc,
                                                 ICS1.Standard, ICS2.Standard);
   else if (ICS1.isUserDefined()) {
TryListConversion(Sema &S, InitListExpr *From, QualType ToType,
   //   element of the list to X.
   //
   // C++14 [over.ics.list]p3:
   //   Otherwise, if the parameter type is "array of N X", if the initializer
   //   list has exactly N elements or if it has fewer than N elements and X is
   //   default-constructible, and if all the elements of the initializer list
   //   can be implicitly converted to X, the implicit conversion sequence is
   //   the worst conversion necessary to convert an element of the list to X.
-  //
-  // FIXME: We're missing a lot of these checks.
-  bool toStdInitializerList = false;
-  QualType X;
-  if (ToType->isArrayType())
-    X = S.Context.getAsArrayType(ToType)->getElementType();
-  else
-    toStdInitializerList = S.isStdInitializerList(ToType, &X);
-  if (!X.isNull()) {
-    for (unsigned i = 0, e = From->getNumInits(); i < e; ++i) {
-      Expr *Init = From->getInit(i);
-      ImplicitConversionSequence ICS =
-          TryCopyInitialization(S, Init, X, SuppressUserConversions,
-                                InOverloadResolution,
-                                AllowObjCWritebackConversion);
-      // If a single element isn't convertible, fail.
-      if (ICS.isBad()) {
-        Result = ICS;
-        break;
+  QualType InitTy = ToType;
+  ArrayType const *AT = S.Context.getAsArrayType(ToType);
+  if (AT || S.isStdInitializerList(ToType, &InitTy)) {
+    unsigned e = From->getNumInits();
+    ImplicitConversionSequence DfltElt;
+    DfltElt.setBad(BadConversionSequence::no_conversion, QualType(),
+                   QualType());
+    if (AT) {
+      // Result has been initialized above as a BadConversionSequence
+      InitTy = AT->getElementType();
+      if (ConstantArrayType const *CT = dyn_cast<ConstantArrayType>(AT)) {
+        if (CT->getSize().ult(e)) {
+          // Too many inits, fatally bad
+          Result.setBad(BadConversionSequence::too_many_initializers, From,
+                        ToType);
+          Result.setInitializerListContainerType(ToType);
+          return Result;
+        }
+        if (CT->getSize().ugt(e)) {
+          // Need an init from empty {}, is there one?
+          InitListExpr EmptyList(S.Context, From->getEndLoc(), None,
+                                 From->getEndLoc());
+          EmptyList.setType(S.Context.VoidTy);
+          DfltElt = TryListConversion(
+              S, &EmptyList, InitTy, SuppressUserConversions,
+              InOverloadResolution, AllowObjCWritebackConversion);
+          if (DfltElt.isBad()) {
+            // No {} init, fatally bad
+            Result.setBad(BadConversionSequence::too_few_initializers, From,
+                          ToType);
+            Result.setInitializerListContainerType(ToType);
+            return Result;
+          }
+        }
       }
-      // Otherwise, look for the worst conversion.
-      if (Result.isBad() || CompareImplicitConversionSequences(
-                                S, From->getBeginLoc(), ICS, Result) ==
-                                ImplicitConversionSequence::Worse)
-        Result = ICS;
     }
 
-    // For an empty list, we won't have computed any conversion sequence.
-    // Introduce the identity conversion sequence.
-    if (From->getNumInits() == 0) {
-      Result.setStandard();
-      Result.Standard.setAsIdentityConversion();
-      Result.Standard.setFromType(ToType);
-      Result.Standard.setAllToTypes(ToType);
+    Result.Standard.setFromType(InitTy);
+    Result.Standard.setAllToTypes(InitTy);
+    for (unsigned i = 0; i < e; ++i) {
+      Expr *Init = From->getInit(i);
+      ImplicitConversionSequence ICS = TryCopyInitialization(
+          S, Init, InitTy, SuppressUserConversions, InOverloadResolution,
+          AllowObjCWritebackConversion);
+
+      // Keep the worse conversion seen so far.
+      // FIXME: Sequences are not totally ordered, so 'worse' can be
+      // ambiguous. CWG has been informed.
+      if (CompareImplicitConversionSequences(S, From->getBeginLoc(), ICS,
+                                             Result) ==
+          ImplicitConversionSequence::Worse) {
+        Result = ICS;
+        // Bail as soon as we find something unconvertible.
+        if (Result.isBad()) {
+          Result.setInitializerListContainerType(ToType);
+          return Result;
+        }
+      }
     }
 
-    Result.setStdInitializerListElement(toStdInitializerList);
+    // If we needed any implicit {} initialization, compare that now.
+    // over.ics.list/6 indicates we should compare that conversion.  Again CWG
+    // has been informed that this might not be the best thing.
+    if (!DfltElt.isBad() && CompareImplicitConversionSequences(
+                                S, From->getEndLoc(), DfltElt, Result) ==
+                                ImplicitConversionSequence::Worse)
+      Result = DfltElt;
+
+    Result.setInitializerListContainerType(ToType);
     return Result;
   }
 
   // C++14 [over.ics.list]p4:
   // C++11 [over.ics.list]p3:
   //   Otherwise, if the parameter is a non-aggregate class X and overload
   //   resolution chooses a single best constructor [...] the implicit
   //   conversion sequence is a user-defined conversion sequence. If multiple
   //   constructors are viable but none is better than the others, the
   //   implicit conversion sequence is a user-defined conversion sequence.
   if (ToType->isRecordType() && !ToType->isAggregateType()) {
     // This function can deal with initializer lists.
     return TryUserDefinedConversion(S, From, ToType, SuppressUserConversions,
                                     AllowedExplicit::None,
                                     InOverloadResolution, /*CStyle=*/false,
                                     AllowObjCWritebackConversion,
                                     /*AllowObjCConversionOnExplicit=*/false);
   }
 
   // C++14 [over.ics.list]p5:
   // C++11 [over.ics.list]p4:
   //   Otherwise, if the parameter has an aggregate type which can be
   //   initialized from the initializer list [...] the implicit conversion
   //   sequence is a user-defined conversion sequence.
case BadConversionSequence::rvalue_ref_to_lvalue: {
       Diag(Method->getLocation(), diag::note_previous_decl)
         << Method->getDeclName();
       return ExprError();
     }
 
     case BadConversionSequence::no_conversion:
     case BadConversionSequence::unrelated_class:
       break;
+
+    case BadConversionSequence::too_few_initializers:
+    case BadConversionSequence::too_many_initializers:
+      llvm_unreachable("Lists are not objects");
     }
 
     return Diag(From->getBeginLoc(), diag::err_member_function_call_bad_type)
            << ImplicitParamRecordType << FromRecordType
            << From->getSourceRange();
   }
 
   if (ICS.Standard.Second == ICK_Derived_To_Base) {
static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
   }
 
   // Special diagnostic for failure to convert an initializer list, since
   // telling the user that it has type void is not useful.
   if (FromExpr && isa<InitListExpr>(FromExpr)) {
     S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_list_argument)
         << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
         << (FromExpr ? FromExpr->getSourceRange() : SourceRange()) << FromTy
-        << ToTy << (unsigned)isObjectArgument << I + 1;
+        << ToTy << (unsigned)isObjectArgument << I + 1
+        << (Conv.Bad.Kind == BadConversionSequence::too_few_initializers ? 1
+            : Conv.Bad.Kind == BadConversionSequence::too_many_initializers
+                ? 2
+                : 0);
     MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
     return;
   }
 
   // Diagnose references or pointers to incomplete types differently,
   // since it's far from impossible that the incompleteness triggered
   // the failure.
   QualType TempFromTy = FromTy.getNonReferenceType();

clang/test/SemaCXX/overload-ary-bind.cpp

+// RUN: %clang_cc1 -std=c++20 -verify %s
+// RUN: %clang_cc1 -std=c++17 -verify %s
+
+namespace One {
+char (&b(int(&&)[1]))[1]; // #1 expected-note{{too many initializers}}
+char (&b(int(&&)[2]))[2]; // #2 expected-note{{too many initializers}}
+
+void f() {
+  static_assert(sizeof(b({1})) == 1);    // #1
+  static_assert(sizeof(b({1, 2})) == 2); // #2
+
+  b({1, 2, 3}); // expected-error{{no matching function}}
+}
+} // namespace One
+
+namespace Two {
+struct Bob {
+  Bob(int = 1);
+};
+
+char (&b(Bob(&&)[1]))[1]; // #1
+char (&b(Bob(&&)[2]))[2]; // #2
+
+void f() {
+  static_assert(sizeof(b({})) == 1);         // #1
+  static_assert(sizeof(b({Bob()})) == 1);    // #1
+  static_assert(sizeof(b({2, Bob()})) == 2); // #2
+}
+} // namespace Two
+
+namespace Three {
+struct Kevin {
+  Kevin(int);
+};
+
+char (&b(Kevin(&&)[2]))[2]; // #2 expected-note{{too few initializers}}
+
+void f() {
+  b({2}); // #1 expected-error{{no matching function}}
+}
+} // namespace Three
+
+namespace Four {
+char (&b(int(&&)[1], float))[1];  // #1 expected-note{{candidate}}
+char (&b(int(&&)[1], double))[2]; // #2 expected-note{{candidate}}
+
+char (&c(float, int(&&)[1]))[1];  // #1 expected-note{{candidate}}
+char (&c(double, int(&&)[1]))[2]; // #2 expected-note{{candidate}}
+
+void f() {
+  b({1}, 0); // expected-error{{is ambiguous}}
+  c(0, {1}); // expected-error{{is ambiguous}}
+}
+} // namespace Four
+
+typedef decltype(sizeof(char)) size_t;
+namespace std {
+// sufficient initializer list
+template <class _E>
+class initializer_list {
+  const _E *__begin_;
+  size_t __size_;
+
+  constexpr initializer_list(const _E *__b, size_t __s)
+      : __begin_(__b),
+        __size_(__s) {}
+
+public:
+  typedef _E value_type;
+  typedef const _E &reference;
+  typedef const _E &const_reference;
+  typedef size_t size_type;
+
+  typedef const _E *iterator;
+  typedef const _E *const_iterator;
+
+  constexpr initializer_list() : __begin_(nullptr), __size_(0) {}
+
+  constexpr size_t size() const { return __size_; }
+  constexpr const _E *begin() const { return __begin_; }
+  constexpr const _E *end() const { return __begin_ + __size_; }
+};
+} // namespace std
+
+namespace Five {
+struct ugly {
+  ugly(char *);
+  ugly(int);
+};
+char (&f(std::initializer_list<char *>))[1]; // #1
+char (&f(std::initializer_list<ugly>))[2];   // #2
+void g() {
+  // Pick #2 as #1 not viable (3->char * fails).
+  static_assert(sizeof(f({"hello", 3})) == 2); // expected-warning{{not allow}}
+}
+
+} // namespace Five