rsmith added a comment.
Looks like this is an implementation of DR1307. Please also add a test to
`test/CXX/drs/dr13xx.cpp`.
================
Comment at: clang/include/clang/Sema/Overload.h:548-549
+
+ /// The std::initializer_list expression to convert from.
+ const InitListExpr *StandardInitializerListFrom{nullptr};
+
----------------
Storing the `IntListExpr` here doesn't seem like it captures the necessary
information. Consider:
```
void f2(std::pair<const char*, const char*> (&&)[]); // #1
void f2(std::string (&&)[]); // #2
void g2() { f2({"foo","bar"}); } // should call #2
```
Here, the `InitListExpr` in both cases has two elements. We don't store the
converted `InitListExpr`, because we don't -- and aren't allowed to -- build
the converted `InitListExpr` until we've chosen the right overload. But I think
what we're supposed to do in this case is to notice that calling #1 would
initialize only one element of the array (one pair), whereas calling #2 would
initialize two elements of the array, so we should call #2 here.
In effect, what you need to do is to compute the effective array bound for the
case where we're initializing an array of unknown bound, and prefer the
conversion sequence with the lower effective array bound. `InitListChecker`
already works this out internally, see
[here](https://github.com/llvm/llvm-project/blob/c0bcd11068fc13e45b253c6c315882097f94c121/clang/lib/Sema/SemaInit.cpp#L1937),
but currently only does that when actually doing the conversion (when not in
`VerifyOnly` mode); you'd need to adjust that.
Given that the code that needs this (the P0388 part) is dead code for now,
perhaps the best thing would be to do only the array bound comparison in this
patch, and we can compute the proper array bound for said comparison in a
future patch.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:3690-3703
+class CompareListInitializationSequences {
+ // List-initialization sequence L1 is a better conversion sequence than
+ // list-initialization sequence L2 if:
+ // ...
+ // - C++17:
+ // L1 converts to type "array of N1 T", L2 converts to type "array of N2
T",
+ // and N1 is smaller than N2.,
----------------
I think this would be simpler if structured a little differently:
* Rename `CompareListInitializationSequences::Conversion` to
`ListInitializationSequence`.
* Make `CompareListInitializationSequences::Compare` a static member of
`ListInitializationSequence`.
* Move the element type comparison into `...::Compare` and remove the
`CompareListInitializationSequences` class, inlining the (now very simple)
constructor into its only user.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:3856-3861
if (ICS1.isStdInitializerListElement() &&
!ICS2.isStdInitializerListElement())
return ImplicitConversionSequence::Better;
if (!ICS1.isStdInitializerListElement() &&
ICS2.isStdInitializerListElement())
return ImplicitConversionSequence::Worse;
----------------
Should this be part of `CompareListInitializationSequences` too?
================
Comment at: clang/lib/Sema/SemaOverload.cpp:5150-5156
+ } else if (ToType->isConstantArrayType()) {
+ // Has the initializer list exactly N elements or fewer than N elements?
+ uint64_t Size = S.getASTContext()
+ .getAsConstantArrayType(ToType)
+ ->getSize()
+ .getZExtValue();
+ if (From->getNumInits() > Size)
----------------
Simplify, and we may as well avoid assuming that the size fits in 64 bits since
it's easy to do so here (even though there are lots of other places where we
make that assumption).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87563/new/
https://reviews.llvm.org/D87563
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
