[gcc/devel/ranger] libstdc++: Fix capturing of lvalue references in_RangeAdaptor::operator()

[gcc/devel/ranger] libstdc++: Fix capturing of lvalue references in_RangeAdaptor::operator()

[Aldy Hernandez]

https://gcc.gnu.org/g:6e63438a0d7085e508d82063af2f04d26fa46494

commit 6e63438a0d7085e508d82063af2f04d26fa46494
Author: Patrick Palka <ppalka@redhat.com>
Date:   Wed Feb 19 14:10:32 2020 -0500

    libstdc++: Fix capturing of lvalue references in_RangeAdaptor::operator()
    
    This fixes a dangling-reference issue with views::split and other multi-argument
    adaptors that may take its extra arguments by reference.
    
    When creating the _RangeAdaptorClosure in _RangeAdaptor::operator(), we
    currently capture all provided arguments by value.  When we then use the
    _RangeAdaptorClosure and call it with a range, as in e.g.
    
        v = views::split(p)(range),
    
    we forward the range and the captures to the underlying adaptor routine.  But
    then when the temporary _RangeAdaptorClosure goes out of scope, the by-value
    captures get destroyed and the references to these captures in the resulting view
    become dangling.
    
    This patch fixes this problem by capturing lvalue references by reference in
    _RangeAdaptorClosure::operator(), and then forwarding the captures appropriately
    to the underlying adaptor routine.
    
    libstdc++-v3/ChangeLog:
    
            * include/std/ranges (views::__adaptor::__maybe_refwrap): New utility
            function.
            (views::__adaptor::_RangeAdaptor::operator()): Add comments.  Use
            __maybe_refwrap to capture lvalue references by reference, and then use
            unwrap_reference_t to forward the by-reference captures as references.
            * testsuite/std/ranges/adaptors/split.cc: Augment test.
            * testsuite/std/ranges/adaptors/split_neg.cc: New test.

Diff:
---
 libstdc++-v3/ChangeLog                             |  8 ++++
 libstdc++-v3/include/std/ranges                    | 50 ++++++++++++++++++++--
 .../testsuite/std/ranges/adaptors/split.cc         | 18 ++++++++
 .../testsuite/std/ranges/adaptors/split_neg.cc     | 49 +++++++++++++++++++++
 4 files changed, 122 insertions(+), 3 deletions(-)

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index deade1c29c0..3649864644e 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,5 +1,13 @@
 2020-02-20  Patrick Palka  <ppalka@redhat.com>
 
+	* include/std/ranges (views::__adaptor::__maybe_refwrap): New utility
+	function.
+	(views::__adaptor::_RangeAdaptor::operator()): Add comments.  Use
+	__maybe_refwrap to capture lvalue references by reference, and then use
+	unwrap_reference_t to forward the by-reference captures as references.
+	* testsuite/std/ranges/adaptors/split.cc: Augment test.
+	* testsuite/std/ranges/adaptors/split_neg.cc: New test.
+
 	* include/std/ranges (iota_view): Forward declare _Sentinel.
 	(iota_view::_Iterator): Befriend _Sentinel.
 	(iota_view::_Sentinel::_M_equal): New member function.
diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 6358ce86f79..8c925fa278b 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -1051,6 +1051,21 @@ namespace views
 {
   namespace __adaptor
   {
+    template<typename _Tp>
+      inline constexpr auto
+      __maybe_refwrap(_Tp& __arg)
+      { return reference_wrapper<_Tp>{__arg}; }
+
+    template<typename _Tp>
+      inline constexpr auto
+      __maybe_refwrap(const _Tp& __arg)
+      { return reference_wrapper<const _Tp>{__arg}; }
+
+    template<typename _Tp>
+      inline constexpr decltype(auto)
+      __maybe_refwrap(_Tp&& __arg)
+      { return std::forward<_Tp>(__arg); }
+
     template<typename _Callable>
       struct _RangeAdaptorClosure;
 
@@ -1079,18 +1094,47 @@ namespace views
 	  constexpr auto
 	  operator()(_Args&&... __args) const
 	  {
+	    // [range.adaptor.object]: If a range adaptor object accepts more
+	    // than one argument, then the following expressions are equivalent:
+	    //
+	    //   (1) adaptor(range, args...)
+	    //   (2) adaptor(args...)(range)
+	    //   (3) range | adaptor(args...)
+	    //
+	    // In this case, adaptor(args...) is a range adaptor closure object.
+	    //
+	    // We handle (1) and (2) here, and (3) is just a special case of a
+	    // more general case already handled by _RangeAdaptorClosure.
 	    if constexpr (is_invocable_v<_Callable, _Args...>)
 	      {
 		static_assert(sizeof...(_Args) != 1,
 			      "a _RangeAdaptor that accepts only one argument "
 			      "should be defined as a _RangeAdaptorClosure");
+		// Here we handle adaptor(range, args...) -- just forward all
+		// arguments to the underlying adaptor routine.
 		return _Callable{}(std::forward<_Args>(__args)...);
 	      }
 	    else
 	      {
-		auto __closure = [__args...] <typename _Range> (_Range&& __r) {
-		  return _Callable{}(std::forward<_Range>(__r), __args...);
-		};
+		// Here we handle adaptor(args...)(range).
+		// Given args..., we return a _RangeAdaptorClosure that takes a
+		// range argument, such that (2) is equivalent to (1).
+		//
+		// We need to be careful about how we capture args... in this
+		// closure.  By using __maybe_refwrap, we capture lvalue
+		// references by reference (through a reference_wrapper) and
+		// otherwise capture by value.
+		auto __closure
+		  = [...__args(__maybe_refwrap(std::forward<_Args>(__args)))]
+		    <typename _Range> (_Range&& __r) {
+		      // This static_cast has two purposes: it forwards a
+		      // reference_wrapper<T> capture as a T&, and otherwise
+		      // forwards the captured argument as an rvalue.
+		      return _Callable{}(std::forward<_Range>(__r),
+			       (static_cast<unwrap_reference_t
+					    <remove_const_t<decltype(__args)>>>
+				(__args))...);
+		    };
 		using _ClosureType = decltype(__closure);
 		return _RangeAdaptorClosure<_ClosureType>(std::move(__closure));
 	      }
diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
index 8b3bfcc0930..e25031c0aea 100644
--- a/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
+++ b/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
@@ -74,10 +74,28 @@ test03()
   VERIFY( i == v.end() );
 }
 
+void
+test04()
+{
+  auto x = "the  quick  brown  fox"sv;
+  std::initializer_list<char> p = {' ', ' '};
+  static_assert(!ranges::view<decltype(p)>);
+  static_assert(std::same_as<decltype(p | views::all),
+			     ranges::ref_view<decltype(p)>>);
+  auto v = x | views::split(p);
+  auto i = v.begin();
+  VERIFY( ranges::equal(*i++, "the"sv) );
+  VERIFY( ranges::equal(*i++, "quick"sv) );
+  VERIFY( ranges::equal(*i++, "brown"sv) );
+  VERIFY( ranges::equal(*i++, "fox"sv) );
+  VERIFY( i == v.end() );
+}
+
 int
 main()
 {
   test01();
   test02();
   test03();
+  test04();
 }
diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/split_neg.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/split_neg.cc
new file mode 100644
index 00000000000..6b876c57591
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/ranges/adaptors/split_neg.cc
@@ -0,0 +1,49 @@
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <algorithm>
+#include <ranges>
+#include <string_view>
+
+namespace ranges = std::ranges;
+namespace views = std::ranges::views;
+
+void
+test01()
+{
+  using namespace std::literals;
+  auto x = "the  quick  brown  fox"sv;
+  auto v = views::split(x, std::initializer_list<char>{' ', ' '});
+  v.begin(); // { dg-error "" }
+}
+
+void
+test02()
+{
+  using namespace std::literals;
+  auto x = "the  quick  brown  fox"sv;
+  auto v = x | views::split(std::initializer_list<char>{' ', ' '}); // { dg-error "no match" }
+  v.begin();
+}
+
+// { dg-prune-output "in requirements" }
+// { dg-error "deduction failed" "" { target *-*-* } 0 }
+// { dg-error "no match" "" { target *-*-* } 0 }
+// { dg-error "constraint failure" "" { target *-*-* } 0 }