From patchwork Wed Jan 11 12:21:25 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 90878 Delivered-To: patch@linaro.org Received: by 10.140.20.99 with SMTP id 90csp1093710qgi; Wed, 11 Jan 2017 04:22:05 -0800 (PST) X-Received: by 10.84.218.205 with SMTP id g13mr12680860plm.78.1484137325910; Wed, 11 Jan 2017 04:22:05 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id l36si5699607plg.145.2017.01.11.04.22.05 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 11 Jan 2017 04:22:05 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-445856-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-445856-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-445856-patch=linaro.org@gcc.gnu.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=ZFa+EZKNKlP4h4oxq z18zsJbNp80tLajD1u9MGYq/jJQ5uzUugxIytUclXhYZVLz2jdr2JLsjG9Vu8Yzp 4cScMl/rSwXK6/uNywzeEeHaoDbf845r22kVIyv46dSMWtNjG1NStcFEW9ER4HSp zT7O8vvqVYbcGuxAYCzSkRzzF4= 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:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=eyg0MUBZO+Fjy2KmhcBvegp 4ilM=; b=BV+f9fWVXt+5M68/c85FGAv+uHtf/Ty/mha7Ai7f2LS7wtyzrtFNfi0 sA2sqB+6CRodaFhftai73jDSKHyBtmi8YI3Kfr5WOyNkWS91RT0FUrxRtipOMq7i FoWcC1KtNxP+RxuSgsVFf7aPM0bVfRhY5Q0FjzuB5hdWS5Z/Md+s= Received: (qmail 7690 invoked by alias); 11 Jan 2017 12:21: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 7659 invoked by uid 89); 11 Jan 2017 12:21:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-5.1 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=cumbersome, Song, H*i:sk:Q6dsy2g, H*i:CAPQZVxuKn7gOa X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 11 Jan 2017 12:21:27 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A5F2980F7B; Wed, 11 Jan 2017 12:21:27 +0000 (UTC) Received: from localhost (ovpn-116-26.ams2.redhat.com [10.36.116.26]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v0BCLQEf001811; Wed, 11 Jan 2017 07:21:26 -0500 Date: Wed, 11 Jan 2017 12:21:25 +0000 From: Jonathan Wakely To: Tim Song Cc: libstdc++ , gcc-patches Subject: Re: [PATCH] PR77528 add default constructors for container adaptors Message-ID: <20170111122125.GS13348@redhat.com> References: <20170110173312.GM13348@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.7.1 (2016-10-04) On 10/01/17 13:15 -0500, Tim Song wrote: >On Tue, Jan 10, 2017 at 12:33 PM, Jonathan Wakely wrote: >> The standard says that the container adaptors have a constructor with >> a default argument, which serves as a default constructor. That >> involves default-constructing the underlying sequence as the default >> argument and then move-constructing the member variable from that >> argument. Because std::deque allocates memory in its move constructor >> this means the default constructor of an adaptor using std::deque >> will allocate twice, which is wasteful and expensive. >> >> This change adds a separate default constructor, defined as defaulted >> (and adding default member-initializers to ensure the member variables >> get value-initialized). This avoids the move-construction, so we only >> allocate once when using std::deque. >> > >The new default member initializers use {}, and it's not too hard to find >test cases where {} and value-initialization do different things, including >cases where {} doesn't compile but () does. (So much for "uniform" >initialization.) OK that's easily fixed. >> Because the default constructor is defined as defaulted it will be >> deleted when the underlying sequence isn't default constructible, > >That's not correct. There's no implicit deletion due to the presence of DMIs. >The reason the explicit instantiation works is that constructors explicitly >defaulted at their first declaration are not implicitly defined until ODR-used. > >So, unlike the SFINAE-based approach outlined in the bugzilla issue, this >patch causes is_default_constructible> >to trigger a hard error (though the standard's version isn't SFINAE-friendly >either). Oops, thanks. >Also, the change to .../priority_queue/requirements/explicit_instantiation/1.cc >adds a Cmp class but doesn't actually use it in the explicit instantiation. Fixed. This patch uses the _Enable_default_constructor mixin to properly delete the default constructors. It's a bit cumbersome, because we have to add an initializer for the base class to every ctor-initializer-list, but I think I prefer this to making the default constructor a constrained template. This also includes tests for is_default_constructible to ensure we don't get the same hard errors as the previous version. I'm still testing this, and could be persuaded to go with the constrained templates if there's a good reason to. commit 2bd10a76508336d92decfefe858fc23c5bc272f0 Author: Jonathan Wakely Date: Wed Jan 11 12:15:27 2017 +0000 PR77528 conditionally delete default constructors PR libstdc++/77528 * include/bits/stl_queue.h (queue, priority_queue): Use derivation from _Enable_default_constructor mixin to conditionally delete disable default construction * include/bits/stl_stack.h (stack): Likewise. * testsuite/23_containers/priority_queue/requirements/constructible.cc: New. * testsuite/23_containers/priority_queue/requirements/ explicit_instantiation/1.cc: Test more instantiations. * testsuite/23_containers/priority_queue/requirements/ explicit_instantiation/1_c++98.cc: Likewise. * testsuite/23_containers/queue/requirements/constructible.cc: New. * testsuite/23_containers/stack/requirements/constructible.cc: New. diff --git a/libstdc++-v3/include/bits/stl_queue.h b/libstdc++-v3/include/bits/stl_queue.h index 6417b30..ebd6089 100644 --- a/libstdc++-v3/include/bits/stl_queue.h +++ b/libstdc++-v3/include/bits/stl_queue.h @@ -60,6 +60,7 @@ #include #if __cplusplus >= 201103L # include +# include #endif namespace std _GLIBCXX_VISIBILITY(default) @@ -94,6 +95,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION */ template > class queue +#if __cplusplus >= 201103L + : _Enable_default_constructor::value, + queue<_Tp, _Sequence>> +#endif { // concept requirements typedef typename _Sequence::value_type _Sequence_value_type; @@ -114,6 +119,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template using _Uses = typename enable_if::value>::type; + + using _Tag = _Enable_default_constructor_tag; + using _Base = _Enable_default_constructor< + is_default_constructible<_Sequence>::value, queue>; #endif public: @@ -133,7 +142,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION */ /// @c c is the underlying container. #if __cplusplus >= 201103L - _Sequence c{}; + _Sequence c = _Sequence(); #else _Sequence c; #endif @@ -151,32 +160,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION explicit queue(const _Sequence& __c) - : c(__c) { } + : _Base(_Tag()), c(__c) { } explicit queue(_Sequence&& __c) - : c(std::move(__c)) { } + : _Base(_Tag()), c(std::move(__c)) { } template> explicit queue(const _Alloc& __a) - : c(__a) { } + : _Base(_Tag()), c(__a) { } template> queue(const _Sequence& __c, const _Alloc& __a) - : c(__c, __a) { } + : _Base(_Tag()), c(__c, __a) { } template> queue(_Sequence&& __c, const _Alloc& __a) - : c(std::move(__c), __a) { } + : _Base(_Tag()), c(std::move(__c), __a) { } template> queue(const queue& __q, const _Alloc& __a) - : c(__q.c, __a) { } + : _Base(_Tag()), c(__q.c, __a) { } template> queue(queue&& __q, const _Alloc& __a) - : c(std::move(__q.c), __a) { } + : _Base(_Tag()), c(std::move(__q.c), __a) { } #endif /** @@ -418,6 +427,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template, typename _Compare = less > class priority_queue +#if __cplusplus >= 201103L + : _Enable_default_constructor<__and_, + is_default_constructible<_Compare>>::value, + priority_queue<_Tp, _Sequence, _Compare>> +#endif { // concept requirements typedef typename _Sequence::value_type _Sequence_value_type; @@ -432,6 +446,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template using _Uses = typename enable_if::value>::type; + + using _Tag = _Enable_default_constructor_tag; + using _Base = _Enable_default_constructor< + __and_, + is_default_constructible<_Compare>>::value, + priority_queue>; #endif public: @@ -447,11 +467,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION protected: // See queue::c for notes on these names. #if __cplusplus >= 201103L - _Sequence c{}; - _Compare comp{}; + _Sequence c = _Sequence(); + _Compare comp = _Compare(); #else _Sequence c; - _Compare comp; + _Compare comp; #endif public: @@ -470,40 +490,41 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION explicit priority_queue(const _Compare& __x, const _Sequence& __s) - : c(__s), comp(__x) + : _Base(_Tag()), c(__s), comp(__x) { std::make_heap(c.begin(), c.end(), comp); } explicit priority_queue(const _Compare& __x, _Sequence&& __s = _Sequence()) - : c(std::move(__s)), comp(__x) + : _Base(_Tag()), c(std::move(__s)), comp(__x) { std::make_heap(c.begin(), c.end(), comp); } template> explicit priority_queue(const _Alloc& __a) - : c(__a), comp() { } + : _Base(_Tag()), c(__a), comp() { } template> priority_queue(const _Compare& __x, const _Alloc& __a) - : c(__a), comp(__x) { } + : _Base(_Tag()), c(__a), comp(__x) { } template> priority_queue(const _Compare& __x, const _Sequence& __c, const _Alloc& __a) - : c(__c, __a), comp(__x) { } + : _Base(_Tag()), c(__c, __a), comp(__x) { } template> priority_queue(const _Compare& __x, _Sequence&& __c, const _Alloc& __a) - : c(std::move(__c), __a), comp(__x) { } + : _Base(_Tag()), c(std::move(__c), __a), comp(__x) { } template> priority_queue(const priority_queue& __q, const _Alloc& __a) - : c(__q.c, __a), comp(__q.comp) { } + : _Base(_Tag()), c(__q.c, __a), comp(__q.comp) { } template> priority_queue(priority_queue&& __q, const _Alloc& __a) - : c(std::move(__q.c), __a), comp(std::move(__q.comp)) { } + : _Base(_Tag()), c(std::move(__q.c), __a), comp(std::move(__q.comp)) + { } #endif /** @@ -537,7 +558,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION priority_queue(_InputIterator __first, _InputIterator __last, const _Compare& __x, const _Sequence& __s) - : c(__s), comp(__x) + : _Base(_Tag()), c(__s), comp(__x) { __glibcxx_requires_valid_range(__first, __last); c.insert(c.end(), __first, __last); @@ -548,7 +569,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION priority_queue(_InputIterator __first, _InputIterator __last, const _Compare& __x = _Compare(), _Sequence&& __s = _Sequence()) - : c(std::move(__s)), comp(__x) + : _Base(_Tag()), c(std::move(__s)), comp(__x) { __glibcxx_requires_valid_range(__first, __last); c.insert(c.end(), __first, __last); diff --git a/libstdc++-v3/include/bits/stl_stack.h b/libstdc++-v3/include/bits/stl_stack.h index a0f9ee5..dacf91f 100644 --- a/libstdc++-v3/include/bits/stl_stack.h +++ b/libstdc++-v3/include/bits/stl_stack.h @@ -60,6 +60,7 @@ #include #if __cplusplus >= 201103L # include +# include #endif namespace std _GLIBCXX_VISIBILITY(default) @@ -97,6 +98,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION */ template > class stack +#if __cplusplus >= 201103L + : _Enable_default_constructor::value, + stack<_Tp, _Sequence>> +#endif { // concept requirements typedef typename _Sequence::value_type _Sequence_value_type; @@ -118,6 +123,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template using _Uses = typename enable_if::value>::type; + + using _Tag = _Enable_default_constructor_tag; + using _Base = _Enable_default_constructor< + is_default_constructible<_Sequence>::value, stack>; #endif public: @@ -130,7 +139,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION protected: // See queue::c for notes on this name. #if __cplusplus >= 201103L - _Sequence c{}; + _Sequence c = _Sequence(); #else _Sequence c; #endif @@ -149,32 +158,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION explicit stack(const _Sequence& __c) - : c(__c) { } + : _Base(_Tag()), c(__c) { } explicit stack(_Sequence&& __c) - : c(std::move(__c)) { } + : _Base(_Tag()), c(std::move(__c)) { } template> explicit stack(const _Alloc& __a) - : c(__a) { } + : _Base(_Tag()), c(__a) { } template> stack(const _Sequence& __c, const _Alloc& __a) - : c(__c, __a) { } + : _Base(_Tag()), c(__c, __a) { } template> stack(_Sequence&& __c, const _Alloc& __a) - : c(std::move(__c), __a) { } + : _Base(_Tag()), c(std::move(__c), __a) { } template> stack(const stack& __q, const _Alloc& __a) - : c(__q.c, __a) { } + : _Base(_Tag()), c(__q.c, __a) { } template> stack(stack&& __q, const _Alloc& __a) - : c(std::move(__q.c), __a) { } + : _Base(_Tag()), c(std::move(__q.c), __a) { } #endif /** diff --git a/libstdc++-v3/testsuite/23_containers/priority_queue/requirements/constructible.cc b/libstdc++-v3/testsuite/23_containers/priority_queue/requirements/constructible.cc new file mode 100644 index 0000000..fa412f3 --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/priority_queue/requirements/constructible.cc @@ -0,0 +1,49 @@ +// { dg-do compile } + +// Copyright (C) 2017 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 +// . + + +// This file tests explicit instantiation of library containers. + +#include + +using std::priority_queue; +using std::vector; + +template +constexpr bool default_constructible() +{ return std::is_default_constructible::value; } + +static_assert(default_constructible>(), + "priority_queue"); + +struct NonDefaultConstructible : vector { + NonDefaultConstructible(int) { } +}; +struct Cmp : std::less { + Cmp(int) { } +}; +static_assert( + !default_constructible>(), + "priority_queue"); +static_assert( + !default_constructible>(), + "priority_queue"); +static_assert( + !default_constructible, Cmp>>(), + "priority_queue, Cmp>"); diff --git a/libstdc++-v3/testsuite/23_containers/priority_queue/requirements/explicit_instantiation/1.cc b/libstdc++-v3/testsuite/23_containers/priority_queue/requirements/explicit_instantiation/1.cc index 77cade0..6386d1d 100644 --- a/libstdc++-v3/testsuite/23_containers/priority_queue/requirements/explicit_instantiation/1.cc +++ b/libstdc++-v3/testsuite/23_containers/priority_queue/requirements/explicit_instantiation/1.cc @@ -31,3 +31,5 @@ struct Cmp : std::less { Cmp(int) { } }; template class std::priority_queue; +template class std::priority_queue; +template class std::priority_queue, Cmp>; diff --git a/libstdc++-v3/testsuite/23_containers/priority_queue/requirements/explicit_instantiation/1_c++98.cc b/libstdc++-v3/testsuite/23_containers/priority_queue/requirements/explicit_instantiation/1_c++98.cc index 0b5b287..c9530cb 100644 --- a/libstdc++-v3/testsuite/23_containers/priority_queue/requirements/explicit_instantiation/1_c++98.cc +++ b/libstdc++-v3/testsuite/23_containers/priority_queue/requirements/explicit_instantiation/1_c++98.cc @@ -30,4 +30,6 @@ struct NonDefaultConstructible : std::vector { struct Cmp : std::less { Cmp(int) { } }; +template class std::priority_queue; template class std::priority_queue; +template class std::priority_queue, Cmp>; diff --git a/libstdc++-v3/testsuite/23_containers/queue/requirements/constructible.cc b/libstdc++-v3/testsuite/23_containers/queue/requirements/constructible.cc new file mode 100644 index 0000000..99a8b84 --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/queue/requirements/constructible.cc @@ -0,0 +1,37 @@ +// { dg-do compile } + +// Copyright (C) 2017 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 +// . + + +// This file tests explicit instantiation of library containers. + +#include + +using std::queue; + +template +constexpr bool default_constructible() +{ return std::is_default_constructible::value; } + +static_assert(default_constructible>(), "queue"); + +struct NonDefaultConstructible : std::deque { + NonDefaultConstructible(int) { } +}; +static_assert(!default_constructible>(), + "queue"); diff --git a/libstdc++-v3/testsuite/23_containers/stack/requirements/constructible.cc b/libstdc++-v3/testsuite/23_containers/stack/requirements/constructible.cc new file mode 100644 index 0000000..0d6e174 --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/stack/requirements/constructible.cc @@ -0,0 +1,37 @@ +// { dg-do compile } + +// Copyright (C) 2017 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 +// . + + +// This file tests explicit instantiation of library containers. + +#include + +using std::stack; + +template +constexpr bool default_constructible() +{ return std::is_default_constructible::value; } + +static_assert(default_constructible>(), "stack"); + +struct NonDefaultConstructible : std::deque { + NonDefaultConstructible(int) { } +}; +static_assert(!default_constructible>(), + "stack");