diff mbox

Add non-const overload of std::string::data()

Message ID 20160713111244.GA12384@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely July 13, 2016, 11:12 a.m. UTC
Also fix confusion between pointer and _CharT*, so that allocators with
fancy pointers work correctly.

The _M_data() function returns pointer, but we were using it where
_CharT* was required. This introduces a new _M_c_str() function which
returns a _CharT* instead, and uses that for c_str() and both
overloads of data().

Most places using _M_data() can just use data() instead, as they only
need a const _CharT*. A few need to use _M_c_str() to get _CharT* so
they can write to it (they could use the new data() overload, except
that it's not defined until C++17).

Tested x86_64-linux, committed to trunk.

	* include/bits/basic_string.h [_GLIBCXX_USE_CXX11_ABI] (_M_c_str):
	New function.
	(_M_disjunct, basic_string(const basic_string&, size_t)): Use data()
	instead of _M_data().
	(basic_string(const basic_string&, size_t, size_t, const _Alloc&)):
	Likewise.
	(append(const basic_string&)): Likewise.
	(append(const basic_string&, size_type, size_type)): Likewise.
	(assign(const basic_string&, size_type, size_type)): Likewise.
	(insert(size_type, const basic_string&)): Likewise.
	(insert(size_type, const basic_string&, size_type, size_type)):
	Likewise.
	(replace(size_type, size_type, const basic_string&, size_type,
	size_type)): Likewise.
	(replace(__const_iterator, __const_iterator, const basic_string&)):
	Likewise.
	(c_str(), data()): Use c_str() instead of _M_data().
	(data()): Add non-const overload as per LWG 2391 and P0272R1.
	(compare(const basic_string&)): Use data() instead of _M_data().
	[!_GLIBCXX_USE_CXX11_ABI] (data()): Add non-const overload.
	* include/bits/basic_string.tcc [_GLIBCXX_USE_CXX11_ABI] (_M_mutate):
	Pass raw pointers to _S_copy.
	(_M_erase, _M_replace_aux): Pass raw pointers to _S_move and
	_S_assign.
	(find(const _CharT*, size_type, size_type)): Use data instead of
	_M_data().
	* testsuite/21_strings/basic_string/allocator/char/ext_ptr.cc: New.
	* testsuite/21_strings/basic_string/operations/data/char/2.cc: New.
	* testsuite/21_strings/basic_string/operations/data/wchar_t/2.cc: New.
commit 4300aa2087d90cd2d55852fa38f082a89bc7e72d
Author: redi <redi@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Wed Jul 13 11:08:37 2016 +0000

    Add non-const overload of std::string::data()
    
    Also fix confusion between pointer and _CharT*, so that allocators with
    fancy pointers work correctly.
    
    	* include/bits/basic_string.h [_GLIBCXX_USE_CXX11_ABI] (_M_c_str):
    	New function.
    	(_M_disjunct, basic_string(const basic_string&, size_t)): Use data()
    	instead of _M_data().
    	(basic_string(const basic_string&, size_t, size_t, const _Alloc&)):
    	Likewise.
    	(append(const basic_string&)): Likewise.
    	(append(const basic_string&, size_type, size_type)): Likewise.
    	(assign(const basic_string&, size_type, size_type)): Likewise.
    	(insert(size_type, const basic_string&)): Likewise.
    	(insert(size_type, const basic_string&, size_type, size_type)):
    	Likewise.
    	(replace(size_type, size_type, const basic_string&, size_type,
    	size_type)): Likewise.
    	(replace(__const_iterator, __const_iterator, const basic_string&)):
    	Likewise.
    	(c_str(), data()): Use c_str() instead of _M_data().
    	(data()): Add non-const overload as per LWG 2391 and P0272R1.
    	(compare(const basic_string&)): Use data() instead of _M_data().
    	[!_GLIBCXX_USE_CXX11_ABI] (data()): Add non-const overload.
    	* include/bits/basic_string.tcc [_GLIBCXX_USE_CXX11_ABI] (_M_mutate):
    	Pass raw pointers to _S_copy.
    	(_M_erase, _M_replace_aux): Pass raw pointers to _S_move and
    	_S_assign.
    	(find(const _CharT*, size_type, size_type)): Use data instead of
    	_M_data().
    	* testsuite/21_strings/basic_string/allocator/char/ext_ptr.cc: New.
    	* testsuite/21_strings/basic_string/operations/data/char/2.cc: New.
    	* testsuite/21_strings/basic_string/operations/data/wchar_t/2.cc: New.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@238291 138bc75d-0d04-0410-961f-82ee72b054a4

Comments

Jonathan Wakely July 13, 2016, 2:59 p.m. UTC | #1
On 13/07/16 12:12 +0100, Jonathan Wakely wrote:
>Also fix confusion between pointer and _CharT*, so that allocators with
>fancy pointers work correctly.
>
>The _M_data() function returns pointer, but we were using it where
>_CharT* was required. This introduces a new _M_c_str() function which
>returns a _CharT* instead, and uses that for c_str() and both
>overloads of data().
>
>Most places using _M_data() can just use data() instead, as they only
>need a const _CharT*. A few need to use _M_c_str() to get _CharT* so
>they can write to it (they could use the new data() overload, except
>that it's not defined until C++17).
>
>Tested x86_64-linux, committed to trunk.
>
>	* include/bits/basic_string.h [_GLIBCXX_USE_CXX11_ABI] (_M_c_str):
>	New function.

I forgot to add the new exports to the commit, new patch coming
shortly...
Jonathan Wakely July 13, 2016, 3:57 p.m. UTC | #2
On 13/07/16 15:59 +0100, Jonathan Wakely wrote:
>On 13/07/16 12:12 +0100, Jonathan Wakely wrote:
>>Also fix confusion between pointer and _CharT*, so that allocators with
>>fancy pointers work correctly.
>>
>>The _M_data() function returns pointer, but we were using it where
>>_CharT* was required. This introduces a new _M_c_str() function which
>>returns a _CharT* instead, and uses that for c_str() and both
>>overloads of data().
>>
>>Most places using _M_data() can just use data() instead, as they only
>>need a const _CharT*. A few need to use _M_c_str() to get _CharT* so
>>they can write to it (they could use the new data() overload, except
>>that it's not defined until C++17).
>>
>>Tested x86_64-linux, committed to trunk.
>>
>>	* include/bits/basic_string.h [_GLIBCXX_USE_CXX11_ABI] (_M_c_str):
>>	New function.
>
>I forgot to add the new exports to the commit, new patch coming
>shortly...

I'm reverting that change for now. We need to regenerate baseline
symbols files before we can bump the shared lib version and add new
symbols.
diff mbox

Patch

diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index 374c985..f60d9e0 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -155,6 +155,11 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
 #endif
       }
 
+      // Get a raw pointer (rather than _Alloc::pointer).
+      _CharT*
+      _M_c_str() const
+      { return std::__addressof(*_M_dataplus._M_p); }
+
       void
       _M_capacity(size_type __capacity)
       { _M_allocated_capacity = __capacity; }
@@ -285,8 +290,8 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
       bool
       _M_disjunct(const _CharT* __s) const _GLIBCXX_NOEXCEPT
       {
-	return (less<const _CharT*>()(__s, _M_data())
-		|| less<const _CharT*>()(_M_data() + this->size(), __s));
+	return (less<const _CharT*>()(__s, data())
+		|| less<const _CharT*>()(data() + this->size(), __s));
       }
 
       // When __n = 1 way faster than the general multichar
@@ -411,7 +416,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
 		   size_type __n = npos)
       : _M_dataplus(_M_local_data())
       {
-	const _CharT* __start = __str._M_data()
+	const _CharT* __start = __str.data()
 	  + __str._M_check(__pos, "basic_string::basic_string");
 	_M_construct(__start, __start + __str._M_limit(__pos, __n));
       }
@@ -427,8 +432,8 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
 		   size_type __n, const _Alloc& __a)
       : _M_dataplus(_M_local_data(), __a)
       {
-	const _CharT* __start
-	  = __str._M_data() + __str._M_check(__pos, "string::string");
+	const _CharT* __start = __str.data()
+	  + __str._M_check(__pos, "basic_string::basic_string");
 	_M_construct(__start, __start + __str._M_limit(__pos, __n));
       }
 
@@ -1066,7 +1071,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
        */
       basic_string&
       append(const basic_string& __str)
-      { return _M_append(__str._M_data(), __str.size()); }
+      { return _M_append(__str.data(), __str.size()); }
 
       /**
        *  @brief  Append a substring.
@@ -1083,7 +1088,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
        */
       basic_string&
       append(const basic_string& __str, size_type __pos, size_type __n)
-      { return _M_append(__str._M_data()
+      { return _M_append(__str.data()
 			 + __str._M_check(__pos, "basic_string::append"),
 			 __str._M_limit(__pos, __n)); }
 
@@ -1216,7 +1221,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
        */
       basic_string&
       assign(const basic_string& __str, size_type __pos, size_type __n)
-      { return _M_replace(size_type(0), this->size(), __str._M_data()
+      { return _M_replace(size_type(0), this->size(), __str.data()
 			  + __str._M_check(__pos, "basic_string::assign"),
 			  __str._M_limit(__pos, __n)); }
 
@@ -1413,7 +1418,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
       basic_string&
       insert(size_type __pos1, const basic_string& __str)
       { return this->replace(__pos1, size_type(0),
-			     __str._M_data(), __str.size()); }
+			     __str.data(), __str.size()); }
 
       /**
        *  @brief  Insert a substring.
@@ -1436,7 +1441,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
       basic_string&
       insert(size_type __pos1, const basic_string& __str,
 	     size_type __pos2, size_type __n)
-      { return this->replace(__pos1, size_type(0), __str._M_data()
+      { return this->replace(__pos1, size_type(0), __str.data()
 			     + __str._M_check(__pos2, "basic_string::insert"),
 			     __str._M_limit(__pos2, __n)); }
 
@@ -1619,7 +1624,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
       */
       basic_string&
       replace(size_type __pos, size_type __n, const basic_string& __str)
-      { return this->replace(__pos, __n, __str._M_data(), __str.size()); }
+      { return this->replace(__pos, __n, __str.data(), __str.size()); }
 
       /**
        *  @brief  Replace characters with value from another string.
@@ -1642,7 +1647,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
       basic_string&
       replace(size_type __pos1, size_type __n1, const basic_string& __str,
 	      size_type __pos2, size_type __n2)
-      { return this->replace(__pos1, __n1, __str._M_data()
+      { return this->replace(__pos1, __n1, __str.data()
 			     + __str._M_check(__pos2, "basic_string::replace"),
 			     __str._M_limit(__pos2, __n2)); }
 
@@ -1734,7 +1739,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
       basic_string&
       replace(__const_iterator __i1, __const_iterator __i2,
 	      const basic_string& __str)
-      { return this->replace(__i1, __i2, __str._M_data(), __str.size()); }
+      { return this->replace(__i1, __i2, __str.data(), __str.size()); }
 
       /**
        *  @brief  Replace range of characters with C substring.
@@ -1975,17 +1980,29 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
       */
       const _CharT*
       c_str() const _GLIBCXX_NOEXCEPT
-      { return _M_data(); }
+      { return _M_c_str(); }
 
       /**
        *  @brief  Return const pointer to contents.
        *
-       *  This is a handle to internal data.  Do not modify or dire things may
+       *  This is a pointer to internal data.  Do not modify or dire things may
        *  happen.
       */
       const _CharT*
       data() const _GLIBCXX_NOEXCEPT
-      { return _M_data(); }
+      { return _M_c_str(); }
+
+#if __cplusplus > 201402L
+      /**
+       *  @brief  Return non-const pointer to contents.
+       *
+       *  This is a pointer to the character sequence held by the string.
+       *  Modifying the characters in the sequence is allowed.
+      */
+      _CharT*
+      data() noexcept
+      { return _M_c_str(); }
+#endif
 
       /**
        *  @brief  Return copy of allocator used to construct this string.
@@ -2405,7 +2422,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
 	const size_type __osize = __str.size();
 	const size_type __len = std::min(__size, __osize);
 
-	int __r = traits_type::compare(_M_data(), __str.data(), __len);
+	int __r = traits_type::compare(data(), __str.data(), __len);
 	if (!__r)
 	  __r = _S_compare(__size, __osize);
 	return __r;
@@ -4361,6 +4378,12 @@  _GLIBCXX_END_NAMESPACE_CXX11
       data() const _GLIBCXX_NOEXCEPT
       { return _M_data(); }
 
+#if __cplusplus > 201402L
+      _CharT*
+      data() noexcept
+      { return _M_data(); }
+#endif
+
       /**
        *  @brief  Return copy of allocator used to construct this string.
       */
diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
index 2b6644d..1e6d38e 100644
--- a/libstdc++-v3/include/bits/basic_string.tcc
+++ b/libstdc++-v3/include/bits/basic_string.tcc
@@ -315,14 +315,15 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       size_type __new_capacity = length() + __len2 - __len1;
       pointer __r = _M_create(__new_capacity, capacity());
+      _CharT* __p = std::__addressof(*__r);
 
       if (__pos)
-	this->_S_copy(__r, _M_data(), __pos);
+	this->_S_copy(__p, _M_c_str(), __pos);
       if (__s && __len2)
-	this->_S_copy(__r + __pos, __s, __len2);
+	this->_S_copy(__p + __pos, __s, __len2);
       if (__how_much)
-	this->_S_copy(__r + __pos + __len2,
-		      _M_data() + __pos + __len1, __how_much);
+	this->_S_copy(__p + __pos + __len2,
+		      _M_c_str() + __pos + __len1, __how_much);
 
       _M_dispose();
       _M_data(__r);
@@ -336,8 +337,9 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     {
       const size_type __how_much = length() - __pos - __n;
 
+      _CharT* __p = _M_c_str();
       if (__how_much && __n)
-	this->_S_move(_M_data() + __pos, _M_data() + __pos + __n, __how_much);
+	this->_S_move(__p + __pos, __p + __pos + __n, __how_much);
 
       _M_set_length(length() - __n);
     }
@@ -400,7 +402,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       if (__new_size <= this->capacity())
 	{
-	  pointer __p = this->_M_data() + __pos1;
+	  _CharT* __p = this->_M_c_str() + __pos1;
 
 	  const size_type __how_much = __old_size - __pos1 - __n1;
 	  if (__how_much && __n1 != __n2)
@@ -410,7 +412,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	this->_M_mutate(__pos1, __n1, 0, __n2);
 
       if (__n2)
-	this->_S_assign(this->_M_data() + __pos1, __n2, __c);
+	this->_S_assign(this->_M_c_str() + __pos1, __n2, __c);
 
       this->_M_set_length(__new_size);
       return *this;
@@ -1179,7 +1181,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     {
       __glibcxx_requires_string_len(__s, __n);
       const size_type __size = this->size();
-      const _CharT* __data = _M_data();
+      const _CharT* __data = data();
 
       if (__n == 0)
 	return __pos <= __size ? __pos : npos;
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/ext_ptr.cc b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/ext_ptr.cc
new file mode 100644
index 0000000..0cc8e17
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/ext_ptr.cc
@@ -0,0 +1,52 @@ 
+// Copyright (C) 2016 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/>.
+
+// { dg-options "-std=gnu++11" }
+
+#include <string>
+#include <memory>
+#include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
+
+using C = char;
+const C c = 'a';
+using traits = std::char_traits<C>;
+
+// basic_string is not required to support fancy pointers:
+// http://cplusplus.github.io/LWG/lwg-closed.html#2084
+
+using __gnu_test::CustomPointerAlloc;
+
+void test01()
+{
+#if _GLIBCXX_USE_CXX11_ABI
+  bool test __attribute__((unused)) = true;
+  typedef CustomPointerAlloc<C> alloc_type;
+  typedef std::basic_string<C, traits, alloc_type> test_type;
+  test_type v;
+  v.assign(1, c);
+  VERIFY( ++v.begin() == v.end() );
+
+  v.assign(100, c);
+  VERIFY( (v.begin() + 100) == v.end() );
+#endif
+}
+
+int main()
+{
+  test01();
+}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/data/char/2.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/data/char/2.cc
new file mode 100644
index 0000000..046780d
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/data/char/2.cc
@@ -0,0 +1,40 @@ 
+// Copyright (C) 2016 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/>.
+
+// { dg-options "-std=gnu++17" }
+
+// 21.3.1.7 [string.ops] string operations
+
+#include <string>
+#include <testsuite_hooks.h>
+
+void
+test01()
+{
+  std::string s;
+  char* p = s.data();
+  VERIFY( *p == '\0' );
+  s = "a string that is longer than a short string";
+  p = s.data();
+  VERIFY( p == &s.front() );
+}
+
+int
+main()
+{
+  test01();
+}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/data/wchar_t/2.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/data/wchar_t/2.cc
new file mode 100644
index 0000000..d4a3206
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/data/wchar_t/2.cc
@@ -0,0 +1,40 @@ 
+// Copyright (C) 2016 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/>.
+
+// { dg-options "-std=gnu++17" }
+
+// 21.3.1.7 [string.ops] string operations
+
+#include <string>
+#include <testsuite_hooks.h>
+
+void
+test01()
+{
+  std::wstring s;
+  wchar_t* p = s.data();
+  VERIFY( *p == L'\0' );
+  s = L"a string that is longer than a short string";
+  p = s.data();
+  VERIFY( p == &s.front() );
+}
+
+int
+main()
+{
+  test01();
+}