Constrain function assignment operator (2574)

[zoecarver]

Summary

This patch fixes issue 2574.

[zoecarver]

test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/F_assign.pass.cpp

128 ↗

(On Diff #203232)

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.

[ldionne]

Thanks for working on this.

include/functional

2189 ↗

(On Diff #203232)

Here.

2231 ↗

(On Diff #203232)

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.

test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/F_assign.pass.cpp

128 ↗

(On Diff #203232)

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.

[zoecarver]

include/functional

2231 ↗

(On Diff #203232)

Good catch. I will remove the reference here. If you think it would be better, I can make another patch to update all instances of _EnableIfCallable.

[ldionne]

include/functional

2231 ↗

(On Diff #203232)

I would update this patch as suggested.

[zoecarver]

include/functional

2231 ↗

(On Diff #203232)

Do you want me to update all three times _EnableIfCallable is used, in this patch (that's fine, just checking)?

[zoecarver]

Address @ldionne comments

[ldionne]

LGTM. It would be great if you could provide a patch that moves adding the reference outside of __callable, such that the code becomes: _EnableIfCallable<typename decay<_Fp>::type&> (notice the added reference here). You'd have to look at other usages of _EnableIfCallable too. Note that some usages of _EnableIfCallable correspond to places in the Standard where the term Lvalue-Callable is used. We could consider adding a LValueCallable trait to mirror that. Just a suggestion.

[zoecarver]

It would be great if you could provide a patch that moves adding the reference outside of __callable.

Sounds good. Will do.

[zoecarver]

Resolved by 8aa2266fd8dd237c84daf12c2eec0df244558f49.

libcxx/include/functional

template<class _Alloc>
       function(allocator_arg_t, const _Alloc&, function&&);
     template<class _Fp, class _Alloc, class = _EnableIfCallable<_Fp>>
       function(allocator_arg_t, const _Alloc& __a, _Fp __f);
 #endif
 
     function& operator=(const function&);
     function& operator=(function&&) _NOEXCEPT;
     function& operator=(nullptr_t) _NOEXCEPT;
-    template<class _Fp, class = _EnableIfCallable<_Fp>>
+    template<class _Fp, class = _EnableIfCallable<typename decay<_Fp>::type>>
     function& operator=(_Fp&&);
 
     ~function();
 
     // function modifiers:
     void swap(function&) _NOEXCEPT;
 
 #if _LIBCPP_STD_VER <= 14

libcxx/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/F_assign.pass.cpp

 #if TEST_STD_VER >= 11
     {
         using Fn = std::function<void(int, int, int)>;
         static_assert(std::is_assignable<Fn&, LValueCallable&>::value, "");
         static_assert(std::is_assignable<Fn&, LValueCallable>::value, "");
         static_assert(!std::is_assignable<Fn&, RValueCallable&>::value, "");
         static_assert(!std::is_assignable<Fn&, RValueCallable>::value, "");
     }
+    {
+        using Fn = std::function<void(int, int, int)>;
+        static_assert(std::is_assignable<Fn&, Fn&&>::value, "");
+    }
+    {
+        using F1 = std::function<void(int, int)>;
+        using F2 = std::function<void(int, int, int)>;
+        static_assert(!std::is_assignable<F1&, F2&&>::value, "");
+    }
+    {
+        using F1 = std::function<int(int, int)>;
+        using F2 = std::function<A  (int, int)>;
+        static_assert(!std::is_assignable<F1&, F2&&>::value, "");
+        static_assert(!std::is_assignable<F2&, F1&&>::value, "");
+    }
+    {
+        using F1 = std::function<void(int, int)>;
+        using F2 = std::function<void(int, int)&&>;
+        static_assert(!std::is_assignable<F1&, F2&&>::value, "");
+    }
 #endif
 
   return 0;
 }

libcxx/www/cxx1z_status.html

<!--   <I>Note: "NAD" means that the issue was deemed "Not a defect"</I> -->
   <tr><td><a href="https://wg21.link/LWG2557">2557</a></td><td>Logical operator traits are broken in the zero-argument case</td><td>Jacksonville</td><td>Complete</td></tr>
   <tr><td><a href="https://wg21.link/LWG2558">2558</a></td><td>[fund.ts.v2] Logical operator traits are broken in the zero-argument case</td><td>Jacksonville</td><td>Complete</td></tr>
   <tr><td><a href="https://wg21.link/LWG2559">2559</a></td><td>Error in LWG 2234's resolution</td><td>Jacksonville</td><td>Complete</td></tr>
   <tr><td><a href="https://wg21.link/LWG2560">2560</a></td><td><tt>is_constructible</tt> underspecified when applied to a function type</td><td>Jacksonville</td><td>Broken in 3.6; See r261653.</td></tr>
   <tr><td><a href="https://wg21.link/LWG2565">2565</a></td><td><tt>std::function</tt>'s move constructor should guarantee nothrow for <tt>reference_wrapper</tt>s and function pointers</td><td>Jacksonville</td><td>Complete</td></tr>
   <tr><td><a href="https://wg21.link/LWG2566">2566</a></td><td>Requirements on the first template parameter of container adaptors</td><td>Jacksonville</td><td>Complete</td></tr>
   <tr><td><a href="https://wg21.link/LWG2571">2571</a></td><td>&sect;[map.modifiers]/2 imposes nonsensical requirement on <tt>insert(InputIterator, InputIterator)</tt></td><td>Jacksonville</td><td>Complete</td></tr>
   <tr><td><a href="https://wg21.link/LWG2572">2572</a></td><td>The remarks for <tt>shared_ptr::operator*</tt> should apply to <i>cv</i>-qualified <tt>void</tt> as well</td><td>Jacksonville</td><td>Complete</td></tr>
-  <tr><td><a href="https://wg21.link/LWG2574">2574</a></td><td>[fund.ts.v2] <tt>std::experimental::function::operator=(F&amp;&amp;)</tt> should be constrained</td><td>Jacksonville</td><td></td></tr>
+  <tr><td><a href="https://wg21.link/LWG2574">2574</a></td><td>[fund.ts.v2] <tt>std::experimental::function::operator=(F&amp;&amp;)</tt> should be constrained</td><td>Jacksonville</td><td>Complete</td></tr>
   <tr><td><a href="https://wg21.link/LWG2575">2575</a></td><td>[fund.ts.v2] <tt>experimental::function::assign</tt> should be removed</td><td>Jacksonville</td><td></td></tr>
   <tr><td><a href="https://wg21.link/LWG2576">2576</a></td><td><tt>istream_iterator</tt> and <tt>ostream_iterator</tt> should use <tt>std::addressof</tt></td><td>Jacksonville</td><td>Complete</td></tr>
   <tr><td><a href="https://wg21.link/LWG2577">2577</a></td><td><tt>{shared,unique}_lock</tt> should use <tt>std::addressof</tt></td><td>Jacksonville</td><td>Complete</td></tr>
   <tr><td><a href="https://wg21.link/LWG2579">2579</a></td><td>Inconsistency wrt Allocators in <tt>basic_string</tt> assignment vs. <tt>basic_string::assign</tt></td><td>Jacksonville</td><td>Complete</td></tr>
   <tr><td><a href="https://wg21.link/LWG2581">2581</a></td><td>Specialization of <tt>&lt;type_traits&gt;</tt> variable templates should be prohibited</td><td>Jacksonville</td><td>Complete</td></tr>
   <tr><td><a href="https://wg21.link/LWG2582">2582</a></td><td>&sect;[res.on.functions]/2's prohibition against incomplete types shouldn't apply to type traits</td><td>Jacksonville</td><td>Complete</td></tr>
   <tr><td><a href="https://wg21.link/LWG2583">2583</a></td><td>There is no way to supply an allocator for <tt>basic_string(str, pos)</tt></td><td>Jacksonville</td><td>Complete</td></tr>
   <tr><td><a href="https://wg21.link/LWG2585">2585</a></td><td><tt>forward_list::resize(size_type, const value_type&amp;)</tt> effects incorrect</td><td>Jacksonville</td><td>Complete</td></tr>