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 later 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 capture 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 range adaptor.

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.
---
 libstdc++-v3/include/std/ranges               | 50 ++++++++++++++++--
 .../testsuite/std/ranges/adaptors/split.cc    | 18 +++++++
 .../std/ranges/adaptors/split_neg.cc          | 51 +++++++++++++++++++
 3 files changed, 116 insertions(+), 3 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/split_neg.cc

diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index a2c1be50594..34de6965dcd 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -1072,6 +1072,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;
 
@@ -1100,18 +1115,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..9c4aeec8e27
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/ranges/adaptors/split_neg.cc
@@ -0,0 +1,51 @@
+// 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>
+#include <testsuite_hooks.h>
+#include <testsuite_iterators.h>
+
+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 }
-- 
2.25.1.291.ge68e29171c

Reply via email to