[libcxx-commits] [PATCH] D112904: [libc++] P0433R2: test that deduction guides are properly SFINAEd away.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.ctor/deduct.fail.cpp:9
+
+// UNSUPPORTED: c++03, c++11, c++14
+
----------------
var-const wrote:
> ldionne wrote:
> > var-const wrote:
> > > ldionne wrote:
> > > > Is there a reason why this one isn't based on SFINAE checks like the container tests?
> > > For containers, the requirement is for deduction guides to `not participate in overload resolution`, but for unique pointers, the requirement is that `If class template argument deduction would select the function template corresponding to this constructor, then the program is ill-formed`. I interpreted `ill-formed` as "resulting in a hard error" -- please let me know if this is incorrect.
> > Oh, then this is correct. It's a really unusual specification for this kind of failure though.
> I looked into this more closely, and there's actually been a change on this between C++17 and C++20.
> 
> In C++17, the requirement is:
> ```
> If class template argument deduction would select the function template corresponding to this constructor, then the program is ill-formed.
> ```
> However, [P1460](wg21.link/p1460) changed this in C++20 to:
> ```
> Mandates: This constructor is not selected by class template argument deduction.
> ```
> Now the words `ill-formed` are gone, and with the new wording, it seems like these constructors should just SFINAE away without causing a hard error. Moreover, this seems like a slight change in behavior. I don't see any discussion of this case in the paper, and I presume the new behavior is slightly more user-friendly.
> 
> It seems reasonable to me to only implement and test the new behavior, including in C++17 mode as well (FWIW, it was already implemented in a SFINAE-friendly way). Because of that, I replaced the previous `deduct.fail.cpp` file with a new `deduct.pass.cpp` test that uses `SFINAEs_away`. Let me know what you think!
"Mandates" always means "static_assert, not SFINAE." When the standard means SFINAE, it uses "Constraints" instead. However, I think this is an editorial defect in p1460's changes: it replaced a soft "Remark" with a hard "Mandates." It should have said:

`Remarks: This constructor is never selected by class template argument deduction.`

or simply

`unique_ptr(type_identity_t<pointer> p)`

libstdc++ and MSVC both interpret the intent as "this constructor shouldn't contribute to CTAD," which matches what you settled on here, IIUC.
I'll email LWG suggesting a new issue to clarify this wording.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112904