[v3] basic_string spurious use of a default constructible allocator - LWG2788

Message ID CAAcHv8euPnFShaO6dF308m91t2pxszfB2UT2rDijMNEhn6cVEw@mail.gmail.com
State New
Headers show
Series
  • [v3] basic_string spurious use of a default constructible allocator - LWG2788
Related show

Commit Message

Nina Dinka Ranns May 27, 2019, 10:53 a.m.
Tested on Linux x86_64
basic_string spurious use of a default constructible allocator - LWG2788

2019-05-27  Nina Dinka Ranns  <dinka.ranns@gmail.com>
        basic_string spurious use of a default constructible allocator - LWG2788
        * bits/basic_string.tcc:
        (_M_replace_dispatch()): string temporary now constructed with
the current allocator
        * testsuite/21_strings/basic_string/allocator/char/lwg2788.cc: New
        * testsuite/21_strings/basic_string/allocator/wchar_t/lwg2788.cc: New

Comments

Jonathan Wakely May 30, 2019, 7:48 p.m. | #1
On 27/05/19 11:53 +0100, Nina Dinka Ranns wrote:
>Tested on Linux x86_64

>basic_string spurious use of a default constructible allocator - LWG2788

>

>2019-05-27  Nina Dinka Ranns  <dinka.ranns@gmail.com>

>        basic_string spurious use of a default constructible allocator - LWG2788

>        * bits/basic_string.tcc:

>        (_M_replace_dispatch()): string temporary now constructed with

>the current allocator

>        * testsuite/21_strings/basic_string/allocator/char/lwg2788.cc: New

>        * testsuite/21_strings/basic_string/allocator/wchar_t/lwg2788.cc: New


Thanks!


>Index: libstdc++-v3/include/bits/basic_string.tcc

>===================================================================

>--- libstdc++-v3/include/bits/basic_string.tcc	(revision 271509)

>+++ libstdc++-v3/include/bits/basic_string.tcc	(working copy)

>@@ -381,7 +381,9 @@

> 			  _InputIterator __k1, _InputIterator __k2,

> 			  std::__false_type)

>       {

>-	const basic_string __s(__k1, __k2);

>+    // _GLIBCXX_RESOLVE_LIB_DEFECTS

>+    // 2788. unintentionally require a default constructible allocator


I've fixed the indentation of this comment.

>+	const basic_string __s(__k1, __k2, this->get_allocator());

> 	const size_type __n1 = __i2 - __i1;

> 	return _M_replace(__i1 - begin(), __n1, __s._M_data(),

> 			  __s.size());

>Index: libstdc++-v3/testsuite/20_util/nonesuch/nonesuch.cc

>===================================================================

>--- libstdc++-v3/testsuite/20_util/nonesuch/nonesuch.cc	(revision 271509)

>+++ libstdc++-v3/testsuite/20_util/nonesuch/nonesuch.cc	(nonexistent)


And dropped this part of the patch which spuriously removed an
unrelated file.

Tested powerpc64le-linux with both string ABIs.

The version I've committed is attached.
commit a0ee1818604eb6998e9cc26005ba754021a3f067
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu May 30 17:45:38 2019 +0100

    LWG2788 basic_string spurious use of a default constructible allocator
    
    This only change the cxx11 basic_string, because COW strings don't
    correctly propagate allocators anyway.
    
    2019-05-30  Nina Dinka Ranns  <dinka.ranns@gmail.com>
    
            LWG2788 basic_string spurious use of a default constructible allocator
            * include/bits/basic_string.tcc [_GLIBCXX_USE_CXX11_ABI]
            (basic_string::_M_replace_dispatch): Construct temporary string with
            the current allocator.
            * testsuite/21_strings/basic_string/allocator/char/lwg2788.cc: New.
            * testsuite/21_strings/basic_string/allocator/wchar_t/lwg2788.cc: New.

diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
index e2315cb1467..ab986a6c827 100644
--- a/libstdc++-v3/include/bits/basic_string.tcc
+++ b/libstdc++-v3/include/bits/basic_string.tcc
@@ -381,7 +381,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 			  _InputIterator __k1, _InputIterator __k2,
 			  std::__false_type)
       {
-	const basic_string __s(__k1, __k2);
+	// _GLIBCXX_RESOLVE_LIB_DEFECTS
+	// 2788. unintentionally require a default constructible allocator
+	const basic_string __s(__k1, __k2, this->get_allocator());
 	const size_type __n1 = __i2 - __i1;
 	return _M_replace(__i1 - begin(), __n1, __s._M_data(),
 			  __s.size());
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/lwg2788.cc b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/lwg2788.cc
new file mode 100644
index 00000000000..36dd414e887
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/lwg2788.cc
@@ -0,0 +1,85 @@
+// { dg-do run { target c++11 } }
+// { dg-require-effective-target cxx11-abi }
+
+// 2019-05-27  Nina Dinka Ranns  <dinka.ranns@gmail.com>
+//
+// Copyright (C) 2015-2019 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/>.
+
+#include <string>
+#include <testsuite_hooks.h>
+
+using C = char;
+using traits = std::char_traits<C>;
+int constructCount = 0;
+
+static void resetCounter()
+{
+ constructCount = 0;
+}
+
+template <class Tp>
+struct TestAllocator
+{
+  typedef Tp value_type;
+  using size_type = unsigned;
+
+  TestAllocator() noexcept { constructCount++; }
+
+  template <class T>
+  TestAllocator(const TestAllocator<T>&) {}
+
+  Tp *allocate(std::size_t n)
+  { return std::allocator<Tp>().allocate(n); }
+
+  void deallocate(Tp *p, std::size_t n)
+  { std::allocator<Tp>().deallocate(p, n); }
+
+};
+
+template <class T, class U>
+bool operator==(const TestAllocator<T>&, const TestAllocator<U>&)
+{ return true; }
+template <class T, class U>
+bool operator!=(const TestAllocator<T>&, const TestAllocator<U>&)
+{ return false; }
+
+void test01()
+{
+  typedef TestAllocator<C> alloc_type;
+  typedef std::basic_string<C, traits, alloc_type> test_type;
+  test_type v1{alloc_type()};
+  std::string v2{"some_content"};
+
+  resetCounter();
+  v1.assign(v2.begin(),v2.end());
+  VERIFY( constructCount == 0);
+
+  v1.append(v2.begin(),v2.end());
+  VERIFY( constructCount == 0);
+
+  v1.insert(v1.begin(),v1.begin(),v1.end());
+  VERIFY( constructCount == 0);
+
+  v1.replace(v1.begin(),v1.end(),v1.begin(),v1.end());
+  VERIFY( constructCount == 0);
+}
+int main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/lwg2788.cc b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/lwg2788.cc
new file mode 100644
index 00000000000..8e26dd7d4a9
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/lwg2788.cc
@@ -0,0 +1,85 @@
+// { dg-do run { target c++11 } }
+// { dg-require-effective-target cxx11-abi }
+
+// 2019-05-27  Nina Dinka Ranns  <dinka.ranns@gmail.com>
+//
+// Copyright (C) 2015-2019 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/>.
+
+#include <string>
+#include <testsuite_hooks.h>
+
+using C = wchar_t;
+using traits = std::char_traits<C>;
+int constructCount = 0;
+
+static void resetCounter()
+{
+ constructCount = 0;
+}
+
+template <class Tp>
+struct TestAllocator
+{
+  typedef Tp value_type;
+  using size_type = unsigned;
+
+  TestAllocator() noexcept { constructCount++; }
+
+  template <class T>
+  TestAllocator(const TestAllocator<T>&) {}
+
+  Tp *allocate(std::size_t n)
+  { return std::allocator<Tp>().allocate(n); }
+
+  void deallocate(Tp *p, std::size_t n)
+  { std::allocator<Tp>().deallocate(p, n); }
+
+};
+
+template <class T, class U>
+bool operator==(const TestAllocator<T>&, const TestAllocator<U>&)
+{ return true; }
+template <class T, class U>
+bool operator!=(const TestAllocator<T>&, const TestAllocator<U>&)
+{ return false; }
+
+void test01()
+{
+  typedef TestAllocator<C> alloc_type;
+  typedef std::basic_string<C, traits, alloc_type> test_type;
+  test_type v1{alloc_type()};
+  std::wstring v2{L"some_content"};
+
+  resetCounter();
+  v1.assign(v2.begin(),v2.end());
+  VERIFY( constructCount == 0);
+
+  v1.append(v2.begin(),v2.end());
+  VERIFY( constructCount == 0);
+
+  v1.insert(v1.begin(),v1.begin(),v1.end());
+  VERIFY( constructCount == 0);
+
+  v1.replace(v1.begin(),v1.end(),v1.begin(),v1.end());
+  VERIFY( constructCount == 0);
+}
+int main()
+{
+  test01();
+  return 0;
+}

Patch

Index: libstdc++-v3/include/bits/basic_string.tcc
===================================================================
--- libstdc++-v3/include/bits/basic_string.tcc	(revision 271509)
+++ libstdc++-v3/include/bits/basic_string.tcc	(working copy)
@@ -381,7 +381,9 @@ 
 			  _InputIterator __k1, _InputIterator __k2,
 			  std::__false_type)
       {
-	const basic_string __s(__k1, __k2);
+    // _GLIBCXX_RESOLVE_LIB_DEFECTS
+    // 2788. unintentionally require a default constructible allocator
+	const basic_string __s(__k1, __k2, this->get_allocator());
 	const size_type __n1 = __i2 - __i1;
 	return _M_replace(__i1 - begin(), __n1, __s._M_data(),
 			  __s.size());
Index: libstdc++-v3/testsuite/20_util/nonesuch/nonesuch.cc
===================================================================
--- libstdc++-v3/testsuite/20_util/nonesuch/nonesuch.cc	(revision 271509)
+++ libstdc++-v3/testsuite/20_util/nonesuch/nonesuch.cc	(nonexistent)
@@ -1,39 +0,0 @@ 
-// 2019-05-14  Nina Dinka Ranns  <dinka.ranns@gmail.com>
-// Copyright (C) 2019 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-do compile { target c++14 } }
-
-#include <type_traits>
-// Example taken from LWG2960
-
-using std::__nonesuch;
-struct such {};
-void f(const such&){};
-void f(const std::__nonesuch&);
-
-int main(){
- static_assert(!std::is_default_constructible<__nonesuch>::value,
-		 "__nonesuch is default constructible");
- static_assert(!std::is_copy_constructible<__nonesuch>::value,
-		 "__nonesuch is copy constructible");
- static_assert(!std::is_assignable<__nonesuch, __nonesuch>::value,
-		 "__nonesuch is assignable");
- static_assert(!std::is_destructible<__nonesuch>::value,
-		 "__nonesuch is destructible");
- f({});
-}
Index: libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/lwg2788.cc
===================================================================
--- libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/lwg2788.cc	(nonexistent)
+++ libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/lwg2788.cc	(working copy)
@@ -0,0 +1,84 @@ 
+//{ dg-do run { target c++11 } }
+
+// 2019-05-27  Nina Dinka Ranns  <dinka.ranns@gmail.com>
+//
+// Copyright (C) 2015-2019 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/>.
+
+#include <string>
+#include <testsuite_hooks.h>
+
+using C = char;
+using traits = std::char_traits<C>;
+int constructCount = 0;
+
+static void resetCounter()
+{
+ constructCount = 0;
+}
+
+template <class Tp>
+struct TestAllocator
+{
+  typedef Tp value_type;
+  using size_type = unsigned;
+
+  TestAllocator() noexcept { constructCount++; }
+
+  template <class T>
+  TestAllocator(const TestAllocator<T>&) {}
+
+  Tp *allocate(std::size_t n)
+  { return std::allocator<Tp>().allocate(n); }
+
+  void deallocate(Tp *p, std::size_t n)
+  { std::allocator<Tp>().deallocate(p, n); }
+
+};
+
+template <class T, class U>
+bool operator==(const TestAllocator<T>&, const TestAllocator<U>&)
+{ return true; }
+template <class T, class U>
+bool operator!=(const TestAllocator<T>&, const TestAllocator<U>&)
+{ return false; }
+
+void test01()
+{
+  typedef TestAllocator<C> alloc_type;
+  typedef std::basic_string<C, traits, alloc_type> test_type;
+  test_type v1{alloc_type()};
+  std::string v2{"some_content"};
+
+  resetCounter();
+  v1.assign(v2.begin(),v2.end());
+  VERIFY( constructCount == 0);
+
+  v1.append(v2.begin(),v2.end());
+  VERIFY( constructCount == 0);
+
+  v1.insert(v1.begin(),v1.begin(),v1.end());
+  VERIFY( constructCount == 0);
+
+  v1.replace(v1.begin(),v1.end(),v1.begin(),v1.end());
+  VERIFY( constructCount == 0);
+}
+int main()
+{
+  test01();
+  return 0;
+}
Index: libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/lwg2788.cc
===================================================================
--- libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/lwg2788.cc	(nonexistent)
+++ libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/lwg2788.cc	(working copy)
@@ -0,0 +1,84 @@ 
+//{ dg-do run { target c++11 } }
+
+// 2019-05-27  Nina Dinka Ranns  <dinka.ranns@gmail.com>
+//
+// Copyright (C) 2015-2019 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/>.
+
+#include <string>
+#include <testsuite_hooks.h>
+
+using C = wchar_t;
+using traits = std::char_traits<C>;
+int constructCount = 0;
+
+static void resetCounter()
+{
+ constructCount = 0;
+}
+
+template <class Tp>
+struct TestAllocator
+{
+  typedef Tp value_type;
+  using size_type = unsigned;
+
+  TestAllocator() noexcept { constructCount++; }
+
+  template <class T>
+  TestAllocator(const TestAllocator<T>&) {}
+
+  Tp *allocate(std::size_t n)
+  { return std::allocator<Tp>().allocate(n); }
+
+  void deallocate(Tp *p, std::size_t n)
+  { std::allocator<Tp>().deallocate(p, n); }
+
+};
+
+template <class T, class U>
+bool operator==(const TestAllocator<T>&, const TestAllocator<U>&)
+{ return true; }
+template <class T, class U>
+bool operator!=(const TestAllocator<T>&, const TestAllocator<U>&)
+{ return false; }
+
+void test01()
+{
+  typedef TestAllocator<C> alloc_type;
+  typedef std::basic_string<C, traits, alloc_type> test_type;
+  test_type v1{alloc_type()};
+  std::wstring v2{L"some_content"};
+
+  resetCounter();
+  v1.assign(v2.begin(),v2.end());
+  VERIFY( constructCount == 0);
+
+  v1.append(v2.begin(),v2.end());
+  VERIFY( constructCount == 0);
+
+  v1.insert(v1.begin(),v1.begin(),v1.end());
+  VERIFY( constructCount == 0);
+
+  v1.replace(v1.begin(),v1.end(),v1.begin(),v1.end());
+  VERIFY( constructCount == 0);
+}
+int main()
+{
+  test01();
+  return 0;
+}