[libcxx-commits] [PATCH] D62928: Constrain function assignment operator (2574)

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Thanks for working on this.



================
Comment at: include/functional:2189
             static const bool value = is_same<void, _Rp>::value ||
                 is_convertible<typename __invoke_of<_Fp&, _ArgTypes...>::type,
                                _Rp>::value;
----------------
Here.


================
Comment at: include/functional:2231
     function& operator=(nullptr_t) _NOEXCEPT;
-    template<class _Fp, class = _EnableIfCallable<_Fp>>
+    template<class _Fp, class = _EnableIfCallable<typename decay<_Fp>::type&>>
     function& operator=(_Fp&&);
----------------
I think this should just be `_EnableIfCallable<typename decay<_Fp>::type>`, since we already add the reference inside `__callable` above.

Alternatively, I think it would be even better to NOT add the reference inside `__callable`, and instead do it when we call `_EnableIfCallable`.


================
Comment at: test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/F_assign.pass.cpp:128
+        using F1 = std::function<int(int, int)>;
+        using F2 = std::function<A  (int, int)>;
+        static_assert(!std::is_assignable<F1&, F2&&>::value, "");
----------------
zoecarver wrote:
> Is it okay to allow assignment of functions with return types that are convertible to each other? The issue says:
> 
> > Callable (C++14 §20.9.11.2) for argument types ArgTypes... and return type R.
`Callable` seems to allow implicit conversions to the return type, so I think that's OK. However, `A` is not convertible to `int` (or vice-versa), so I think your test is correct as-is. You could also test that a `F1` can't be assigned to a `F2`.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D62928