Patchwork [v3] Fix allocator-aware container requirements for forward_list

login
register
mail settings
Submitter Jonathan Wakely
Date Nov. 2, 2012, 1:34 a.m.
Message ID <CAH6eHdQife9PeHS1N9anpW1TiJJGeDOTFL1QvQX+b1-1S=epOw@mail.gmail.com>
Download mbox | patch
Permalink /patch/196456/
State New
Headers show

Comments

Jonathan Wakely - Nov. 2, 2012, 1:34 a.m.
This fixes the new LWG issue 2210 and only uses allocator::construct()
for the elements not the nodes.

2012-11-02  Jonathan Wakely  <jwakely.gcc@gmail.com>

        * include/bits/forward_list.h (forward_list(size_type)): Add missing
        allocator parameter.
        (_Fwd_list_node_base): Use NSDMI and define constructor as defaulted.
        (_Fwd_list_node::_M_value): Replace with uninitialized storage.
        (_Fwd_list_node::_M_valptr()): Define functions to access storage.
        (_Fwd_list_iterator, _Fwd_list_const_iterator): Use _M_valptr.
        (_Fwd_list_base::_M_create_node): Only use allocator to construct the
        element not the node.
        * include/bits/forward_list.tcc (_Fwd_list_base::_M_erase_after): Only
        use allocator to destroy the element not the node.
        * testsuite/23_containers/forward_list/cons/11.cc: Remove unused
        headers.
        * testsuite/23_containers/forward_list/cons/12.cc: Likewise.
        * testsuite/23_containers/forward_list/cons/13.cc: New.
        * testsuite/23_containers/forward_list/cons/14.cc: New.

Tested x86_64-linux, committed to trunk.

The pretty printers need an update, which I'll do asap.
commit 751520cfc531da12190b93c12a208f83fec81e4e
Author: Jonathan Wakely <jwakely.gcc@gmail.com>
Date:   Fri Nov 2 00:15:27 2012 +0000

    	* include/bits/forward_list.h (forward_list(size_type)): Add missing
    	allocator parameter.
    	(_Fwd_list_node_base): Use NSDMI and define constructor as defaulted.
    	(_Fwd_list_node::_M_value): Replace with uninitialized storage.
    	(_Fwd_list_node::_M_valptr()): Define functions to access storage.
    	(_Fwd_list_iterator, _Fwd_list_const_iterator): Use _M_valptr.
    	(_Fwd_list_base::_M_create_node): Only use allocator to construct the
    	element not the node.
    	* include/bits/forward_list.tcc (_Fwd_list_base::_M_erase_after): Only
    	use allocator to destroy the element not the node.
    	* testsuite/23_containers/forward_list/cons/11.cc: Remove unused
    	headers.
    	* testsuite/23_containers/forward_list/cons/12.cc: Likewise.
    	* testsuite/23_containers/forward_list/cons/13.cc: New.
    	* testsuite/23_containers/forward_list/cons/14.cc: New.

Patch

diff --git a/libstdc++-v3/include/bits/forward_list.h b/libstdc++-v3/include/bits/forward_list.h
index b40fe9b..9efabcf 100644
--- a/libstdc++-v3/include/bits/forward_list.h
+++ b/libstdc++-v3/include/bits/forward_list.h
@@ -48,9 +48,9 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
    */
   struct _Fwd_list_node_base
   {
-    _Fwd_list_node_base() : _M_next(0) { }
+    _Fwd_list_node_base() = default;
 
-    _Fwd_list_node_base* _M_next;
+    _Fwd_list_node_base* _M_next = nullptr;
 
     _Fwd_list_node_base*
     _M_transfer_after(_Fwd_list_node_base* __begin,
@@ -86,19 +86,30 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
   /**
    *  @brief  A helper node class for %forward_list.
-   *          This is just a linked list with a data value in each node.
+   *          This is just a linked list with uninitialized storage for a
+   *          data value in each node.
    *          There is a sorting utility method.
    */
   template<typename _Tp>
     struct _Fwd_list_node
     : public _Fwd_list_node_base
     {
-      template<typename... _Args>
-        _Fwd_list_node(_Args&&... __args)
-        : _Fwd_list_node_base(), 
-          _M_value(std::forward<_Args>(__args)...) { }
+      _Fwd_list_node() = default;
+
+      typename aligned_storage<sizeof(_Tp), alignment_of<_Tp>::value>::type
+	_M_storage;
+
+      _Tp*
+      _M_valptr() noexcept
+      {
+	return static_cast<_Tp*>(static_cast<void*>(&_M_storage));
+      }
 
-      _Tp _M_value;
+      const _Tp*
+      _M_valptr() const noexcept
+      {
+	return static_cast<const _Tp*>(static_cast<const void*>(&_M_storage));
+      }
     };
 
   /**
@@ -127,12 +138,11 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
       reference
       operator*() const
-      { return static_cast<_Node*>(this->_M_node)->_M_value; }
+      { return *static_cast<_Node*>(this->_M_node)->_M_valptr(); }
 
       pointer
       operator->() const
-      { return std::__addressof(static_cast<_Node*>
-				(this->_M_node)->_M_value); }
+      { return static_cast<_Node*>(this->_M_node)->_M_valptr(); }
 
       _Self&
       operator++()
@@ -199,12 +209,11 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
       reference
       operator*() const
-      { return static_cast<_Node*>(this->_M_node)->_M_value; }
+      { return *static_cast<_Node*>(this->_M_node)->_M_valptr(); }
 
       pointer
       operator->() const
-      { return std::__addressof(static_cast<_Node*>
-				(this->_M_node)->_M_value); }
+      { return static_cast<_Node*>(this->_M_node)->_M_valptr(); }
 
       _Self&
       operator++()
@@ -339,9 +348,11 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
           _Node* __node = this->_M_get_node();
           __try
             {
-              _Node_alloc_traits::construct(_M_get_Node_allocator(), __node,
-                                            std::forward<_Args>(__args)...);
-              __node->_M_next = 0;
+	      _Tp_alloc_type __a(_M_get_Node_allocator());
+	      typedef allocator_traits<_Tp_alloc_type> _Alloc_traits;
+	      ::new ((void*)__node) _Node();
+	      _Alloc_traits::construct(__a, __node->_M_valptr(),
+				       std::forward<_Args>(__args)...);
             }
           __catch(...)
             {
@@ -457,8 +468,8 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
        *  constructed elements.
        */
       explicit
-      forward_list(size_type __n)
-      : _Base()
+      forward_list(size_type __n, const _Alloc& __al = _Alloc())
+      : _Base(_Node_alloc_type(__al))
       { _M_default_initialize(__n); }
 
       /**
@@ -738,7 +749,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       front()
       {
         _Node* __front = static_cast<_Node*>(this->_M_impl._M_head._M_next);
-        return __front->_M_value;
+        return *__front->_M_valptr();
       }
 
       /**
@@ -749,7 +760,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       front() const
       {
         _Node* __front = static_cast<_Node*>(this->_M_impl._M_head._M_next);
-        return __front->_M_value;
+        return *__front->_M_valptr();
       }
 
       // 23.3.4.5 modiļ¬ers:
diff --git a/libstdc++-v3/include/bits/forward_list.tcc b/libstdc++-v3/include/bits/forward_list.tcc
index 7395b20..757f319 100644
--- a/libstdc++-v3/include/bits/forward_list.tcc
+++ b/libstdc++-v3/include/bits/forward_list.tcc
@@ -53,7 +53,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
           while (__curr)
             {
               __to->_M_next =
-                _M_create_node(std::move_if_noexcept(__curr->_M_value));
+                _M_create_node(std::move_if_noexcept(*__curr->_M_valptr()));
               __to = __to->_M_next;
               __curr = static_cast<_Node*>(__curr->_M_next);
             }
@@ -81,7 +81,9 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
     {
       _Node* __curr = static_cast<_Node*>(__pos->_M_next);
       __pos->_M_next = __curr->_M_next;
-      _Node_alloc_traits::destroy(_M_get_Node_allocator(), __curr);
+      _Tp_alloc_type __a(_M_get_Node_allocator());
+      allocator_traits<_Tp_alloc_type>::destroy(__a, __curr->_M_valptr());
+      __curr->~_Node();
       _M_put_node(__curr);
       return __pos->_M_next;
     }
@@ -97,7 +99,9 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
         {
           _Node* __temp = __curr;
           __curr = static_cast<_Node*>(__curr->_M_next);
-          _Node_alloc_traits::destroy(_M_get_Node_allocator(), __temp);
+	  _Tp_alloc_type __a(_M_get_Node_allocator());
+	  allocator_traits<_Tp_alloc_type>::destroy(__a, __temp->_M_valptr());
+	  __temp->~_Node();
           _M_put_node(__temp);
         }
       __pos->_M_next = __last;
@@ -300,10 +304,9 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
       while (_Node* __tmp = static_cast<_Node*>(__curr->_M_next))
         {
-          if (__tmp->_M_value == __val)
+          if (*__tmp->_M_valptr() == __val)
 	    {
-	      if (std::__addressof(__tmp->_M_value)
-		  != std::__addressof(__val))
+	      if (__tmp->_M_valptr() != std::__addressof(__val))
 		{
 		  this->_M_erase_after(__curr);
 		  continue;
@@ -327,7 +330,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	_Node* __curr = static_cast<_Node*>(&this->_M_impl._M_head);
         while (_Node* __tmp = static_cast<_Node*>(__curr->_M_next))
           {
-            if (__pred(__tmp->_M_value))
+            if (__pred(*__tmp->_M_valptr()))
               this->_M_erase_after(__curr);
             else
               __curr = static_cast<_Node*>(__curr->_M_next);
@@ -364,10 +367,10 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
         _Node_base* __node = &this->_M_impl._M_head;
         while (__node->_M_next && __list._M_impl._M_head._M_next)
           {
-            if (__comp(static_cast<_Node*>
-                       (__list._M_impl._M_head._M_next)->_M_value,
-                       static_cast<_Node*>
-                       (__node->_M_next)->_M_value))
+            if (__comp(*static_cast<_Node*>
+                       (__list._M_impl._M_head._M_next)->_M_valptr(),
+                       *static_cast<_Node*>
+                       (__node->_M_next)->_M_valptr()))
               __node->_M_transfer_after(&__list._M_impl._M_head,
                                         __list._M_impl._M_head._M_next);
             __node = __node->_M_next;
@@ -460,7 +463,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
                         __p = static_cast<_Node*>(__p->_M_next);
                         --__psize;
                       }
-                    else if (__comp(__p->_M_value, __q->_M_value))
+                    else if (__comp(*__p->_M_valptr(), *__q->_M_valptr()))
                       {
                         // First node of p is lower; e must come from p.
                         __e = __p;
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/cons/11.cc b/libstdc++-v3/testsuite/23_containers/forward_list/cons/11.cc
index 30ae3fc..fa8c8db 100644
--- a/libstdc++-v3/testsuite/23_containers/forward_list/cons/11.cc
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/cons/11.cc
@@ -20,8 +20,6 @@ 
 // 23.3.4.2 forward_list construction [forwardlist.cons]
 
 #include <forward_list>
-#include <testsuite_hooks.h>
-#include <testsuite_allocator.h>
 
 bool fail = false;
 
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/cons/12.cc b/libstdc++-v3/testsuite/23_containers/forward_list/cons/12.cc
index 42889db..3e28735 100644
--- a/libstdc++-v3/testsuite/23_containers/forward_list/cons/12.cc
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/cons/12.cc
@@ -21,8 +21,6 @@ 
 // 23.3.4.2 forward_list construction [forwardlist.cons]
 
 #include <forward_list>
-#include <testsuite_hooks.h>
-#include <testsuite_allocator.h>
 
 bool fail = false;
 
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/cons/13.cc b/libstdc++-v3/testsuite/23_containers/forward_list/cons/13.cc
new file mode 100644
index 0000000..e21e02e
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/cons/13.cc
@@ -0,0 +1,55 @@ 
+// { dg-options "-std=gnu++11" }
+
+// Copyright (C) 2012 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/>.
+
+// 23.3.4.2 forward_list construction [forwardlist.cons]
+
+#include <forward_list>
+#include <memory>
+#include <scoped_allocator>
+#include <testsuite_hooks.h>
+
+struct A
+{
+  typedef std::allocator<A> allocator_type;
+
+  A() : ok(false) { }
+  A(const A&) : ok(false) { }
+  A(const allocator_type&) : ok(true) { }
+  A(const A&, const allocator_type&) : ok(true) { }
+
+  bool ok;
+};
+
+void test01()
+{
+  typedef std::scoped_allocator_adaptor<A::allocator_type> alloc_type;
+  typedef std::forward_list<A, alloc_type> list;
+
+  list l1(1);
+  VERIFY( l1.begin()->ok );
+
+  A a;
+  list l2(1, a);
+  VERIFY( l2.begin()->ok );
+}
+
+int main()
+{
+  test01();
+}
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/cons/14.cc b/libstdc++-v3/testsuite/23_containers/forward_list/cons/14.cc
new file mode 100644
index 0000000..8bb17ee
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/cons/14.cc
@@ -0,0 +1,35 @@ 
+// { dg-do compile }
+// { dg-options "-std=gnu++11" }
+
+// Copyright (C) 2012 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/>.
+
+// 23.3.4.2 forward_list construction [forwardlist.cons]
+
+#include <forward_list>
+#include <scoped_allocator>
+
+void test01()
+{
+  using namespace std;
+  using list = forward_list<int>;
+  forward_list<list, scoped_allocator_adaptor<list::allocator_type>> l;
+
+  // Check for forward_list(size_type, const allocator_type&)
+  l.emplace_front(1u);
+}
+