diff mbox

[v3] define string::pop_back()

Message ID CAH6eHdSWO3Cq9LT6BnKfRYmQDQp=pyk-kBRqSvnJQFr34=UPGg@mail.gmail.com
State New
Headers show

Commit Message

Jonathan Wakely Nov. 7, 2011, 12:06 a.m. UTC
I first posted this a month ago, this adjusts the exports and moves
the tests under separate char and wchar_t directories as requested by
Paolo.

        * include/bits/basic_string.h (basic_string::at): Move adjacent to other
        overload.
        (basic_string::pop_back): Define.
        * include/debug/string (__gnu_debug::basic_string::pop_back): Likewise.
        * include/ext/vstring.h (__versa_string::pop_back): Likewise.
        * config/abi/pre/gnu.ver: Add new symbols.
        * testsuite/21_strings/basic_string/modifiers/char/pop_back.cc: New.
        * testsuite/21_strings/basic_string/modifiers/wchar_t/pop_back.cc: New.
        * testsuite/21_strings/basic_string/range_access.cc: Split to ...
        * testsuite/21_strings/basic_string/range_access/char/1.cc: Here and ...
        * testsuite/21_strings/basic_string/range_access/wchar_t/1.cc: Here.
        * testsuite/ext/vstring/modifiers/char/pop_back.cc: New.
        * testsuite/ext/vstring/modifiers/wchar_t/pop_back.cc: New.

Tested x86_64-linux, committed to trunk.

Comments

Eric Botcazou Nov. 7, 2011, 9:17 a.m. UTC | #1
>         * include/bits/basic_string.h (basic_string::at): Move adjacent to
> other overload.
>         (basic_string::pop_back): Define.
>         * include/debug/string (__gnu_debug::basic_string::pop_back):
> Likewise. * include/ext/vstring.h (__versa_string::pop_back): Likewise. *
> config/abi/pre/gnu.ver: Add new symbols.
>         * testsuite/21_strings/basic_string/modifiers/char/pop_back.cc:
> New. * testsuite/21_strings/basic_string/modifiers/wchar_t/pop_back.cc:
> New. * testsuite/21_strings/basic_string/range_access.cc: Split to ... *
> testsuite/21_strings/basic_string/range_access/char/1.cc: Here and ... *
> testsuite/21_strings/basic_string/range_access/wchar_t/1.cc: Here. *
> testsuite/ext/vstring/modifiers/char/pop_back.cc: New.
>         * testsuite/ext/vstring/modifiers/wchar_t/pop_back.cc: New.

This breaks bootstrap on Solaris:

ld: fatal: libstdc++-symbols.ver-sun: 4806: symbol `std::basic_string<char, 
std::char_traits<char>, std::allocator<char> >::pop_back()' is already defined 
in file: libstdc++-symbols.ver-sun

_ZNSs8pop_backEv is referenced twice in the version file.
Jonathan Wakely Nov. 7, 2011, 9:20 a.m. UTC | #2
On 7 November 2011 09:17, Eric Botcazou wrote:
>>         * include/bits/basic_string.h (basic_string::at): Move adjacent to
>> other overload.
>>         (basic_string::pop_back): Define.
>>         * include/debug/string (__gnu_debug::basic_string::pop_back):
>> Likewise. * include/ext/vstring.h (__versa_string::pop_back): Likewise. *
>> config/abi/pre/gnu.ver: Add new symbols.
>>         * testsuite/21_strings/basic_string/modifiers/char/pop_back.cc:
>> New. * testsuite/21_strings/basic_string/modifiers/wchar_t/pop_back.cc:
>> New. * testsuite/21_strings/basic_string/range_access.cc: Split to ... *
>> testsuite/21_strings/basic_string/range_access/char/1.cc: Here and ... *
>> testsuite/21_strings/basic_string/range_access/wchar_t/1.cc: Here. *
>> testsuite/ext/vstring/modifiers/char/pop_back.cc: New.
>>         * testsuite/ext/vstring/modifiers/wchar_t/pop_back.cc: New.
>
> This breaks bootstrap on Solaris:
>
> ld: fatal: libstdc++-symbols.ver-sun: 4806: symbol `std::basic_string<char,
> std::char_traits<char>, std::allocator<char> >::pop_back()' is already defined
> in file: libstdc++-symbols.ver-sun
>
> _ZNSs8pop_backEv is referenced twice in the version file.

Thanks, I'll try to work out the right adjustment for the solaris
symbols file but can't test it.
Eric Botcazou Nov. 7, 2011, 9:40 a.m. UTC | #3
> Thanks, I'll try to work out the right adjustment for the solaris
> symbols file but can't test it.

The 2 new symbols are duplicated, because they are matched by a regexp:

    ##_ZNSs[0-58-9][g-z]* (glob)
    _ZNSs4nposE;
    _ZNSs4rendEv;
    _ZNSs4swapERSs;
    _ZNSs8pop_backEv;
    _ZNSs9push_backEc;

    ##_ZNSbIwSt11char_traitsIwESaIwEE[0-58-9][g-z]* (glob)
    _ZNSbIwSt11char_traitsIwESaIwEE4nposE;
    _ZNSbIwSt11char_traitsIwESaIwEE4rendEv;
    _ZNSbIwSt11char_traitsIwESaIwEE4swapERS2_;
    _ZNSbIwSt11char_traitsIwESaIwEE8pop_backEv;
    _ZNSbIwSt11char_traitsIwESaIwEE9push_backEw;

so removing the 8 in the regexp seems to be sufficient.
Jonathan Wakely Nov. 7, 2011, 10:34 a.m. UTC | #4
On 7 November 2011 09:40, Eric Botcazou wrote:
>> Thanks, I'll try to work out the right adjustment for the solaris
>> symbols file but can't test it.
>
> The 2 new symbols are duplicated, because they are matched by a regexp:
>
>    ##_ZNSs[0-58-9][g-z]* (glob)
>    _ZNSs4nposE;
>    _ZNSs4rendEv;
>    _ZNSs4swapERSs;
>    _ZNSs8pop_backEv;
>    _ZNSs9push_backEc;
>
>    ##_ZNSbIwSt11char_traitsIwESaIwEE[0-58-9][g-z]* (glob)
>    _ZNSbIwSt11char_traitsIwESaIwEE4nposE;
>    _ZNSbIwSt11char_traitsIwESaIwEE4rendEv;
>    _ZNSbIwSt11char_traitsIwESaIwEE4swapERS2_;
>    _ZNSbIwSt11char_traitsIwESaIwEE8pop_backEv;
>    _ZNSbIwSt11char_traitsIwESaIwEE9push_backEw;
>
> so removing the 8 in the regexp seems to be sufficient.

Won't that fail to match string::max_size?  For GNU I added a regex
for that explicitly:

+    _ZNKSs8max_size*;
+    _ZNKSbIwSt11char_traitsIwESaIwEE8max_size*;
Eric Botcazou Nov. 7, 2011, 10:57 a.m. UTC | #5
> Won't that fail to match string::max_size?

No, if it doesn't appear in the list, then it isn't matched by the regexp.

> For GNU I added a regex for that explicitly:
>
> +    _ZNKSs8max_size*;
> +    _ZNKSbIwSt11char_traitsIwESaIwEE8max_size*;

What is GNU here?  The Solaris version file is a massaged version of gnu.ver.
You patched gnu.ver incorrectly so symbols are now matched twice; you need to 
fix that by avoiding the double matching.
diff mbox

Patch

Index: include/bits/basic_string.h
===================================================================
--- include/bits/basic_string.h	(revision 181047)
+++ include/bits/basic_string.h	(working copy)
@@ -865,6 +865,26 @@ 
 	return _M_data()[__n];
       }
 
+      /**
+       *  @brief  Provides access to the data contained in the %string.
+       *  @param __n The index of the character to access.
+       *  @return  Read/write reference to the character.
+       *  @throw  std::out_of_range  If @a n is an invalid index.
+       *
+       *  This function provides for safer data access.  The parameter is
+       *  first checked that it is in the range of the string.  The function
+       *  throws out_of_range if the check fails.  Success results in
+       *  unsharing the string.
+       */
+      reference
+      at(size_type __n)
+      {
+	if (__n >= size())
+	  __throw_out_of_range(__N("basic_string::at"));
+	_M_leak();
+	return _M_data()[__n];
+      }
+
 #ifdef __GXX_EXPERIMENTAL_CXX0X__
       /**
        *  Returns a read/write reference to the data at the first
@@ -899,26 +919,6 @@ 
       { return operator[](this->size() - 1); }
 #endif
 
-      /**
-       *  @brief  Provides access to the data contained in the %string.
-       *  @param __n The index of the character to access.
-       *  @return  Read/write reference to the character.
-       *  @throw  std::out_of_range  If @a n is an invalid index.
-       *
-       *  This function provides for safer data access.  The parameter is
-       *  first checked that it is in the range of the string.  The function
-       *  throws out_of_range if the check fails.  Success results in
-       *  unsharing the string.
-       */
-      reference
-      at(size_type __n)
-      {
-	if (__n >= size())
-	  __throw_out_of_range(__N("basic_string::at"));
-	_M_leak();
-	return _M_data()[__n];
-      }
-
       // Modifiers:
       /**
        *  @brief  Append a string to this string.
@@ -1394,7 +1394,18 @@ 
       iterator
       erase(iterator __first, iterator __last);
  
+#ifdef __GXX_EXPERIMENTAL_CXX0X__
       /**
+       *  @brief  Remove the last character.
+       *
+       *  The string must be non-empty.
+       */
+      void
+      pop_back()
+      { erase(size()-1, 1); }
+#endif // __GXX_EXPERIMENTAL_CXX0X__
+
+      /**
        *  @brief  Replace characters with value from another string.
        *  @param __pos  Index of first character to replace.
        *  @param __n  Number of characters to be replaced.
Index: include/debug/string
===================================================================
--- include/debug/string	(revision 181047)
+++ include/debug/string	(working copy)
@@ -580,6 +580,16 @@ 
       return iterator(__res, this);
     }
 
+#ifdef __GXX_EXPERIMENTAL_CXX0X__
+    void
+    pop_back()
+    {
+      __glibcxx_check_nonempty();
+      _Base::pop_back();
+      this->_M_invalidate_all();
+    }
+#endif // __GXX_EXPERIMENTAL_CXX0X__
+
     basic_string&
     replace(size_type __pos1, size_type __n1, const basic_string& __str)
     {
Index: include/ext/vstring.h
===================================================================
--- include/ext/vstring.h	(revision 181047)
+++ include/ext/vstring.h	(working copy)
@@ -1140,7 +1140,18 @@ 
 	return iterator(this->_M_data() + __pos);
       }
 
+#ifdef __GXX_EXPERIMENTAL_CXX0X__
       /**
+       *  @brief  Remove the last character.
+       *
+       *  The string must be non-empty.
+       */
+      void
+      pop_back()
+      { this->_M_erase(size()-1, 1); }
+#endif // __GXX_EXPERIMENTAL_CXX0X__
+
+      /**
        *  @brief  Replace characters with value from another string.
        *  @param __pos  Index of first character to replace.
        *  @param __n  Number of characters to be replaced.
Index: config/abi/pre/gnu.ver
===================================================================
--- config/abi/pre/gnu.ver	(revision 181047)
+++ config/abi/pre/gnu.ver	(working copy)
@@ -225,9 +225,10 @@ 
     _ZNKSs[0-3][a-b]*;
     _ZNKSs[5-9][a-b]*;
     _ZNKSs[0-9][d-e]*;
-    _ZNKSs[0-9][g-z]*;
+    _ZNKSs[0-79][g-z]*;
     _ZNKSs[0-9][0-9][a-z]*;
     _ZNKSs4find*;
+    _ZNKSs8max_size*;
     _ZNKSs[a-z]*;
     _ZNKSs4_Rep12_M_is_leakedEv;
     _ZNKSs4_Rep12_M_is_sharedEv;
@@ -286,10 +287,11 @@ 
     _ZNKSbIwSt11char_traitsIwESaIwEE[0-3][a-b]*;
     _ZNKSbIwSt11char_traitsIwESaIwEE[5-9][a-b]*;
     _ZNKSbIwSt11char_traitsIwESaIwEE[0-9][d-e]*;
-    _ZNKSbIwSt11char_traitsIwESaIwEE[0-9][g-z]*;
+    _ZNKSbIwSt11char_traitsIwESaIwEE[0-79][g-z]*;
     _ZNKSbIwSt11char_traitsIwESaIwEE[0-9][0-9][a-z]*;
     _ZNKSbIwSt11char_traitsIwESaIwEE[a-z]*;
     _ZNKSbIwSt11char_traitsIwESaIwEE4find*;
+    _ZNKSbIwSt11char_traitsIwESaIwEE8max_size*;
     _ZNKSbIwSt11char_traitsIwESaIwEE4_Rep12_M_is_leakedEv;
     _ZNKSbIwSt11char_traitsIwESaIwEE4_Rep12_M_is_sharedEv;
     _ZNKSbIwSt11char_traitsIwESaIwEE6_M_repEv;
@@ -1302,6 +1304,11 @@ 
     _ZNSt14numeric_limitsInE*;
     _ZNSt14numeric_limitsIoE*;
 
+    # std::string::pop_back()
+    _ZNSs8pop_backEv;
+    # std::wstring::pop_back()
+    _ZNSbIwSt11char_traitsIwESaIwEE8pop_backEv;
+
 } GLIBCXX_3.4.16;
 
 # Symbols in the support library (libsupc++) have their own tag.
Index: testsuite/21_strings/basic_string/modifiers/wchar_t/pop_back.cc
===================================================================
--- testsuite/21_strings/basic_string/modifiers/wchar_t/pop_back.cc	(revision 0)
+++ testsuite/21_strings/basic_string/modifiers/wchar_t/pop_back.cc	(revision 0)
@@ -0,0 +1,41 @@ 
+// Copyright (C) 2011 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/>.
+
+// 21.4.6.5 basic_string::pop_back
+// { dg-options "-std=gnu++0x" }
+
+#include <string>
+#include <testsuite_hooks.h>
+
+void test01()
+{
+  bool test __attribute__((unused)) = true;
+
+  const std::wstring cstr(L"Badger");
+  std::wstring str = cstr;
+  str.pop_back();
+  VERIFY( str.size() == cstr.size() - 1 );
+
+  str += cstr.back();
+  VERIFY( str == cstr );
+}
+
+int main()
+{ 
+  test01();
+  return 0;
+}
Index: testsuite/21_strings/basic_string/modifiers/char/pop_back.cc
===================================================================
--- testsuite/21_strings/basic_string/modifiers/char/pop_back.cc	(revision 0)
+++ testsuite/21_strings/basic_string/modifiers/char/pop_back.cc	(revision 0)
@@ -0,0 +1,41 @@ 
+// Copyright (C) 2011 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/>.
+
+// 21.4.6.5 basic_string::pop_back
+// { dg-options "-std=gnu++0x" }
+
+#include <string>
+#include <testsuite_hooks.h>
+
+void test01()
+{
+  bool test __attribute__((unused)) = true;
+
+  const std::string cstr("Badger");
+  std::string str = cstr;
+  str.pop_back();
+  VERIFY( str.size() == cstr.size() - 1 );
+
+  str += cstr.back();
+  VERIFY( str == cstr );
+}
+
+int main()
+{ 
+  test01();
+  return 0;
+}
Index: testsuite/21_strings/basic_string/range_access.cc
===================================================================
--- testsuite/21_strings/basic_string/range_access.cc	(revision 181047)
+++ testsuite/21_strings/basic_string/range_access.cc	(working copy)
@@ -1,37 +0,0 @@ 
-// { dg-do compile }
-// { dg-options "-std=gnu++0x" }
-
-// Copyright (C) 2010 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 Pred 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/>.
-
-// 24.6.5, range access [iterator.range]
-
-#include <string>
-
-void
-test01()
-{
-  std::string s("Hello, World!");
-  std::begin(s);
-  std::end(s);
-
-#ifdef _GLIBCXX_USE_WCHAR_T
-  std::wstring ws(L"Hello, World!");
-  std::begin(ws);
-  std::end(ws);
-#endif
-}
Index: testsuite/21_strings/basic_string/range_access/wchar_t/1.cc
===================================================================
--- testsuite/21_strings/basic_string/range_access/wchar_t/1.cc	(revision 0)
+++ testsuite/21_strings/basic_string/range_access/wchar_t/1.cc	(revision 0)
@@ -0,0 +1,33 @@ 
+// { dg-do compile }
+// { dg-options "-std=gnu++0x" }
+
+// Copyright (C) 2010. 2011 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 Pred 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/>.
+
+// 24.6.5, range access [iterator.range]
+
+#include <string>
+
+void
+test01()
+{
+#ifdef _GLIBCXX_USE_WCHAR_T
+  std::wstring ws(L"Hello, World!");
+  std::begin(ws);
+  std::end(ws);
+#endif
+}
Index: testsuite/21_strings/basic_string/range_access/char/1.cc
===================================================================
--- testsuite/21_strings/basic_string/range_access/char/1.cc	(revision 0)
+++ testsuite/21_strings/basic_string/range_access/char/1.cc	(revision 0)
@@ -0,0 +1,31 @@ 
+// { dg-do compile }
+// { dg-options "-std=gnu++0x" }
+
+// Copyright (C) 2010, 2011 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 Pred 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/>.
+
+// 24.6.5, range access [iterator.range]
+
+#include <string>
+
+void
+test01()
+{
+  std::string s("Hello, World!");
+  std::begin(s);
+  std::end(s);
+}
Index: testsuite/ext/vstring/modifiers/wchar_t/pop_back.cc
===================================================================
--- testsuite/ext/vstring/modifiers/wchar_t/pop_back.cc	(revision 0)
+++ testsuite/ext/vstring/modifiers/wchar_t/pop_back.cc	(revision 0)
@@ -0,0 +1,45 @@ 
+// Copyright (C) 2011 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/>.
+
+// 21.4.6.5 basic_string::pop_back
+// { dg-options "-std=gnu++0x" }
+
+#include <ext/vstring.h>
+#include <testsuite_hooks.h>
+
+template<typename StrT>
+int test01()
+{
+  bool test __attribute__((unused)) = true;
+
+  const StrT cstr(L"Badger");
+  StrT str = cstr;
+  str.pop_back();
+  VERIFY( str.size() == cstr.size() - 1 );
+
+  str += cstr.back();
+  VERIFY( str == cstr );
+
+  return test;
+}
+
+int main()
+{ 
+  test01<__gnu_cxx::__wsso_string>();
+  test01<__gnu_cxx::__wrc_string>();
+  return 0;
+}
Index: testsuite/ext/vstring/modifiers/char/pop_back.cc
===================================================================
--- testsuite/ext/vstring/modifiers/char/pop_back.cc	(revision 0)
+++ testsuite/ext/vstring/modifiers/char/pop_back.cc	(revision 0)
@@ -0,0 +1,45 @@ 
+// Copyright (C) 2011 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/>.
+
+// 21.4.6.5 basic_string::pop_back
+// { dg-options "-std=gnu++0x" }
+
+#include <ext/vstring.h>
+#include <testsuite_hooks.h>
+
+template<typename StrT>
+int test01()
+{
+  bool test __attribute__((unused)) = true;
+
+  const StrT cstr("Badger");
+  StrT str = cstr;
+  str.pop_back();
+  VERIFY( str.size() == cstr.size() - 1 );
+
+  str += cstr.back();
+  VERIFY( str == cstr );
+
+  return test;
+}
+
+int main()
+{ 
+  test01<__gnu_cxx::__sso_string>();
+  test01<__gnu_cxx::__rc_string>();
+  return 0;
+}