[PATCH] libstdc++: Fix capturing of lvalue references in_RangeAdaptor::operator()
Patrick Palka
ppalka@redhat.com
Thu Feb 20 17:19:00 GMT 2020
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
More information about the Gcc-patches
mailing list