From patchwork Sun Apr 28 11:38:33 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 240246 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id A988D2C00BC for ; Sun, 28 Apr 2013 21:38:45 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:date:message-id:subject:from:to:content-type; q= dns; s=default; b=Szlefjw0DtumpqQHX6yH70Pr/lBpIK/dZ4Kro392iR1FY7 8Z/TH1r0XuHXO3mQpYjO50T51KyktSbBkxpWIjEIF3Bje19tldaDFP8YtDW8YOjT xkhGsOuv7uMOPbiDvIPfCK56Z2nhv8NfEd4A1fyLQVgfVE1tm/T33ceipu2dQ= 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 :mime-version:date:message-id:subject:from:to:content-type; s= default; bh=U5tXTWCKl5VszKt0SQo2J08tv/8=; b=aglQ6edsIOLT77q3GlLj vtFv0YvCfgB6Melqqk1lktf5jwGxQMJZeMenysfoL/djRnCM6LHtugNmalY69GRN CC5i5KYNVxk472yXDZfAVnXZOhkxqCFOCsp+SES1lGH6ez3Top84tkQUa9Prapnx NRhMjwyWDWDrpvQ3elRDsFU= Received: (qmail 2676 invoked by alias); 28 Apr 2013 11:38:39 -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 2658 invoked by uid 89); 28 Apr 2013 11:38:38 -0000 X-Spam-SWARE-Status: No, score=-2.8 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE, SPF_PASS autolearn=ham version=3.3.1 X-Spam-User: qpsmtpd, 2 recipients Received: from mail-lb0-f181.google.com (HELO mail-lb0-f181.google.com) (209.85.217.181) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Sun, 28 Apr 2013 11:38:36 +0000 Received: by mail-lb0-f181.google.com with SMTP id 13so2809994lba.40 for ; Sun, 28 Apr 2013 04:38:33 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.152.8.231 with SMTP id u7mr1011547laa.27.1367149113868; Sun, 28 Apr 2013 04:38:33 -0700 (PDT) Received: by 10.112.166.101 with HTTP; Sun, 28 Apr 2013 04:38:33 -0700 (PDT) Date: Sun, 28 Apr 2013 12:38:33 +0100 Message-ID: Subject: [patch] libstdc++/51365 for shared_ptr From: Jonathan Wakely To: "libstdc++" , gcc-patches X-Virus-Found: No This fixes shared_ptr to allow 'final' allocators to be used. As an added bonus it also reduces the memory footprint of the shared_ptr control block when constructing a shared_ptr with an empty deleter or when using make_shared/allocate_shared. I decided not to use std::tuple here, because it's a pretty heavy template to instantiate, so added another EBO helper like the one in hashtable_policy.h -- they should be merged and reused for the other containers to fix the rest of PR 51365. PR libstdc++/51365 * include/bits/shared_ptr_base (_Sp_ebo_helper): Helper class to implement EBO safely. (_Sp_counted_base::_M_get_deleter): Add noexcept. (_Sp_counter_ptr): Use noexcept instead of comments. (_Sp_counted_deleter): Likewise. Use _Sp_ebo_helper. (_Sp_counted_ptr_inplace): Likewise. * testsuite/20_util/shared_ptr/cons/51365.cc: New. * testsuite/20_util/shared_ptr/cons/52924.cc: Add rebind member to custom allocator and test construction with custom allocator. * testsuite/20_util/shared_ptr/cons/43820_neg.cc: Adjust dg-error line number. Tested x86_64-linux, committed to trunk. commit 4c5977aae386fa8c60519a8c7b9ba3e448f43c22 Author: Jonathan Wakely Date: Sun Apr 28 12:10:00 2013 +0100 PR libstdc++/51365 * include/bits/shared_ptr_base (_Sp_ebo_helper): Helper class to implement EBO safely. (_Sp_counted_base::_M_get_deleter): Add noexcept. (_Sp_counter_ptr): Use noexcept instead of comments. (_Sp_counted_deleter): Likewise. Use _Sp_ebo_helper. (_Sp_counted_ptr_inplace): Likewise. * testsuite/20_util/shared_ptr/cons/51365.cc: New. * testsuite/20_util/shared_ptr/cons/52924.cc: Add rebind member to custom allocator and test construction with custom allocator. * testsuite/20_util/shared_ptr/cons/43820_neg.cc: Adjust dg-error line number. diff --git a/libstdc++-v3/include/bits/shared_ptr_base.h b/libstdc++-v3/include/bits/shared_ptr_base.h index f463645..a0f513f 100644 --- a/libstdc++-v3/include/bits/shared_ptr_base.h +++ b/libstdc++-v3/include/bits/shared_ptr_base.h @@ -126,7 +126,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { delete this; } virtual void* - _M_get_deleter(const std::type_info&) = 0; + _M_get_deleter(const std::type_info&) noexcept = 0; void _M_add_ref_copy() @@ -284,7 +284,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { public: explicit - _Sp_counted_ptr(_Ptr __p) + _Sp_counted_ptr(_Ptr __p) noexcept : _M_ptr(__p) { } virtual void @@ -296,14 +296,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { delete this; } virtual void* - _M_get_deleter(const std::type_info&) - { return 0; } + _M_get_deleter(const std::type_info&) noexcept + { return nullptr; } _Sp_counted_ptr(const _Sp_counted_ptr&) = delete; _Sp_counted_ptr& operator=(const _Sp_counted_ptr&) = delete; - protected: - _Ptr _M_ptr; // copy constructor must not throw + private: + _Ptr _M_ptr; }; template<> @@ -318,59 +318,91 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION inline void _Sp_counted_ptr::_M_dispose() noexcept { } + template + struct _Sp_ebo_helper; + + /// Specialization using EBO. + template + struct _Sp_ebo_helper<_Nm, _Tp, true> : private _Tp + { + explicit _Sp_ebo_helper(const _Tp& __tp) : _Tp(__tp) { } + + static _Tp& + _S_get(_Sp_ebo_helper& __eboh) { return static_cast<_Tp&>(__eboh); } + }; + + /// Specialization not using EBO. + template + struct _Sp_ebo_helper<_Nm, _Tp, false> + { + explicit _Sp_ebo_helper(const _Tp& __tp) : _M_tp(__tp) { } + + static _Tp& + _S_get(_Sp_ebo_helper& __eboh) + { return __eboh._M_tp; } + + private: + _Tp _M_tp; + }; + // Support for custom deleter and/or allocator template class _Sp_counted_deleter final : public _Sp_counted_base<_Lp> { - // Helper class that stores the Deleter and also acts as an allocator. - // Used to dispose of the owned pointer and the internal refcount - // Requires that copies of _Alloc can free each other's memory. - struct _My_Deleter - : public _Alloc // copy constructor must not throw + class _Impl : _Sp_ebo_helper<0, _Deleter>, _Sp_ebo_helper<1, _Alloc> { - _Deleter _M_del; // copy constructor must not throw - _My_Deleter(_Deleter __d, const _Alloc& __a) - : _Alloc(__a), _M_del(__d) { } + typedef _Sp_ebo_helper<0, _Deleter> _Del_base; + typedef _Sp_ebo_helper<1, _Alloc> _Alloc_base; + + public: + _Impl(_Ptr __p, _Deleter __d, const _Alloc& __a) noexcept + : _M_ptr(__p), _Del_base(__d), _Alloc_base(__a) + { } + + _Deleter& _M_del() noexcept { return _Del_base::_S_get(*this); } + _Alloc& _M_alloc() noexcept { return _Alloc_base::_S_get(*this); } + + _Ptr _M_ptr; }; public: // __d(__p) must not throw. - _Sp_counted_deleter(_Ptr __p, _Deleter __d) - : _M_ptr(__p), _M_del(__d, _Alloc()) { } + _Sp_counted_deleter(_Ptr __p, _Deleter __d) noexcept + : _M_impl(__p, __d, _Alloc()) { } // __d(__p) must not throw. - _Sp_counted_deleter(_Ptr __p, _Deleter __d, const _Alloc& __a) - : _M_ptr(__p), _M_del(__d, __a) { } + _Sp_counted_deleter(_Ptr __p, _Deleter __d, const _Alloc& __a) noexcept + : _M_impl(__p, __d, __a) { } ~_Sp_counted_deleter() noexcept { } virtual void _M_dispose() noexcept - { _M_del._M_del(_M_ptr); } + { _M_impl._M_del()(_M_impl._M_ptr); } virtual void _M_destroy() noexcept { typedef typename allocator_traits<_Alloc>::template rebind_traits<_Sp_counted_deleter> _Alloc_traits; - typename _Alloc_traits::allocator_type __a(_M_del); + typename _Alloc_traits::allocator_type __a(_M_impl._M_alloc()); _Alloc_traits::destroy(__a, this); _Alloc_traits::deallocate(__a, this, 1); } virtual void* - _M_get_deleter(const std::type_info& __ti) + _M_get_deleter(const std::type_info& __ti) noexcept { #ifdef __GXX_RTTI - return __ti == typeid(_Deleter) ? &_M_del._M_del : 0; + return __ti == typeid(_Deleter) ? &_M_impl._M_del() : nullptr; #else - return 0; + return nullptr; #endif } - protected: - _Ptr _M_ptr; // copy constructor must not throw - _My_Deleter _M_del; // copy constructor must not throw + private: + _Impl _M_impl; }; // helpers for make_shared / allocate_shared @@ -380,25 +412,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template class _Sp_counted_ptr_inplace final : public _Sp_counted_base<_Lp> { - // Helper class that stores the pointer and also acts as an allocator. - // Used to dispose of the owned pointer and the internal refcount - // Requires that copies of _Alloc can free each other's memory. - struct _Impl - : public _Alloc // copy constructor must not throw + class _Impl : _Sp_ebo_helper<0, _Alloc> { - _Impl(_Alloc __a) : _Alloc(__a), _M_ptr() { } - _Tp* _M_ptr; + typedef _Sp_ebo_helper<0, _Alloc> _A_base; + + public: + explicit _Impl(_Alloc __a) noexcept : _A_base(__a) { } + + _Alloc& _M_alloc() noexcept { return _A_base::_S_get(*this); } + + __gnu_cxx::__aligned_buffer<_Tp> _M_storage; }; public: template _Sp_counted_ptr_inplace(_Alloc __a, _Args&&... __args) - : _M_impl(__a), _M_storage() + : _M_impl(__a) { - _M_impl._M_ptr = _M_storage._M_ptr(); // _GLIBCXX_RESOLVE_LIB_DEFECTS // 2070. allocate_shared should use allocator_traits::construct - allocator_traits<_Alloc>::construct(__a, _M_impl._M_ptr, + allocator_traits<_Alloc>::construct(__a, _M_ptr(), std::forward<_Args>(__args)...); // might throw } @@ -406,7 +439,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION virtual void _M_dispose() noexcept - { allocator_traits<_Alloc>::destroy(_M_impl, _M_impl._M_ptr); } + { + allocator_traits<_Alloc>::destroy(_M_impl._M_alloc(), _M_ptr()); + } // Override because the allocator needs to know the dynamic type virtual void @@ -414,7 +449,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typedef typename allocator_traits<_Alloc>::template rebind_traits<_Sp_counted_ptr_inplace> _Alloc_traits; - typename _Alloc_traits::allocator_type __a(_M_impl); + typename _Alloc_traits::allocator_type __a(_M_impl._M_alloc()); _Alloc_traits::destroy(__a, this); _Alloc_traits::deallocate(__a, this, 1); } @@ -424,17 +459,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _M_get_deleter(const std::type_info& __ti) noexcept { #ifdef __GXX_RTTI - return __ti == typeid(_Sp_make_shared_tag) ? _M_storage._M_addr() : 0; + return __ti == typeid(_Sp_make_shared_tag) ? _M_ptr() : nullptr; #else - return 0; + return nullptr; #endif } private: + _Tp* _M_ptr() noexcept { return _M_impl._M_storage._M_ptr(); } + _Impl _M_impl; - __gnu_cxx::__aligned_buffer<_Tp> _M_storage; }; + template<_Lock_policy _Lp> class __shared_count { @@ -592,7 +629,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION void* _M_get_deleter(const std::type_info& __ti) const noexcept - { return _M_pi ? _M_pi->_M_get_deleter(__ti) : 0; } + { return _M_pi ? _M_pi->_M_get_deleter(__ti) : nullptr; } bool _M_less(const __shared_count& __rhs) const noexcept diff --git a/libstdc++-v3/testsuite/20_util/shared_ptr/cons/43820_neg.cc b/libstdc++-v3/testsuite/20_util/shared_ptr/cons/43820_neg.cc index 3a5f053..b6d1009 100644 --- a/libstdc++-v3/testsuite/20_util/shared_ptr/cons/43820_neg.cc +++ b/libstdc++-v3/testsuite/20_util/shared_ptr/cons/43820_neg.cc @@ -32,7 +32,7 @@ void test01() { X* px = 0; std::shared_ptr p1(px); // { dg-error "here" } - // { dg-error "incomplete" "" { target *-*-* } 770 } + // { dg-error "incomplete" "" { target *-*-* } 807 } std::shared_ptr p9(ap()); // { dg-error "here" } // { dg-error "incomplete" "" { target *-*-* } 307 } diff --git a/libstdc++-v3/testsuite/20_util/shared_ptr/cons/51365.cc b/libstdc++-v3/testsuite/20_util/shared_ptr/cons/51365.cc new file mode 100644 index 0000000..757e7eb --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/shared_ptr/cons/51365.cc @@ -0,0 +1,51 @@ +// { dg-options "-std=gnu++0x" } +// { dg-do compile } + +// Copyright (C) 2012-2013 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 +// . + +#include + +// libstdc++/51365 +// Test with 'final' deleter and allocator. + +struct A { }; + +struct D final +{ + void operator()(A*) { } +}; + +template +struct Alloc final : std::allocator +{ + Alloc() = default; + template Alloc(const Alloc&) { } + + template + struct rebind + { typedef Alloc other; }; +}; + +A a; +D d; + +Alloc al; + +auto sd = std::shared_ptr(&a, d); +auto sa = std::shared_ptr(&a, d, al); +auto as = std::allocate_shared(al); diff --git a/libstdc++-v3/testsuite/20_util/shared_ptr/cons/52924.cc b/libstdc++-v3/testsuite/20_util/shared_ptr/cons/52924.cc index b6c47ce..6949c36 100644 --- a/libstdc++-v3/testsuite/20_util/shared_ptr/cons/52924.cc +++ b/libstdc++-v3/testsuite/20_util/shared_ptr/cons/52924.cc @@ -22,14 +22,12 @@ // libstdc++/52924 -struct A { } a; +struct A { }; struct D { ~D() noexcept(false) { } void operator()(A*) { } -} d; - -auto sp = std::shared_ptr(&a, d); +}; template struct Alloc : std::allocator @@ -37,8 +35,16 @@ struct Alloc : std::allocator Alloc() = default; ~Alloc() noexcept(false) { } template Alloc(const Alloc&) { } + + template + struct rebind + { typedef Alloc other; }; }; +A a; +D d; Alloc al; +auto sd = std::shared_ptr(&a, d); +auto sa = std::shared_ptr(&a, d, al); auto as = std::allocate_shared(al);