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