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

[Erich Keane via libcxx-commits]

erichkeane wrote:

>As I understood the author, more time was needed to prepare the tests and the fix. I just tried to bring the tip of the tree to a good state soon (our internal release process was blocked on this) without putting any pressure on the author.

Typically when the author is engaging fairly quickly and only needs a short time (he had already identified the problem and only had process-related concerns), it is preferred to be understanding/polite and give them time to work on it.

While I am sympathetic to your internal processes, it seems to me that they should be more tolerant of a ToT that is not particularly stable, as is the case with Clang.

> With the standard as is, `B(A)` should be a better match because of [[over.ics.list]p(7.1)](https://wg21.link/over.ics.list#7.1), it is an Exact Match, and `B(std::vector<A>)` is a user defined conversion.
> 
> With the current CWG2311 fix, the call to `B(A)` is an Exact Match because it's an `A` prvalue.
> 
> In either case, `B(A)` should unambiguously be chosen over `B(std::vector<A>)` given `{ A() }` as an initializer.
> 
> https://github.com/llvm/llvm-project/blob/7b11c08c664863fbcd7a3058179b0af3de5d28e4/clang/lib/Sema/SemaOverload.cpp#L1593
> 
> ^ This line is the issue: `isCopyConstructor()` checks only for lvalue reference copy constructors, and the move constructor is selected (i.e., if you force a copy constructor to be called instead, it works: https://godbolt.org/z/rGEsPdMx8).
> 
> There is no reason to check for copy constructors here. Removing `Constructor->isCopyConstructor() &&` fixes the issue (`B(A)` is chosen, and all existing tests still pass)
> 
> @cor3ntin I have some extra test cases I would want to add too. Should I make a new pull request? Or are we going to reopen this one and target llvm 19?

If you haven't already, you can re-open this one.  However, you'll ALSO have to make a 'differential' PR against 18.x so that this can be fixed there too.

https://github.com/llvm/llvm-project/pull/77768