This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
Re: [PATCH] PR77528 add default constructors for container adaptors
On 10/01/17 13:15 -0500, Tim Song wrote:
On Tue, Jan 10, 2017 at 12:33 PM, Jonathan Wakely <jwakely@redhat.com> 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<queue<T, NonDefaultConstructible>>
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 <jwakely@redhat.com>
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 <debug/debug.h>
#if __cplusplus >= 201103L
# include <bits/uses_allocator.h>
+# include <bits/enable_special_members.h>
#endif
namespace std _GLIBCXX_VISIBILITY(default)
@@ -94,6 +95,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*/
template<typename _Tp, typename _Sequence = deque<_Tp> >
class queue
+#if __cplusplus >= 201103L
+ : _Enable_default_constructor<is_default_constructible<_Sequence>::value,
+ queue<_Tp, _Sequence>>
+#endif
{
// concept requirements
typedef typename _Sequence::value_type _Sequence_value_type;
@@ -114,6 +119,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<typename _Alloc>
using _Uses = typename
enable_if<uses_allocator<_Sequence, _Alloc>::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<typename _Alloc, typename _Requires = _Uses<_Alloc>>
explicit
queue(const _Alloc& __a)
- : c(__a) { }
+ : _Base(_Tag()), c(__a) { }
template<typename _Alloc, typename _Requires = _Uses<_Alloc>>
queue(const _Sequence& __c, const _Alloc& __a)
- : c(__c, __a) { }
+ : _Base(_Tag()), c(__c, __a) { }
template<typename _Alloc, typename _Requires = _Uses<_Alloc>>
queue(_Sequence&& __c, const _Alloc& __a)
- : c(std::move(__c), __a) { }
+ : _Base(_Tag()), c(std::move(__c), __a) { }
template<typename _Alloc, typename _Requires = _Uses<_Alloc>>
queue(const queue& __q, const _Alloc& __a)
- : c(__q.c, __a) { }
+ : _Base(_Tag()), c(__q.c, __a) { }
template<typename _Alloc, typename _Requires = _Uses<_Alloc>>
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 _Tp, typename _Sequence = vector<_Tp>,
typename _Compare = less<typename _Sequence::value_type> >
class priority_queue
+#if __cplusplus >= 201103L
+ : _Enable_default_constructor<__and_<is_default_constructible<_Sequence>,
+ 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<typename _Alloc>
using _Uses = typename
enable_if<uses_allocator<_Sequence, _Alloc>::value>::type;
+
+ using _Tag = _Enable_default_constructor_tag;
+ using _Base = _Enable_default_constructor<
+ __and_<is_default_constructible<_Sequence>,
+ 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<typename _Alloc, typename _Requires = _Uses<_Alloc>>
explicit
priority_queue(const _Alloc& __a)
- : c(__a), comp() { }
+ : _Base(_Tag()), c(__a), comp() { }
template<typename _Alloc, typename _Requires = _Uses<_Alloc>>
priority_queue(const _Compare& __x, const _Alloc& __a)
- : c(__a), comp(__x) { }
+ : _Base(_Tag()), c(__a), comp(__x) { }
template<typename _Alloc, typename _Requires = _Uses<_Alloc>>
priority_queue(const _Compare& __x, const _Sequence& __c,
const _Alloc& __a)
- : c(__c, __a), comp(__x) { }
+ : _Base(_Tag()), c(__c, __a), comp(__x) { }
template<typename _Alloc, typename _Requires = _Uses<_Alloc>>
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<typename _Alloc, typename _Requires = _Uses<_Alloc>>
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<typename _Alloc, typename _Requires = _Uses<_Alloc>>
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 <debug/debug.h>
#if __cplusplus >= 201103L
# include <bits/uses_allocator.h>
+# include <bits/enable_special_members.h>
#endif
namespace std _GLIBCXX_VISIBILITY(default)
@@ -97,6 +98,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*/
template<typename _Tp, typename _Sequence = deque<_Tp> >
class stack
+#if __cplusplus >= 201103L
+ : _Enable_default_constructor<is_default_constructible<_Sequence>::value,
+ stack<_Tp, _Sequence>>
+#endif
{
// concept requirements
typedef typename _Sequence::value_type _Sequence_value_type;
@@ -118,6 +123,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<typename _Alloc>
using _Uses = typename
enable_if<uses_allocator<_Sequence, _Alloc>::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<typename _Alloc, typename _Requires = _Uses<_Alloc>>
explicit
stack(const _Alloc& __a)
- : c(__a) { }
+ : _Base(_Tag()), c(__a) { }
template<typename _Alloc, typename _Requires = _Uses<_Alloc>>
stack(const _Sequence& __c, const _Alloc& __a)
- : c(__c, __a) { }
+ : _Base(_Tag()), c(__c, __a) { }
template<typename _Alloc, typename _Requires = _Uses<_Alloc>>
stack(_Sequence&& __c, const _Alloc& __a)
- : c(std::move(__c), __a) { }
+ : _Base(_Tag()), c(std::move(__c), __a) { }
template<typename _Alloc, typename _Requires = _Uses<_Alloc>>
stack(const stack& __q, const _Alloc& __a)
- : c(__q.c, __a) { }
+ : _Base(_Tag()), c(__q.c, __a) { }
template<typename _Alloc, typename _Requires = _Uses<_Alloc>>
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
+// <http://www.gnu.org/licenses/>.
+
+
+// This file tests explicit instantiation of library containers.
+
+#include <queue>
+
+using std::priority_queue;
+using std::vector;
+
+template<typename A>
+constexpr bool default_constructible()
+{ return std::is_default_constructible<A>::value; }
+
+static_assert(default_constructible<priority_queue<int>>(),
+ "priority_queue<int>");
+
+struct NonDefaultConstructible : vector<int> {
+ NonDefaultConstructible(int) { }
+};
+struct Cmp : std::less<int> {
+ Cmp(int) { }
+};
+static_assert(
+ !default_constructible<priority_queue<int, NonDefaultConstructible>>(),
+ "priority_queue<int, NonDefaultConstructible>");
+static_assert(
+ !default_constructible<priority_queue<int, NonDefaultConstructible, Cmp>>(),
+ "priority_queue<int, NonDefaultConstructible, Cmp>");
+static_assert(
+ !default_constructible<priority_queue<int, vector<int>, Cmp>>(),
+ "priority_queue<int, vector<int>, 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<int> {
Cmp(int) { }
};
template class std::priority_queue<int, NonDefaultConstructible>;
+template class std::priority_queue<int, NonDefaultConstructible, Cmp>;
+template class std::priority_queue<int, std::vector<int>, 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<int> {
struct Cmp : std::less<int> {
Cmp(int) { }
};
+template class std::priority_queue<int, NonDefaultConstructible>;
template class std::priority_queue<int, NonDefaultConstructible, Cmp>;
+template class std::priority_queue<int, std::vector<int>, 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
+// <http://www.gnu.org/licenses/>.
+
+
+// This file tests explicit instantiation of library containers.
+
+#include <queue>
+
+using std::queue;
+
+template<typename A>
+constexpr bool default_constructible()
+{ return std::is_default_constructible<A>::value; }
+
+static_assert(default_constructible<queue<int>>(), "queue<int>");
+
+struct NonDefaultConstructible : std::deque<int> {
+ NonDefaultConstructible(int) { }
+};
+static_assert(!default_constructible<queue<int, NonDefaultConstructible>>(),
+ "queue<int, NonDefaultConstructible>");
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
+// <http://www.gnu.org/licenses/>.
+
+
+// This file tests explicit instantiation of library containers.
+
+#include <stack>
+
+using std::stack;
+
+template<typename A>
+constexpr bool default_constructible()
+{ return std::is_default_constructible<A>::value; }
+
+static_assert(default_constructible<stack<int>>(), "stack<int>");
+
+struct NonDefaultConstructible : std::deque<int> {
+ NonDefaultConstructible(int) { }
+};
+static_assert(!default_constructible<stack<int, NonDefaultConstructible>>(),
+ "stack<int, NonDefaultConstructible>");