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.
This commit is contained in:
parent
5586e5060f
commit
6e63438a0d
@ -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.
|
||||
|
@ -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));
|
||||
}
|
||||
|
@ -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();
|
||||
}
|
||||
|
49
libstdc++-v3/testsuite/std/ranges/adaptors/split_neg.cc
Normal file
49
libstdc++-v3/testsuite/std/ranges/adaptors/split_neg.cc
Normal file
@ -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 }
|
Loading…
x
Reference in New Issue
Block a user