[PATCH] D133683: [c++] implements tentative DR1432 for partial ordering of function template

Tue Sep 27 11:56:47 PDT 2022

ychen added a comment.

In D133683#3818804 <https://reviews.llvm.org/D133683#3818804>, @mizvekov wrote:

> In D133683#3818752 <https://reviews.llvm.org/D133683#3818752>, @ychen wrote:
>
>> I didn't add that because this patch is logically part of D128745 <https://reviews.llvm.org/D128745> which has a release note already.
>
> Okay, I see that the existing release note entry talks just about implementing those DRs without going specific, so that is fine.
> It does not mention that DR1432 implementation is tentative as there is no published resolution though.
>
>> I think GCC is the one that is not conforming. MSVC agrees it is ambiguous (https://godbolt.org/z/T5zbxaTYq). Basically, only the first parameter is considered for partial ordering. (https://eel.is/c++draft/temp.deduct.partial#3.1)
>
> I could totally buy that the GCC behavior is accidental, but it does make sense, with the idea behind DR1432, that we should prefer the first overload because that deduction is simpler, or because it does not deduce a parameter pack.
>
> If there was a resolution, the answer here would be clear cut, but since there isn't, we are making up our own rules and getting into a language design discussion.
> If you don't think that is clear cut, it's fine, we can move this discussion elsewhere :)

It is clear cut by https://eel.is/c++draft/temp.deduct.partial#3.1. It is orthogonal to the DRs implemented in D128745 <https://reviews.llvm.org/D128745> and here.
https://godbolt.org/z/s49sqrabY  : if you call `g(42,42)`, it would reveal that the function without pack indeed wins. It is ambiguous because the caller `g(42)` does not use the pack. So the wording prevents us from letting the non-pack version win. I do somewhat agree that it is more consistent and predictable to always consider all function parameters for partial ordering. Yeah, that needs a whole separate discussion on https://eel.is/c++draft/temp.deduct.partial#3.1.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133683/new/

https://reviews.llvm.org/D133683