From patchwork Thu Feb 20 17:19:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Palka X-Patchwork-Id: 1241578 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-519862-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha1 header.s=default header.b=DY9lT62J; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=gfrVRNcM; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48NhDR5b3Xz9sRG for ; Fri, 21 Feb 2020 04:20:05 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:mime-version:content-type :content-transfer-encoding; q=dns; s=default; b=i22shjegUnKR0zZS PGMrDuChQluHIAtPBSpolIE9E3RMOJLra0urzZKJPAN7pURRwvCj5PwWjTfRb4ot ZK53sZ6I69ulS+eySSIpqWR5QrSJ7427U+e3zajTN8loQMicEX3hpnWwsrrEhBUw OIZNEXc+lt5pnVuy0RX5KCE8u18= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:mime-version:content-type :content-transfer-encoding; s=default; bh=yW73a94mIaIJ1AwiKHfnna Ah38U=; b=DY9lT62J22K6VEsIEl8zlC3v+F8AHzczHQ9E4P0/vAGirL0CER24CX KZOc5hD7/FhFcFwxDrMSi+L28pJjrdl5rpxBZg05QyY8hnsrzVAXaY3r2azGsjXe XmDN+VPCLXRW1Row0TK2rmqTgzy9Y3PJhJauyMoN1zyKUFjA4m3zU= Received: (qmail 54444 invoked by alias); 20 Feb 2020 17:19:57 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 54419 invoked by uid 89); 20 Feb 2020 17:19:56 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=appropriately, capturing X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (207.211.31.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 20 Feb 2020 17:19:54 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1582219193; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=KVKYuvtiuLkPG/LscCtuYVC+RRpzoBGMxKy4pMO3i5k=; b=gfrVRNcMM5XCzw66E4Hj5s3I3wXMoFMoPvWgKLjTtPa6giZwIIWkGGJhC/PTDaIRLQamdy PjEu2BCyjrTj3kyhw0iUytFALnLwlaQtaRmZk5YHStNEt0/KKLKXTm3AFopkADY+TM57la cbZtK3467rX50kpQXhUxmQUY+GUO7X0= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-195-xY7xcR3iPXqiPmUv7xrb9w-1; Thu, 20 Feb 2020 12:19:47 -0500 Received: by mail-qk1-f198.google.com with SMTP id c77so3219503qke.7 for ; Thu, 20 Feb 2020 09:19:47 -0800 (PST) Received: from localhost.localdomain (ool-457d493a.dyn.optonline.net. [69.125.73.58]) by smtp.gmail.com with ESMTPSA id l13sm87622qtf.18.2020.02.20.09.19.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 20 Feb 2020 09:19:45 -0800 (PST) From: Patrick Palka To: gcc-patches@gcc.gnu.org Cc: libstdc++@gcc.gnu.org, jwakely@redhat.com, Patrick Palka Subject: [PATCH] libstdc++: Fix capturing of lvalue references in_RangeAdaptor::operator() Date: Thu, 20 Feb 2020 12:19:43 -0500 Message-Id: <20200220171943.1437278-1-ppalka@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-IsSubscribed: yes 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 + 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; @@ -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...] (_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..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 +// . + +// { dg-options "-std=gnu++2a" } +// { dg-do compile { target c++2a } } + +#include +#include +#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 }