[libcxx-commits] [PATCH] D60368: Add bind_front function (P0356R5)

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added inline comments.


================
Comment at: libcxx/include/functional:3084
+                                       is_constructible<decay_t<_Args>, _Args>...,
+                                       is_move_constructible<_Args>...
+                                       >::value>>
----------------
Quuxplusone wrote:
> zoecarver wrote:
> > Quuxplusone wrote:
> > > `is_move_constructible<decay_t<_Args>>...` perhaps? It looks like `_Args` can contain lvalue-reference types, and also lvalue-reference-to-array types.
> > > 
> > > If this is really a bug I found, please add a regression test too.
> > > Mandates: conjunction_v<is_constructible<FD, F>, is_move_constructible<FD>, is_constructible<BoundArgs, Args>..., is_move_constructible<BoundArgs>...> shall be true.
> > 
> > Additionally, the bound args are decayed meaning that all the args must be move or copy constructible. 
> Ah, I think I get it (or at least I see something I'd missed the first time around).
> ```
> int a[10];
> int f(int *);
> auto fa = std::bind_front(f, a);
> ```
> works by decaying `a` into an `int*` for storage in the closure object.
> 
> But here's an example that triggers my problem, when tested against the current patch:
> ```
> struct S  {
>     explicit S() { }
>     S(S&);
> };
> S s;
> void f(S&);
> auto ff = std::bind_front(f, s);  // OK
> auto gg = std::move(ff);  // cascading errors deep inside the template stuff
> ```
> Here `s` is an `S&`, so `_Args` is `S&`, so it's true that `is_move_constructible_v<_Args>` even though `not is_move_constructible_v<decay_t<_Args>>`. I think either `std::bind_front(f, s)` should be ill-formed (my preference), or else it should produce a closure object with no move-constructor.
I think you're right. This was just an oversight on my part. Because the standard specifically says to check `is_move_constructible<decay_t<_Args>>`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60368