[libc++] Fix LWG3146 "Excessive unwrapping in std::ref/cref"

[• Quuxplusone]

Summary

Drive-by constexprify some of the existing tests.

[philnik]

LGTM % nits

libcxx/include/__functional/reference_wrapper.h

179

Pre-existing

libcxx/test/std/utilities/function.objects/refwrap/refwrap.helpers/cref_2.pass.cpp

39

I think I'd rather see // LWG 3146 here. Both tests are for the LWG issue.

[• Quuxplusone]

Address review comments. Fix the C++03 test failure.
Comment on a hacky workaround in std::cref; I'll post a separate PR showing how we might remove it.

[Mordante]

LGTM!

libcxx/include/__functional/reference_wrapper.h

213

That makes it clear it's supposed to work like this in all Standard modes and it's not a C++20 change.
Obviously this is kind of moot due to D117974.

libcxx/test/std/utilities/function.objects/refwrap/refwrap.helpers/cref_2.pass.cpp

25

Nice to see these constexpr tests!

libcxx/docs/Status/Cxx2bIssues.csv

 `3553 <https://wg21.link/LWG3553>`__,"Useless constraint in ``split_view::outer-iterator::value_type::begin()``","June 2021","","","|ranges|"
 `3555 <https://wg21.link/LWG3555>`__,"``{transform,elements}_view::iterator::iterator_concept`` should consider const-qualification of the underlying range","June 2021","","","|ranges|"
 "","","","",""
 `2191 <https://wg21.link/LWG2191>`__,"Incorrect specification of ``match_results(match_results&&)``","October 2021","",""
 `2381 <https://wg21.link/LWG2381>`__,"Inconsistency in parsing floating point numbers","October 2021","",""
 `2762 <https://wg21.link/LWG2762>`__,"``unique_ptr operator*()`` should be ``noexcept``","October 2021","",""
 `3121 <https://wg21.link/LWG3121>`__,"``tuple`` constructor constraints for ``UTypes&&...`` overloads","October 2021","",""
 `3123 <https://wg21.link/LWG3123>`__,"``duration`` constructor from representation shouldn't be effectively non-throwing","October 2021","","","|chrono|"
-`3146 <https://wg21.link/LWG3146>`__,"Excessive unwrapping in ``std::ref/cref``","October 2021","",""
+`3146 <https://wg21.link/LWG3146>`__,"Excessive unwrapping in ``std::ref/cref``","October 2021","|Complete|","14.0"
 `3152 <https://wg21.link/LWG3152>`__,"``common_type`` and ``common_reference`` have flaws in common","October 2021","",""
 `3293 <https://wg21.link/LWG3293>`__,"``move_iterator operator+()`` has incorrect constraints","October 2021","","","|ranges|"
 `3361 <https://wg21.link/LWG3361>`__,"``safe_range<SomeRange&>`` case","October 2021","","","|ranges|"
 `3392 <https://wg21.link/LWG3392>`__,"``ranges::distance()`` cannot be used on a move-only iterator with a sized sentinel","October 2021","","","|ranges|"
 `3407 <https://wg21.link/LWG3407>`__,"Some problems with the wording changes of P1739R4","October 2021","","","|ranges|"
 `3422 <https://wg21.link/LWG3422>`__,"Issues of ``seed_seq``'s constructors","October 2021","",""
 `3470 <https://wg21.link/LWG3470>`__,"``convertible-to-non-slicing`` seems to reject valid case","October 2021","","","|ranges|"
 `3480 <https://wg21.link/LWG3480>`__,"``directory_iterator`` and ``recursive_directory_iterator`` are not C++20 ranges","October 2021","|Complete|","14.0","|ranges|"

 `3589 <https://wg21.link/LWG3589>`__,"The ``const`` lvalue reference overload of ``get`` for ``subrange`` does not constrain ``I`` to be ``copyable`` when ``N == 0``","October 2021","","","|ranges|"
 `3590 <https://wg21.link/LWG3590>`__,"``split_view::base() const &`` is overconstrained","October 2021","","","|ranges|"
 `3591 <https://wg21.link/LWG3591>`__,"``lazy_split_view<input_view>::inner-iterator::base() &&`` invalidates outer iterators","October 2021","","","|ranges|"
 `3592 <https://wg21.link/LWG3592>`__,"``lazy_split_view`` needs to check the simpleness of Pattern","October 2021","","","|ranges|"
 `3593 <https://wg21.link/LWG3593>`__,"Several iterators' ``base() const &`` and ``lazy_split_view::outer-iterator::value_type::end()`` missing ``noexcept``","October 2021","","","|ranges|"
 `3595 <https://wg21.link/LWG3595>`__,"Exposition-only classes proxy and postfix-proxy for ``common_iterator`` should be fully ``constexpr``","October 2021","","","|ranges|"
 "","","","",""
 `3645 <https://wg21.link/LWG3645>`__,"``resize_and_overwrite`` is overspecified to call its callback with lvalues", "Not voted in","|Complete|","14.0",""
-"","","","",""
+"","","","",""
 

libcxx/include/__functional/reference_wrapper.h

#else
     _LIBCPP_INLINE_VISIBILITY
     typename __invoke_return2<type, _A0 const, _A1 const, _A2 const>::type
     operator() (_A0 const& __a0, _A1 const& __a1, _A2 const& __a2) const {
         return _VSTD::__invoke(get(), __a0, __a1, __a2);
     }
 #endif // _LIBCPP_CXX03_LANG
 };
 
-#if _LIBCPP_STD_VER >= 17
+#if _LIBCPP_STD_VER > 14
 template <class _Tp>
 reference_wrapper(_Tp&) -> reference_wrapper<_Tp>;
 #endif
 
 template <class _Tp>
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
 reference_wrapper<_Tp>
 ref(_Tp& __t) _NOEXCEPT
 {
     return reference_wrapper<_Tp>(__t);
 }
 
 template <class _Tp>
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
 reference_wrapper<_Tp>
 ref(reference_wrapper<_Tp> __t) _NOEXCEPT
 {
-    return _VSTD::ref(__t.get());
+    return __t;
 }
 
 template <class _Tp>
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
 reference_wrapper<const _Tp>
 cref(const _Tp& __t) _NOEXCEPT
 {
     return reference_wrapper<const _Tp>(__t);
 }
 
 template <class _Tp>
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
 reference_wrapper<const _Tp>
 cref(reference_wrapper<_Tp> __t) _NOEXCEPT
 {
-    return _VSTD::cref(__t.get());
+    // C++20 says "return __t", but C++03 lacks the relevant
+    // converting constructor. This should be equivalent.
+    return __t.get();
 }
 
 #ifndef _LIBCPP_CXX03_LANG
 template <class _Tp> void ref(const _Tp&&) = delete;
 template <class _Tp> void cref(const _Tp&&) = delete;
 #endif
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___FUNCTIONAL_REFERENCE_WRAPPER_H

libcxx/test/std/utilities/function.objects/refwrap/refwrap.helpers/cref_1.pass.cpp

 
 // template <ObjectType T> reference_wrapper<const T> cref(const T& t);
 
 #include <functional>
 #include <cassert>
 
 #include "test_macros.h"
 
-int main(int, char**)
+TEST_CONSTEXPR_CXX20 bool test()
 {
-    int i = 0;
-    std::reference_wrapper<const int> r = std::cref(i);
-    assert(&r.get() == &i);
+  return true;
+}
+
+int main(int, char**)
+{
+  test();
+#if TEST_STD_VER > 17
+  static_assert(test());
+#endif
 
   return 0;
 }

libcxx/test/std/utilities/function.objects/refwrap/refwrap.helpers/cref_2.pass.cpp

 
 #include "test_macros.h"
 
 namespace adl {
   struct A {};
   void cref(A) {}
 }
 
-int main(int, char**)
+TEST_CONSTEXPR_CXX20 bool test()
+{
   {
     const int i = 0;
     std::reference_wrapper<const int> r1 = std::cref(i);
     std::reference_wrapper<const int> r2 = std::cref(r1);
     assert(&r2.get() == &i);
-
-    {
+  }
     adl::A a;
     std::reference_wrapper<const adl::A> a1 = std::cref(a);
     std::reference_wrapper<const adl::A> a2 = std::cref(a1);
     assert(&a2.get() == &a);
-    }
+  return true;
+}
+
+int main(int, char**)
+{
+  test();
+#if TEST_STD_VER > 17
+  static_assert(test());
+#endif
 
   return 0;
 }

libcxx/test/std/utilities/function.objects/refwrap/refwrap.helpers/lwg3146.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <functional>
+
+// reference_wrapper
+
+// template <ObjectType T> reference_wrapper<T> ref(reference_wrapper<T> t);
+//   LWG 3146 "Excessive unwrapping in std::ref/cref"
+
+#include <functional>
+#include <cassert>
+
+#include "test_macros.h"
+
+TEST_CONSTEXPR_CXX20 bool test()
+{
+  {
+    int i = 0;
+    std::reference_wrapper<int> ri(i);
+    std::reference_wrapper<std::reference_wrapper<int> > rri(ri);
+    auto rrj = std::ref(rri);
+    ASSERT_SAME_TYPE(decltype(rrj), std::reference_wrapper<std::reference_wrapper<int> >);
+    assert(&rrj.get() == &ri);
+  }
+  {
+    int i = 0;
+    std::reference_wrapper<int> ri(i);
+    std::reference_wrapper<const std::reference_wrapper<int> > rri(ri);
+    auto rrj = std::ref(rri);
+    ASSERT_SAME_TYPE(decltype(rrj), std::reference_wrapper<const std::reference_wrapper<int> >);
+    assert(&rrj.get() == &ri);
+  }
+  {
+    int i = 0;
+    std::reference_wrapper<int> ri(i);
+    std::reference_wrapper<std::reference_wrapper<int> > rri(ri);
+    auto rrj = std::cref(rri);
+    ASSERT_SAME_TYPE(decltype(rrj), std::reference_wrapper<const std::reference_wrapper<int> >);
+    assert(&rrj.get() == &ri);
+  }
+  {
+    int i = 0;
+    std::reference_wrapper<int> ri(i);
+    std::reference_wrapper<const std::reference_wrapper<int> > rri(ri);
+    auto rrj = std::cref(rri);
+    ASSERT_SAME_TYPE(decltype(rrj), std::reference_wrapper<const std::reference_wrapper<int> >);
+    assert(&rrj.get() == &ri);
+  }
+  return true;
+}
+
+int main(int, char**)
+{
+  test();
+#if TEST_STD_VER > 17
+  static_assert(test());
+#endif
+
+  return 0;
+}

libcxx/test/std/utilities/function.objects/refwrap/refwrap.helpers/ref_1.pass.cpp

 
 // template <ObjectType T> reference_wrapper<T> ref(T& t);
 
 #include <functional>
 #include <cassert>
 
 #include "test_macros.h"
 
-int main(int, char**)
+TEST_CONSTEXPR_CXX20 bool test()
 {
-    int i = 0;
-    std::reference_wrapper<int> r = std::ref(i);
-    assert(&r.get() == &i);
+  return true;
+}
+
+int main(int, char**)
+{
+  test();
+#if TEST_STD_VER > 17
+  static_assert(test());
+#endif
 
   return 0;
 }

libcxx/test/std/utilities/function.objects/refwrap/refwrap.helpers/ref_2.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // <functional>
 
 // reference_wrapper
 
-// template <ObjectType T> reference_wrapper<T> ref(reference_wrapper<T>t);
+// template <ObjectType T> reference_wrapper<T> ref(reference_wrapper<T> t);
 
 #include <functional>
 #include <cassert>
 
 #include "counting_predicates.h"
 
 #include "test_macros.h"
 
 bool is5 ( int i ) { return i == 5; }
 
 template <typename T>
 bool call_pred ( T pred ) { return pred(5); }
 
 namespace adl {
   struct A {};
   void ref(A) {}
 }
 
-int main(int, char**)
+TEST_CONSTEXPR_CXX20 bool test()
 {
-    {
     int i = 0;
     std::reference_wrapper<int> r1 = std::ref(i);
     std::reference_wrapper<int> r2 = std::ref(r1);
     assert(&r2.get() == &i);
-    }
-    {
     adl::A a;
     std::reference_wrapper<adl::A> a1 = std::ref(a);
     std::reference_wrapper<adl::A> a2 = std::ref(a1);
     assert(&a2.get() == &a);
-    }
+  return true;
+}
+
+int main(int, char**)
+{
+  test();
+#if TEST_STD_VER > 17
+  static_assert(test());
+#endif
 
-    {
     unary_counting_predicate<bool(*)(int), int> cp(is5);
     assert(!cp(6));
     assert(cp.count() == 1);
     assert(call_pred(cp));
     assert(cp.count() == 1);
     assert(call_pred(std::ref(cp)));
     assert(cp.count() == 2);
-    }
 
   return 0;
 }