From 6e63438a0d7085e508d82063af2f04d26fa46494 Mon Sep 17 00:00:00 2001 From: Patrick Palka Date: Wed, 19 Feb 2020 14:10:32 -0500 Subject: [PATCH] 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. --- libstdc++-v3/ChangeLog | 8 +++ libstdc++-v3/include/std/ranges | 50 +++++++++++++++++-- .../testsuite/std/ranges/adaptors/split.cc | 18 +++++++ .../std/ranges/adaptors/split_neg.cc | 49 ++++++++++++++++++ 4 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/split_neg.cc diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index deade1c29c0..3649864644e 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,5 +1,13 @@ 2020-02-20 Patrick Palka + * 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. diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges index 6358ce86f79..8c925fa278b 100644 --- a/libstdc++-v3/include/std/ranges +++ b/libstdc++-v3/include/std/ranges @@ -1051,6 +1051,21 @@ namespace views { namespace __adaptor { + template + inline constexpr auto + __maybe_refwrap(_Tp& __arg) + { return reference_wrapper<_Tp>{__arg}; } + + template + inline constexpr auto + __maybe_refwrap(const _Tp& __arg) + { return reference_wrapper{__arg}; } + + template + inline constexpr decltype(auto) + __maybe_refwrap(_Tp&& __arg) + { return std::forward<_Tp>(__arg); } + template 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...] (_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)))] + (_Range&& __r) { + // This static_cast has two purposes: it forwards a + // reference_wrapper capture as a T&, and otherwise + // forwards the captured argument as an rvalue. + return _Callable{}(std::forward<_Range>(__r), + (static_cast>> + (__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 p = {' ', ' '}; + static_assert(!ranges::view); + static_assert(std::same_as>); + 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..6b876c57591 --- /dev/null +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/split_neg.cc @@ -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 +// . + +// { dg-options "-std=gnu++2a" } +// { dg-do compile { target c++2a } } + +#include +#include +#include + +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{' ', ' '}); + 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{' ', ' '}); // { 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.30.2