[gcc r15-6272] libstdc++: Fix fancy pointer support in linked lists [PR57272]

Jonathan Wakely redi@gcc.gnu.org
Mon Dec 16 14:24:46 GMT 2024


https://gcc.gnu.org/g:2ce99c0088ed97991f61cbdefa83f682c2ef4364

commit r15-6272-g2ce99c0088ed97991f61cbdefa83f682c2ef4364
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Sun Dec 8 14:34:01 2024 +0000

    libstdc++: Fix fancy pointer support in linked lists [PR57272]
    
    The union members I used in the new _Node types for fancy pointers only
    work for value types that are trivially default constructible. This
    change replaces the anonymous union with a named union so it can be
    given a default constructor and destructor, to leave the variant member
    uninitialized.
    
    This also fixes the incorrect macro names in the alloc_ptr_ignored.cc
    tests as pointed out by François, and fixes some std::list pointer
    confusions that the fixed alloc_ptr_ignored.cc test revealed.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/57272
            * include/bits/forward_list.h (__fwd_list::_Node): Add
            user-provided special member functions to union.
            * include/bits/stl_list.h (__list::_Node): Likewise.
            (_Node_base::_M_hook, _Node_base::swap): Use _M_base() instead
            of std::pointer_traits::pointer_to.
            (_Node_base::_M_transfer): Likewise. Add noexcept.
            (_List_base::_M_put_node): Use 'if constexpr' to avoid using
            pointer_traits::pointer_to when not necessary.
            (_List_base::_M_destroy_node): Fix parameter to be the pointer
            type used internally, not the allocator's pointer.
            (list::_M_create_node): Likewise.
            * testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr.cc:
            Check explicit instantiation of non-trivial value type.
            * testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr.cc:
            Likewise.
            * testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr_ignored.cc:
            Fix macro name.
            * testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr_ignored.cc:
            Likewise.

Diff:
---
 libstdc++-v3/include/bits/forward_list.h           | 15 ++--
 libstdc++-v3/include/bits/stl_list.h               | 92 +++++++++++++---------
 .../explicit_instantiation/alloc_ptr.cc            | 11 +++
 .../explicit_instantiation/alloc_ptr_ignored.cc    |  2 +-
 .../explicit_instantiation/alloc_ptr.cc            | 11 +++
 .../explicit_instantiation/alloc_ptr_ignored.cc    |  2 +-
 6 files changed, 88 insertions(+), 45 deletions(-)

diff --git a/libstdc++-v3/include/bits/forward_list.h b/libstdc++-v3/include/bits/forward_list.h
index f4a53bb4d7ef..3070ef6b1a78 100644
--- a/libstdc++-v3/include/bits/forward_list.h
+++ b/libstdc++-v3/include/bits/forward_list.h
@@ -420,25 +420,30 @@ namespace __fwdlist
       using value_type = typename pointer_traits<_ValPtr>::element_type;
       using _Node_ptr = __ptr_rebind<_ValPtr, _Node>;
 
-      _Node() { }
+      _Node() noexcept { }
       ~_Node() { }
       _Node(_Node&&) = delete;
 
-      union {
+      union _Uninit_storage
+      {
+	_Uninit_storage() noexcept { }
+	~_Uninit_storage() { }
+
 #if ! _GLIBCXX_INLINE_VERSION
 	// For ABI compatibility we need to overalign this member.
 	alignas(__alignof__(value_type)) // XXX GLIBCXX_ABI Deprecated
 #endif
 	value_type _M_data;
       };
+      _Uninit_storage _M_u;
 
       value_type*
       _M_valptr() noexcept
-      { return std::__addressof(_M_data); }
+      { return std::__addressof(_M_u._M_data); }
 
       const value_type*
       _M_valptr() const noexcept
-      { return std::__addressof(_M_data); }
+      { return std::__addressof(_M_u._M_data); }
 
       _Node_ptr
       _M_node_ptr()
@@ -486,7 +491,7 @@ namespace __fwdlist
       [[__nodiscard__]]
       constexpr reference
       operator*() const noexcept
-      { return static_cast<_Node&>(*this->_M_node)._M_data; }
+      { return static_cast<_Node&>(*this->_M_node)._M_u._M_data; }
 
       [[__nodiscard__]]
       constexpr pointer
diff --git a/libstdc++-v3/include/bits/stl_list.h b/libstdc++-v3/include/bits/stl_list.h
index 9f7d27242b60..03c4be79416f 100644
--- a/libstdc++-v3/include/bits/stl_list.h
+++ b/libstdc++-v3/include/bits/stl_list.h
@@ -199,12 +199,12 @@ namespace __list
       swap(_Node_base& __x, _Node_base& __y) noexcept;
 
       void
-      _M_transfer(_Base_ptr const __first, _Base_ptr const __last);
+      _M_transfer(_Base_ptr const __first, _Base_ptr const __last) noexcept;
 
       void
       _M_hook(_Base_ptr const __position) noexcept
       {
-	auto __self = pointer_traits<_Base_ptr>::pointer_to(*this);
+	auto __self = this->_M_base();
 	this->_M_next = __position;
 	this->_M_prev = __position->_M_prev;
 	__position->_M_prev->_M_next = __self;
@@ -224,6 +224,8 @@ namespace __list
       // by std::list::empty(), where it doesn't escape, and by
       // std::list::end() const, where the pointer is used to initialize a
       // const_iterator and so constness is restored.
+      // The standard allows pointer_to to be potentially-throwing,
+      // but we have to assume it doesn't throw to implement std::list.
       _Base_ptr
       _M_base() const noexcept
       {
@@ -290,16 +292,24 @@ namespace __list
       using value_type = typename pointer_traits<_ValPtr>::element_type;
       using _Node_ptr = __ptr_rebind<_ValPtr, _Node>;
 
-      union {
+      _Node() noexcept { }
+      ~_Node() { }
+      _Node(_Node&&) = delete;
+
+      union _Uninit_storage
+      {
+	_Uninit_storage() noexcept { }
+	~_Uninit_storage() { }
+
 	value_type _M_data;
       };
+      _Uninit_storage _M_u;
 
-      _Node() { }
-      ~_Node() { }
-      _Node(_Node&&) = delete;
+      value_type*
+      _M_valptr() noexcept       { return std::__addressof(_M_u._M_data); }
 
-      value_type*       _M_valptr()       { return std::__addressof(_M_data); }
-      value_type const* _M_valptr() const { return std::__addressof(_M_data); }
+      value_type const*
+      _M_valptr() const noexcept { return std::__addressof(_M_u._M_data); }
 
       _Node_ptr
       _M_node_ptr()
@@ -349,7 +359,7 @@ namespace __list
       [[__nodiscard__]]
       constexpr reference
       operator*() const noexcept
-      { return static_cast<_Node&>(*_M_node)._M_data; }
+      { return static_cast<_Node&>(*_M_node)._M_u._M_data; }
 
       [[__nodiscard__]]
       constexpr pointer
@@ -748,6 +758,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 	rebind<typename _Node_traits::_Node>::other _Node_alloc_type;
       typedef __gnu_cxx::__alloc_traits<_Node_alloc_type> _Node_alloc_traits;
 
+#if __cplusplus < 201103L || ! _GLIBCXX_USE_ALLOC_PTR_FOR_LIST
+      typedef _List_node<_Tp>* _Node_ptr;
+#else
+      using _Node_ptr = typename _Node_alloc_traits::pointer;
+#endif
+
       struct _List_impl
       : public _Node_alloc_type
       {
@@ -797,42 +813,47 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       _M_get_node()
       { return _Node_alloc_traits::allocate(_M_impl, 1); }
 
-#if __cplusplus < 201103L || _GLIBCXX_USE_ALLOC_PTR_FOR_LIST
-      void
-      _M_put_node(typename _Node_alloc_traits::pointer __p) _GLIBCXX_NOEXCEPT
-      { _Node_alloc_traits::deallocate(_M_impl, __p, 1); }
-#else
       void
-      _M_put_node(_List_node<_Tp>* __p)
+      _M_put_node(_Node_ptr __p) _GLIBCXX_NOEXCEPT
       {
-	// When not using the allocator's pointer type internally we must
-	// convert between _Node_ptr (i.e. _List_node*) and the allocator's
-	// pointer type.
+#if __cplusplus < 201103L || _GLIBCXX_USE_ALLOC_PTR_FOR_LIST
+	_Node_alloc_traits::deallocate(_M_impl, __p, 1);
+#else
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr
 	using __alloc_pointer = typename _Node_alloc_traits::pointer;
-	auto __ap = pointer_traits<__alloc_pointer>::pointer_to(*__p);
-	_Node_alloc_traits::deallocate(_M_impl, __ap, 1);
-      }
+	if constexpr (is_same<_Node_ptr, __alloc_pointer>::value)
+	  _Node_alloc_traits::deallocate(_M_impl, __p, 1);
+	else
+	  {
+	    // When not using the allocator's pointer type internally we must
+	    // convert __p to __alloc_pointer so it can be deallocated.
+	    auto __ap = pointer_traits<__alloc_pointer>::pointer_to(*__p);
+	    _Node_alloc_traits::deallocate(_M_impl, __ap, 1);
+	  }
+#pragma GCC diagnostic pop
 #endif
+      }
 
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr
       void
-      _M_destroy_node(typename _Node_alloc_traits::pointer __p)
+      _M_destroy_node(_Node_ptr __p)
       {
-#if __cplusplus >= 201103L
 	// Destroy the element
+#if __cplusplus < 201103L
+	_Tp_alloc_type(_M_impl).destroy(__p->_M_valptr());
+#else
 	_Node_alloc_traits::destroy(_M_impl, __p->_M_valptr());
 	// Only destroy the node if the pointers require it.
 	using _Node = typename _Node_traits::_Node;
 	using _Base_ptr = typename _Node_traits::_Node_base::_Base_ptr;
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr
 	if constexpr (!is_trivially_destructible<_Base_ptr>::value)
 	  __p->~_Node();
-#else
-	_Tp_alloc_type(_M_impl).destroy(__p->_M_valptr());
+#pragma GCC diagnostic pop
 #endif
 	this->_M_put_node(__p);
       }
-#pragma GCC diagnostic pop
 
   public:
       typedef _Alloc allocator_type;
@@ -1084,12 +1105,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 	  auto __guard = std::__allocate_guarded_obj(__alloc);
 	  _Node_alloc_traits::construct(__alloc, __guard->_M_valptr(),
 					std::forward<_Args>(__args)...);
-	  auto __p = __guard.release();
-#if _GLIBCXX_USE_ALLOC_PTR_FOR_LIST
-	  return __p;
-#else
-	  return std::__to_address(__p);
-#endif
+	  return __guard.release();
 	}
 #endif
 
@@ -2756,8 +2772,8 @@ namespace __list
     void
     _Node_base<_VoidPtr>::swap(_Node_base& __x, _Node_base& __y) noexcept
     {
-      auto __px = pointer_traits<_Base_ptr>::pointer_to(__x);
-      auto __py = pointer_traits<_Base_ptr>::pointer_to(__y);
+      auto __px = __x._M_base();
+      auto __py = __x._M_base();
 
       if (__x._M_next != __px)
 	{
@@ -2792,11 +2808,11 @@ namespace __list
   template<typename _VoidPtr>
     void
     _Node_base<_VoidPtr>::_M_transfer(_Base_ptr const __first,
-				      _Base_ptr const __last)
+				      _Base_ptr const __last) noexcept
     {
       __glibcxx_assert(__first != __last);
 
-      auto __self = pointer_traits<_Base_ptr>::pointer_to(*this);
+      auto __self = _M_base();
       if (__self != __last)
 	{
 	  // Remove [first, last) from its old position.
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr.cc b/libstdc++-v3/testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr.cc
index 621ff4c652cb..e3bf8a13095a 100644
--- a/libstdc++-v3/testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr.cc
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr.cc
@@ -59,6 +59,17 @@ struct Allocator
 
 template class std::forward_list<int, Allocator<int>>;
 
+struct NonTrivial
+{
+  NonTrivial() { }
+  NonTrivial(const NonTrivial&) { }
+  ~NonTrivial() { }
+  bool operator==(const NonTrivial&) const { return true; } // for remove(T)
+  bool operator<(const NonTrivial&) const { return false; } // for sort()
+};
+
+template class std::forward_list<NonTrivial, Allocator<NonTrivial>>;
+
 #include <testsuite_iterators.h>
 
 void
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr_ignored.cc b/libstdc++-v3/testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr_ignored.cc
index 6205a2ff3bf2..19df931a6a65 100644
--- a/libstdc++-v3/testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr_ignored.cc
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/requirements/explicit_instantiation/alloc_ptr_ignored.cc
@@ -1,4 +1,4 @@
-// { dg-options "-D_GLIBCXX_FWDLIST_USE_ALLOC_PTR=0" }
+// { dg-options "-D_GLIBCXX_USE_ALLOC_PTR_FOR_FWD_LIST=0" }
 // { dg-do compile { target c++11 } }
 
 #include "alloc_ptr.cc"
diff --git a/libstdc++-v3/testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr.cc b/libstdc++-v3/testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr.cc
index d3b2cfe6d923..c52591b3cf8b 100644
--- a/libstdc++-v3/testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr.cc
+++ b/libstdc++-v3/testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr.cc
@@ -57,6 +57,17 @@ struct Allocator
 
 template class std::list<int, Allocator<int>>;
 
+struct NonTrivial
+{
+  NonTrivial() { }
+  NonTrivial(const NonTrivial&) { }
+  ~NonTrivial() { }
+  bool operator==(const NonTrivial&) const { return true; } // for remove(T)
+  bool operator<(const NonTrivial&) const { return false; } // for sort()
+};
+
+template class std::list<NonTrivial, Allocator<NonTrivial>>;
+
 #include <testsuite_iterators.h>
 
 void
diff --git a/libstdc++-v3/testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr_ignored.cc b/libstdc++-v3/testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr_ignored.cc
index e6a499d27160..f5bbf2cb944e 100644
--- a/libstdc++-v3/testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr_ignored.cc
+++ b/libstdc++-v3/testsuite/23_containers/list/requirements/explicit_instantiation/alloc_ptr_ignored.cc
@@ -1,4 +1,4 @@
-// { dg-options "-D_GLIBCXX_LIST_USE_ALLOC_PTR=0" }
+// { dg-options "-D_GLIBCXX_USE_ALLOC_PTR_FOR_LIST=0" }
 // { dg-do compile { target c++11 } }
 
 #include "alloc_ptr.cc"


More information about the Libstdc++-cvs mailing list