libstdc++: Fix capturing of lvalue references in_RangeAdaptor::operator()
authorPatrick Palka <ppalka@redhat.com>
Wed, 19 Feb 2020 19:10:32 +0000 (14:10 -0500)
committerPatrick Palka <ppalka@redhat.com>
Thu, 20 Feb 2020 19:06:23 +0000 (14:06 -0500)
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.

libstdc++-v3/ChangeLog
libstdc++-v3/include/std/ranges
libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
libstdc++-v3/testsuite/std/ranges/adaptors/split_neg.cc [new file with mode: 0644]

index deade1c29c0ed69cea07c6d6a081b4dbee7881a2..3649864644e505a1b0b125b54d008d4170b4d818 100644 (file)
@@ -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.
index 6358ce86f796d561a9ee773c695006e00a1f6124..8c925fa278b523535f0e6701d0b208ed835ca71e 100644 (file)
@@ -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));
              }
index 8b3bfcc0930ca0bc82b9bcad421778089104a29b..e25031c0aeaaa19768d5f493cd7a12d7f8b95aa1 100644 (file)
@@ -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 (file)
index 0000000..6b876c5
--- /dev/null
@@ -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 }