From patchwork Thu Jan 12 17:29:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 91198 Delivered-To: patch@linaro.org Received: by 10.140.20.99 with SMTP id 90csp1729235qgi; Thu, 12 Jan 2017 09:30:16 -0800 (PST) X-Received: by 10.84.210.5 with SMTP id z5mr23200130plh.32.1484242216474; Thu, 12 Jan 2017 09:30:16 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id b126si9857496pgc.125.2017.01.12.09.30.16 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 12 Jan 2017 09:30:16 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-445985-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-445985-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-445985-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=XuLPc0pHWT4cakiW+ 4tkU6fXdGWUQfDuZR+XeJHI0tZtxsuc0F7mq2JGpxZBA/6UhEDfPmouVoroz2E4i SubpTZ6IG1TwAD1MR4rvgg8gTOkHrIwn/IeK1eDrPs31MqUUxyuebWn9pxYqoaZi 0ZnjkfGyqQOubNz0lvhnm868rs= 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=q8C+snHGy+Tm6dkPA/mGb19 E8GI=; b=KqALaQwsPhm+T1hcwCWgfE5enFVO/ro1Ce/tlvy9YRYAZUckllUohfz XhLbtiQJ4TSteMNV4mYHxi3/fGKgQEy+1jR24Glg+HAX27kA9zGvOZUlE+cvDUAG TD/uwtKz0Drk+ewkub/XWSnBGKByIQ4rn7YmAVCc5jUQ7MHDD5oE= Received: (qmail 78725 invoked by alias); 12 Jan 2017 17:29:47 -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 78704 invoked by uid 89); 12 Jan 2017 17:29:46 -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=xxx 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; Thu, 12 Jan 2017 17:29:41 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (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 5D0E961B90; Thu, 12 Jan 2017 17:29:41 +0000 (UTC) Received: from localhost (ovpn-116-26.ams2.redhat.com [10.36.116.26]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v0CHTe4f023786; Thu, 12 Jan 2017 12:29:40 -0500 Date: Thu, 12 Jan 2017 17:29:38 +0000 From: Jonathan Wakely To: Tim Song Cc: libstdc++ , gcc-patches Subject: Re: [PATCH] PR77528 add default constructors for container adaptors Message-ID: <20170112172938.GX13348@redhat.com> References: <20170110173312.GM13348@redhat.com> <20170111122125.GS13348@redhat.com> <20170111132511.GT13348@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170111132511.GT13348@redhat.com> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.7.1 (2016-10-04) On 11/01/17 13:25 +0000, Jonathan Wakely wrote: >On 11/01/17 08:04 -0500, Tim Song wrote: >>On Wed, Jan 11, 2017 at 7:21 AM, Jonathan Wakely wrote: >>>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. >>> >> >>I'm happy with either approach - my primary concern is making sure >>that is_constructible and friends work and don't lie, in a world where >>increasing numbers of library components depend on it. Though I'm a > >Yes, it's important that we give the right answer. > >>bit curious as to why you found this approach more preferable. > >I dislike making functions into templates when they aren't "supposed" >to be. But I'm in two minds for this case. It's certainly a smaller, >more self-contained change to just add a default constructor template >and not mess about with a new base class and DMIs and all those >mem-initializers. > >>Re the new DMI, my brain compiler says that _Sequence c = _Sequence(); >>breaks anything with an explicit copy/move constructor pre-C++17, but >>I also don't think we care about those, right? > >I dislike them, but maybe the fact they won't work here is a strong >enough reason to get over my dislike of the original approach and just >do it that way instead. I decided to go with the original solution shown in the PR. Tested powerpc64le-linux, committed to trunk. commit b4614f1ef1a9c98650f2454332bb1407cddff13c Author: Jonathan Wakely Date: Thu Jan 12 15:37:27 2017 +0000 PR77528 partially revert r244278 and define default constructors PR libstdc++/77528 * include/bits/stl_queue.h (queue, priority_queue): Remove default member-initializers and define default constructors as templates with constraints. * 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..3a52367 100644 --- a/libstdc++-v3/include/bits/stl_queue.h +++ b/libstdc++-v3/include/bits/stl_queue.h @@ -131,12 +131,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION * of private: to allow derivation. But none of the other * containers allow for derivation. Odd.) */ - /// @c c is the underlying container. -#if __cplusplus >= 201103L - _Sequence c{}; -#else + /// @c c is the underlying container. _Sequence c; -#endif public: /** @@ -147,7 +143,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION queue(const _Sequence& __c = _Sequence()) : c(__c) { } #else - queue() = default; + template::value>::type> + queue() + : c() { } explicit queue(const _Sequence& __c) @@ -446,13 +445,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION protected: // See queue::c for notes on these names. -#if __cplusplus >= 201103L - _Sequence c{}; - _Compare comp{}; -#else - _Sequence c; + _Sequence c; _Compare comp; -#endif public: /** @@ -465,17 +459,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION : c(__s), comp(__x) { std::make_heap(c.begin(), c.end(), comp); } #else - priority_queue() = default; + template, + is_default_constructible<_Seq>>::value>::type> + priority_queue() + : c(), comp() { } explicit - priority_queue(const _Compare& __x, - const _Sequence& __s) + priority_queue(const _Compare& __x, const _Sequence& __s) : c(__s), comp(__x) { std::make_heap(c.begin(), c.end(), comp); } explicit - priority_queue(const _Compare& __x, - _Sequence&& __s = _Sequence()) + priority_queue(const _Compare& __x, _Sequence&& __s = _Sequence()) : c(std::move(__s)), comp(__x) { std::make_heap(c.begin(), c.end(), comp); } diff --git a/libstdc++-v3/include/bits/stl_stack.h b/libstdc++-v3/include/bits/stl_stack.h index a0f9ee5..094ce65 100644 --- a/libstdc++-v3/include/bits/stl_stack.h +++ b/libstdc++-v3/include/bits/stl_stack.h @@ -129,11 +129,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION protected: // See queue::c for notes on this name. -#if __cplusplus >= 201103L - _Sequence c{}; -#else _Sequence c; -#endif public: // XXX removed old def ctor, added def arg to this one to match 14882 @@ -145,7 +141,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION stack(const _Sequence& __c = _Sequence()) : c(__c) { } #else - stack() = default; + template::value>::type> + stack() + : c() { } explicit stack(const _Sequence& __c) 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");