<ranges>: Fix the return types and overload resolution of range ada…

…ptor closure objects (#4211)
microsoft · Nov 29, 2023 · 0274d83 · 0274d83
1 parent 8234695
commit 0274d83
Show file tree

Hide file tree

Showing 2 changed files with 91 additions and 9 deletions.
diff --git a/stl/inc/ranges b/stl/inc/ranges
@@ -133,34 +133,41 @@ namespace ranges {
                 is_nothrow_constructible_v<_Ty1, _ClosureLeft>&& is_nothrow_constructible_v<_Ty2, _ClosureRight>)
                 : _Left(_STD forward<_Ty1>(_Val1)), _Right(_STD forward<_Ty2>(_Val2)) {}
 
+            void operator()(auto&&) &       = delete;
+            void operator()(auto&&) const&  = delete;
+            void operator()(auto&&) &&      = delete;
+            void operator()(auto&&) const&& = delete;
+
+            // TRANSITION, DevCom-10519736, explicit `this->` shouldn't be needed.
+
             template <class _Ty>
-            _NODISCARD constexpr auto operator()(_Ty&& _Val) & noexcept(
+            _NODISCARD constexpr decltype(auto) operator()(_Ty&& _Val) & noexcept(
                 noexcept(_Right(_Left(_STD forward<_Ty>(_Val)))))
-                requires requires { _Right(_Left(_STD forward<_Ty>(_Val))); }
+                requires requires { this->_Right(this->_Left(_STD forward<_Ty>(_Val))); }
             {
                 return _Right(_Left(_STD forward<_Ty>(_Val)));
             }
 
             template <class _Ty>
-            _NODISCARD constexpr auto operator()(_Ty&& _Val) const& noexcept(
+            _NODISCARD constexpr decltype(auto) operator()(_Ty&& _Val) const& noexcept(
                 noexcept(_Right(_Left(_STD forward<_Ty>(_Val)))))
-                requires requires { _Right(_Left(_STD forward<_Ty>(_Val))); }
+                requires requires { this->_Right(this->_Left(_STD forward<_Ty>(_Val))); }
             {
                 return _Right(_Left(_STD forward<_Ty>(_Val)));
             }
 
             template <class _Ty>
-            _NODISCARD constexpr auto operator()(_Ty&& _Val) && noexcept(
+            _NODISCARD constexpr decltype(auto) operator()(_Ty&& _Val) && noexcept(
                 noexcept(_STD move(_Right)(_STD move(_Left)(_STD forward<_Ty>(_Val)))))
-                requires requires { _STD move(_Right)(_STD move(_Left)(_STD forward<_Ty>(_Val))); }
+                requires requires { _STD move(this->_Right)(_STD move(this->_Left)(_STD forward<_Ty>(_Val))); }
             {
                 return _STD move(_Right)(_STD move(_Left)(_STD forward<_Ty>(_Val)));
             }
 
             template <class _Ty>
-            _NODISCARD constexpr auto operator()(_Ty&& _Val) const&& noexcept(
+            _NODISCARD constexpr decltype(auto) operator()(_Ty&& _Val) const&& noexcept(
                 noexcept(_STD move(_Right)(_STD move(_Left)(_STD forward<_Ty>(_Val)))))
-                requires requires { _STD move(_Right)(_STD move(_Left)(_STD forward<_Ty>(_Val))); }
+                requires requires { _STD move(this->_Right)(_STD move(this->_Left)(_STD forward<_Ty>(_Val))); }
             {
                 return _STD move(_Right)(_STD move(_Left)(_STD forward<_Ty>(_Val)));
             }
@@ -180,7 +187,7 @@ namespace ranges {
 
         _EXPORT_STD template <class _Left, class _Right>
             requires (_Range_adaptor_closure_object<_Right> && range<_Left>)
-        _NODISCARD constexpr auto operator|(_Left&& __l, _Right&& __r) noexcept(
+        _NODISCARD constexpr decltype(auto) operator|(_Left&& __l, _Right&& __r) noexcept(
             noexcept(_STD forward<_Right>(__r)(_STD forward<_Left>(__l))))
             requires requires { static_cast<_Right&&>(__r)(static_cast<_Left&&>(__l)); }
         {

diff --git a/tests/std/tests/P2387R3_pipe_support_for_user_defined_range_adaptors/test.cpp b/tests/std/tests/P2387R3_pipe_support_for_user_defined_range_adaptors/test.cpp
@@ -223,6 +223,81 @@ constexpr bool test_mixing_of_range_adaptors() {
     return true;
 }
 
+struct Pinned {
+    Pinned()                         = default;
+    Pinned(const Pinned&)            = delete;
+    Pinned& operator=(const Pinned&) = delete;
+};
+
+constexpr Pinned pinned_object{};
+
+struct PinnedReturningRaco : ranges::range_adaptor_closure<PinnedReturningRaco> {
+    constexpr const Pinned& operator()(ranges::range auto&&) const noexcept {
+        return pinned_object;
+    }
+};
+
+struct RangeIdentity : ranges::range_adaptor_closure<RangeIdentity> {
+    constexpr auto&& operator()(ranges::range auto&& t) const noexcept {
+        return forward<decltype(t)>(t);
+    }
+};
+
+struct ConstOnlyRangeIdentity : ranges::range_adaptor_closure<ConstOnlyRangeIdentity> {
+    constexpr auto&& operator()(ranges::range auto&& t) const noexcept {
+        return forward<decltype(t)>(t);
+    }
+    void operator()(ranges::range auto&&) = delete;
+};
+
+void test_perfect_forwarding_properties() { // COMPILE-ONLY
+    // GH-4153: <ranges>: operator|(_Left&& __l, _Right&& __r) should return decltype(auto)
+    // Intentionally avoid using some traits/concepts to test correctness in non-immediate contexts.
+    {
+        PinnedReturningRaco raco{};
+
+        static_assert(same_as<decltype(TestRange{} | raco), const Pinned&>);
+        static_assert(same_as<decltype(TestRange{} | as_const(raco)), const Pinned&>);
+        static_assert(same_as<decltype(TestRange{} | move(raco)), const Pinned&>);
+        static_assert(same_as<decltype(TestRange{} | move(as_const(raco))), const Pinned&>);
+
+        static_assert(same_as<decltype(raco(TestRange{})), const Pinned&>);
+        static_assert(same_as<decltype(as_const(raco)(TestRange{})), const Pinned&>);
+        static_assert(same_as<decltype(move(raco)(TestRange{})), const Pinned&>);
+        static_assert(same_as<decltype(move(as_const(raco))(TestRange{})), const Pinned&>);
+    }
+    {
+
+        auto combined_pipeline = RangeIdentity{} | PinnedReturningRaco{};
+
+        static_assert(same_as<decltype(TestRange{} | combined_pipeline), const Pinned&>);
+        static_assert(same_as<decltype(TestRange{} | as_const(combined_pipeline)), const Pinned&>);
+        static_assert(same_as<decltype(TestRange{} | move(combined_pipeline)), const Pinned&>);
+        static_assert(same_as<decltype(TestRange{} | move(as_const(combined_pipeline))), const Pinned&>);
+
+        static_assert(same_as<decltype(combined_pipeline(TestRange{})), const Pinned&>);
+        static_assert(same_as<decltype(as_const(combined_pipeline)(TestRange{})), const Pinned&>);
+        static_assert(same_as<decltype(move(combined_pipeline)(TestRange{})), const Pinned&>);
+        static_assert(same_as<decltype(move(as_const(combined_pipeline))(TestRange{})), const Pinned&>);
+    }
+    {
+        auto weird_pipeline     = ConstOnlyRangeIdentity{} | PinnedReturningRaco{};
+        using WeirdPipelineType = decltype(weird_pipeline);
+
+        static_assert(same_as<decltype(TestRange{} | as_const(weird_pipeline)), const Pinned&>);
+        static_assert(same_as<decltype(TestRange{} | move(as_const(weird_pipeline))), const Pinned&>);
+
+        static_assert(!CanPipe<WeirdPipelineType, TestRange>);
+        static_assert(!CanPipe<WeirdPipelineType&, TestRange>);
+
+        static_assert(same_as<decltype(as_const(weird_pipeline)(TestRange{})), const Pinned&>);
+        static_assert(same_as<decltype(move(as_const(weird_pipeline))(TestRange{})), const Pinned&>);
+
+        static_assert(!invocable<WeirdPipelineType, TestRange>);
+        static_assert(!invocable<WeirdPipelineType&, TestRange>);
+    }
+}
+
 int main() {
     assert(test_user_defined_adaptors());
     static_assert(test_user_defined_adaptors());