diff mbox

Use abi_tag attribute on std::list

Message ID 20141010154035.GE4197@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely Oct. 10, 2014, 3:40 p.m. UTC
On 03/10/14 15:49 +0100, Jonathan Wakely wrote:
>On 03/10/14 16:25 +0200, Marc Glisse wrote:
>>On Fri, 3 Oct 2014, Jonathan Wakely wrote:
>>
>>>This is the patch I intend to commit to make std::list::size() O(1) as
>>>required by C++11.
>>>
>>>This is an ABI change, so std::list will get tagged with
>>>abi_tag("cxx11") so that it mangles differently.
>>
>>Assuming a future where we have both _GLIBCXX_ABI_TAG_CXX11 and 
>>_GLIBCXX_ABI_TAG_CXX17, I don't really see how 
>>_GLIBCXX_DEFAULT_ABI_TAG
>>is supposed to work. We don't want to define 
>>_GLIBCXX_DEFAULT_ABI_TAG to _GLIBCXX_ABI_TAG_CXX17 and suddenly have 
>>std::list change mangling.
>
>True.
>
>>Should it be called _GLIBCXX_DEFAULT_ABI_TAG_CXX11, meaning 
>>_GLIBCXX_ABI_TAG_CXX11_IF_ENABLED_AND_NOTHING_OTHERWISE?
>
>I suppose so ... or _GLIBCXX_MAYBE_ABI_TAG_CXX11 ?
>
>>Defining a dummy _M_distance in the old abi case is a bit strange 
>>(we could protect the single use with #if _GLIBCXX_USE_CXX11_ABI), 
>>but why not...
>
>Yeah, I was trying to minimise the preprocessor conditionals, but
>maybe that's one step too far.
>
>>Do you mind if I move (in a future patch once yours is committed) 
>>_M_size into _M_impl::_M_node as suggested in PR 61347?
>
>Gah, that's where I had it until earlier this week, and I looked at it
>and wondered why it was in the _List_impl class (because you only need
>one member in there to benefit from the empty base-class
>optimisation).
>
>I will move it back there, since I already have that code on another
>branch, so there's no point making you change the code to match
>something I've already got!
>
>Thanks for your useful comments (as always).

I've committed the attached patch, which I think is the same as the
one I posted a week ago.

Marc, I agree with your comments, but chose to ignore them ;-) For
now, anyway. I want to get something committed, we can improve it in
stage 3, or when we need to worry about a C++17 ABI change.

Tested x86_64-linux, committed to trunk.

Documentation for the abi_tag("cxx11") changes will follow ASAP, but
probably during stage 3, once all the changes are committed.
diff mbox

Patch

commit 3ac61312eb285d125da8af4c6cdaad1b1ce7d235
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Aug 7 01:13:17 2014 +0100

    	PR libstdc++/49561
    	* acinclude.m4 (GLIBCXX_ENABLE_LIBSTDCXX_CXX11_ABI): Define.
    	* configure.ac: Use GLIBCXX_ENABLE_LIBSTDCXX_CXX11_ABI.
    	* configure: Regenerate.
    	* include/Makefile.am (stamp-cxx11-abi): New target.
    	(c++config.h): Set _GLIBCXX_USE_CXX11_ABI macro.
    	* include/Makefile.in: Regenerate.
    	* include/bits/c++config: Add _GLIBCXX_USE_CXX11_ABI placeholder and
    	define _GLIBCXX_DEFAULT_ABI_TAG.
    	* include/bits/list.tcc (list::emplace(const_iterator, _Args&...)):
    	Increment size.
    	(list::emplace(const_iterator, const value_type&)): Likewise.
    	(list::merge(list&), list::merge(list&, _StrictWeakOrdering)): Adjust
    	list sizes.
    	* include/bits/stl_list.h (_List_base, list): Add ABI tag macro.
    	(_List_base::_M_size): New data member in cxx11 ABI mode.
    	(_List_base::_S_distance(_List_node_base*, _List_node_base*)): New
    	function.
    	(_List_base::_M_get_size(), _List_base::_M_set_size(size_t),
    	_List_base::_M_inc_size(size_t), _List_base::_M_dec_size(size_t),
    	_List_base::_M_distance, _List_base::_M_node_count): New functions for
    	accessing list size correctly for the ABI mode.
    	(_List_base::_List_base(_List_base&&)): Copy size and reset source.
    	(_List_base::_M_init()): Initialize size member.
    	(list::size()): Use _List_base::_M_node_count.
    	(list::swap(list&)): Swap sizes.
    	(list::splice(iterator, list&)): Update sizes.
    	(list::splice(iterator, list&, iterator)): Likewise.
    	(list::insert(iterator, const value_type&)): Update size.
    	(list::insert(iterator, _Args&&...)): Likewise.
    	(list::_M_erase(iterator)): Likewise.
    	* testsuite/23_containers/list/requirements/dr438/assign_neg.cc:
    	Adjust.
    	* testsuite/23_containers/list/requirements/dr438/constructor_1_neg.cc:
    	Adjust.
    	* testsuite/23_containers/list/requirements/dr438/constructor_2_neg.cc:
    	Adjust.
    	* testsuite/23_containers/list/requirements/dr438/insert_neg.cc:
    	Adjust.
    	* testsuite/ext/profile/mutex_extensions_neg.cc: Adjust.

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 2650a5a..0229609 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -3831,6 +3831,25 @@  AC_DEFUN([GLIBCXX_CHECK_SDT_H], [
   AC_MSG_RESULT($glibcxx_cv_sys_sdt_h)
 ])
 
+dnl
+dnl Check if the user wants the new C++11-conforming ABI.
+dnl
+dnl --disable-libstdcxx-cxx11-abi will use old ABI for all types.
+dnl
+dnl Defines:
+dnl  _GLIBCXX_USE_ABI_TAG (always defined, either to 1 or 0)
+dnl
+AC_DEFUN([GLIBCXX_ENABLE_LIBSTDCXX_CXX11_ABI], [
+  AC_ARG_ENABLE([libstdcxx-cxx11-abi],
+    AC_HELP_STRING([--disable-libstdcxx-cxx11-abi],
+		   [disable the C++11-conforming ABI]),,
+		   [enable_libstdcxx_cxx11_abi=yes])
+  if test x"$enable_libstdcxx_cxx11_abi" != xyes; then
+    AC_MSG_NOTICE([C++11-conforming ABI is disabled])
+  fi
+  GLIBCXX_CONDITIONAL(ENABLE_CXX11_ABI, test $enable_libstdcxx_cxx11_abi = yes)
+])
+
 # Macros from the top-level gcc directory.
 m4_include([../config/gc++filt.m4])
 m4_include([../config/tls.m4])
diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index 1de0f6c..eb826e4 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -365,6 +365,8 @@  AC_SUBST(libtool_VERSION)
 
 GLIBCXX_ENABLE_LIBSTDCXX_VISIBILITY([yes])
 
+GLIBCXX_ENABLE_LIBSTDCXX_CXX11_ABI([yes])
+
 ac_ldbl_compat=no
 case "$target" in
   powerpc*-*-linux* | \
diff --git a/libstdc++-v3/include/Makefile.am b/libstdc++-v3/include/Makefile.am
index dee4a3f..e3aed84 100644
--- a/libstdc++-v3/include/Makefile.am
+++ b/libstdc++-v3/include/Makefile.am
@@ -1122,6 +1122,14 @@  stamp-visibility:
 	echo 0 > stamp-visibility
 endif
 
+if ENABLE_CXX11_ABI
+stamp-cxx11-abi:
+	echo 1 > stamp-cxx11-abi
+else
+stamp-cxx11-abi:
+	echo 0 > stamp-cxx11-abi
+endif
+
 # NB: The non-empty default ldbl_compat works around an AIX sed
 # oddity, see libstdc++/31957 for details.
 ${host_builddir}/c++config.h: ${CONFIG_HEADER} \
@@ -1130,11 +1138,13 @@  ${host_builddir}/c++config.h: ${CONFIG_HEADER} \
 			      ${toplevel_srcdir}/gcc/DATESTAMP \
 			      stamp-namespace-version \
 			      stamp-visibility \
-			      stamp-extern-template
+			      stamp-extern-template \
+			      stamp-cxx11-abi
 	@date=`cat ${toplevel_srcdir}/gcc/DATESTAMP` ;\
 	ns_version=`cat stamp-namespace-version` ;\
 	visibility=`cat stamp-visibility` ;\
 	externtemplate=`cat stamp-extern-template` ;\
+	cxx11abi=`cat stamp-cxx11-abi` ;\
 	ldbl_compat='s,g,g,' ;\
 	grep "^[	 ]*#[	 ]*define[	 ][	 ]*_GLIBCXX_LONG_DOUBLE_COMPAT[	 ][	 ]*1[	 ]*$$" \
 	${CONFIG_HEADER} > /dev/null 2>&1 \
@@ -1143,6 +1153,7 @@  ${host_builddir}/c++config.h: ${CONFIG_HEADER} \
 	-e "s,define _GLIBCXX_INLINE_VERSION, define _GLIBCXX_INLINE_VERSION $$ns_version," \
 	-e "s,define _GLIBCXX_HAVE_ATTRIBUTE_VISIBILITY, define _GLIBCXX_HAVE_ATTRIBUTE_VISIBILITY $$visibility," \
 	-e "s,define _GLIBCXX_EXTERN_TEMPLATE$$, define _GLIBCXX_EXTERN_TEMPLATE $$externtemplate," \
+	-e "s,define _GLIBCXX_USE_CXX11_ABI, define _GLIBCXX_USE_CXX11_ABI $$cxx11abi," \
 	-e "$$ldbl_compat" \
 	    < ${glibcxx_srcdir}/include/bits/c++config > $@ ;\
 	sed -e 's/HAVE_/_GLIBCXX_HAVE_/g' \
diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
index ff6afc8..bb58a9b 100644
--- a/libstdc++-v3/include/bits/c++config
+++ b/libstdc++-v3/include/bits/c++config
@@ -193,6 +193,17 @@  namespace std
 #endif
 }
 
+// Use abi_tag("cxx11")
+#ifndef _GLIBCXX_USE_CXX11_ABI
+#define _GLIBCXX_USE_CXX11_ABI
+#endif
+
+#if _GLIBCXX_USE_CXX11_ABI
+# define _GLIBCXX_DEFAULT_ABI_TAG _GLIBCXX_ABI_TAG_CXX11
+#else
+# define _GLIBCXX_DEFAULT_ABI_TAG
+#endif
+
 
 // Defined if inline namespaces are used for versioning.
 #define _GLIBCXX_INLINE_VERSION 
diff --git a/libstdc++-v3/include/bits/list.tcc b/libstdc++-v3/include/bits/list.tcc
index 42855a4..f99ec54 100644
--- a/libstdc++-v3/include/bits/list.tcc
+++ b/libstdc++-v3/include/bits/list.tcc
@@ -89,6 +89,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       {
 	_Node* __tmp = _M_create_node(std::forward<_Args>(__args)...);
 	__tmp->_M_hook(__position._M_const_cast()._M_node);
+	this->_M_inc_size(1);
 	return iterator(__tmp);
       }
 #endif
@@ -104,6 +105,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
     {
       _Node* __tmp = _M_create_node(__x);
       __tmp->_M_hook(__position._M_const_cast()._M_node);
+      this->_M_inc_size(1);
       return iterator(__tmp);
     }
 
@@ -353,6 +355,9 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	      ++__first1;
 	  if (__first2 != __last2)
 	    _M_transfer(__last1, __first2, __last2);
+
+	  this->_M_inc_size(__x._M_get_size());
+	  __x._M_set_size(0);
 	}
     }
 
@@ -387,6 +392,9 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 		++__first1;
 	    if (__first2 != __last2)
 	      _M_transfer(__last1, __first2, __last2);
+
+	    this->_M_inc_size(__x._M_get_size());
+	    __x._M_set_size(0);
 	  }
       }
 
diff --git a/libstdc++-v3/include/bits/stl_list.h b/libstdc++-v3/include/bits/stl_list.h
index dd9bba7..8e6567c 100644
--- a/libstdc++-v3/include/bits/stl_list.h
+++ b/libstdc++-v3/include/bits/stl_list.h
@@ -295,7 +295,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
   /// See bits/stl_deque.h's _Deque_base for an explanation.
   template<typename _Tp, typename _Alloc>
-    class _List_base
+    class _GLIBCXX_DEFAULT_ABI_TAG _List_base
     {
     protected:
       // NOTA BENE
@@ -316,6 +316,19 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
       typedef typename _Alloc::template rebind<_Tp>::other _Tp_alloc_type;
 
+      static size_t
+      _S_distance(const __detail::_List_node_base* __first,
+		  const __detail::_List_node_base* __last)
+      {
+	size_t __n = 0;
+	while (__first != __last)
+	  {
+	    __first = __first->_M_next;
+	    ++__n;
+	  }
+	return __n;
+      }
+
       struct _List_impl
       : public _Node_alloc_type
       {
@@ -338,6 +351,40 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
       _List_impl _M_impl;
 
+#if _GLIBCXX_USE_CXX11_ABI
+      size_t	 _M_size;
+
+      size_t _M_get_size() const { return _M_size; }
+
+      void _M_set_size(size_t __n) { _M_size = __n; }
+
+      void _M_inc_size(size_t __n) { _M_size += __n; }
+
+      void _M_dec_size(size_t __n) { _M_size -= __n; }
+
+      size_t
+      _M_distance(const __detail::_List_node_base* __first,
+		  const __detail::_List_node_base* __last) const
+      { return _S_distance(__first, __last); }
+
+      // return the stored size
+      size_t _M_node_count() const { return _M_size; }
+#else
+      // dummy implementations used when the size is not stored
+      size_t _M_get_size() const { return 0; }
+      void _M_set_size(size_t) { }
+      void _M_inc_size(size_t) { }
+      void _M_dec_size(size_t) { }
+      size_t _M_distance(const void*, const void*) const { return 0; }
+
+      // count the number of nodes
+      size_t _M_node_count() const
+      {
+	return _S_distance(_M_impl._M_node._M_next,
+			   std::__addressof(_M_impl._M_node));
+      }
+#endif
+
       _List_node<_Tp>*
       _M_get_node()
       { return _M_impl._Node_alloc_type::allocate(1); }
@@ -386,7 +433,8 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	    __node->_M_next = __xnode->_M_next;
 	    __node->_M_prev = __xnode->_M_prev;
 	    __node->_M_next->_M_prev = __node->_M_prev->_M_next = __node;
-	    __xnode->_M_next = __xnode->_M_prev = __xnode;
+	    _M_set_size(__x._M_get_size());
+	    __x._M_init();
 	  }
       }
 #endif
@@ -403,6 +451,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       {
         this->_M_impl._M_node._M_next = &this->_M_impl._M_node;
         this->_M_impl._M_node._M_prev = &this->_M_impl._M_node;
+	_M_set_size(0);
       }
     };
 
@@ -453,7 +502,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
    *  %empty. 
   */
   template<typename _Tp, typename _Alloc = std::allocator<_Tp> >
-    class list : protected _List_base<_Tp, _Alloc>
+    class _GLIBCXX_DEFAULT_ABI_TAG list : protected _List_base<_Tp, _Alloc>
     {
       // concept requirements
       typedef typename _Alloc::value_type                _Alloc_value_type;
@@ -893,7 +942,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**  Returns the number of elements in the %list.  */
       size_type
       size() const _GLIBCXX_NOEXCEPT
-      { return std::distance(begin(), end()); }
+      { return this->_M_node_count(); }
 
       /**  Returns the size() of the largest possible %list.  */
       size_type
@@ -1295,6 +1344,10 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	__detail::_List_node_base::swap(this->_M_impl._M_node, 
 					__x._M_impl._M_node);
 
+	size_t __xsize = __x._M_get_size();
+	__x._M_set_size(this->_M_get_size());
+	this->_M_set_size(__xsize);
+
 	// _GLIBCXX_RESOLVE_LIB_DEFECTS
 	// 431. Swapping containers with unequal allocators.
 	std::__alloc_swap<typename _Base::_Node_alloc_type>::
@@ -1339,6 +1392,9 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
 	    this->_M_transfer(__position._M_const_cast(),
 			      __x.begin(), __x.end());
+
+	    this->_M_inc_size(__x._M_get_size());
+	    __x._M_set_size(0);
 	  }
       }
 
@@ -1385,6 +1441,9 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
 	this->_M_transfer(__position._M_const_cast(),
 			  __i._M_const_cast(), __j);
+
+	this->_M_inc_size(1);
+	__x._M_dec_size(1);
       }
 
 #if __cplusplus >= 201103L
@@ -1443,6 +1502,10 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	    if (this != &__x)
 	      _M_check_equal_allocators(__x);
 
+	    size_t __n = this->_M_distance(__first._M_node, __last._M_node);
+	    this->_M_inc_size(__n);
+	    __x._M_dec_size(__n);
+
 	    this->_M_transfer(__position._M_const_cast(),
 			      __first._M_const_cast(),
 			      __last._M_const_cast());
@@ -1688,6 +1751,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       {
         _Node* __tmp = _M_create_node(__x);
         __tmp->_M_hook(__position._M_node);
+	this->_M_inc_size(1);
       }
 #else
      template<typename... _Args>
@@ -1696,6 +1760,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
        {
 	 _Node* __tmp = _M_create_node(std::forward<_Args>(__args)...);
 	 __tmp->_M_hook(__position._M_node);
+	 this->_M_inc_size(1);
        }
 #endif
 
@@ -1703,6 +1768,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       void
       _M_erase(iterator __position) _GLIBCXX_NOEXCEPT
       {
+	this->_M_dec_size(1);
         __position._M_node->_M_unhook();
         _Node* __n = static_cast<_Node*>(__position._M_node);
 #if __cplusplus >= 201103L
diff --git a/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/assign_neg.cc b/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/assign_neg.cc
index 885e753..183753d 100644
--- a/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/assign_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/assign_neg.cc
@@ -18,7 +18,7 @@ 
 // <http://www.gnu.org/licenses/>.
 
 // { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1665 }
+// { dg-error "no matching" "" { target *-*-* } 1728 }
 
 #include <list>
 
diff --git a/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/constructor_1_neg.cc b/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/constructor_1_neg.cc
index ccd9d17..e81ff98 100644
--- a/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/constructor_1_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/constructor_1_neg.cc
@@ -18,7 +18,7 @@ 
 // <http://www.gnu.org/licenses/>.
 
 // { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1617 }
+// { dg-error "no matching" "" { target *-*-* } 1680 }
 
 #include <list>
 
diff --git a/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/constructor_2_neg.cc b/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/constructor_2_neg.cc
index 04966dc..c98aa0f 100644
--- a/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/constructor_2_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/constructor_2_neg.cc
@@ -18,7 +18,7 @@ 
 // <http://www.gnu.org/licenses/>.
 
 // { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1617 }
+// { dg-error "no matching" "" { target *-*-* } 1680 }
 
 #include <list>
 #include <utility>
diff --git a/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/insert_neg.cc b/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/insert_neg.cc
index 759dad6..1397796 100644
--- a/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/insert_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/insert_neg.cc
@@ -18,7 +18,7 @@ 
 // <http://www.gnu.org/licenses/>.
 
 // { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1617 }
+// { dg-error "no matching" "" { target *-*-* } 1680 }
 
 #include <list>
 
diff --git a/libstdc++-v3/testsuite/ext/profile/mutex_extensions_neg.cc b/libstdc++-v3/testsuite/ext/profile/mutex_extensions_neg.cc
index 17df71a..5597c57 100644
--- a/libstdc++-v3/testsuite/ext/profile/mutex_extensions_neg.cc
+++ b/libstdc++-v3/testsuite/ext/profile/mutex_extensions_neg.cc
@@ -25,4 +25,4 @@ 
 
 #include <vector>
 
-// { dg-error "multiple inlined namespaces" "" { target *-*-* } 279 }
+// { dg-error "multiple inlined namespaces" "" { target *-*-* } 290 }