[committed] libstdc++: Diagnose broken allocator rebind members

[committed] libstdc++: Diagnose broken allocator rebind members

[Jonathan Wakely]

Tested x86_64-linux. Pushed to trunk.

-- >8 --

This adds a static assertion to std::allocator_traits::rebind_alloc to
diagnose violations of the rule that rebinding an allocator to its own
value type yields the same allocator type.

This helps to catch the easy mistake of deriving from std::allocator but
forgetting to override the rebind behaviour (no longer an issue in C++20
as std::allocator doesn't have a rebind member that can be inherited).
It also catches bugs like in 23_containers/vector/52591.cc where a typo
means the rebound allocator is a completely different type.

I initially wanted to put this static assert into the body of
allocator_traits:

      static_assert(is_same<rebind_alloc<value_type>, _Alloc>::value,
		    "rebind_alloc<value_type> must be Alloc");

However, this causes a regression in the test for PR libstdc++/72792.
It seems that instantiating std::allocator_traits should be allowed for
invalid allocator types as long as you don't try to rebind them. To
support that, only assert in the __allocator_traits_base::__rebind class
template (in both the primary template and the partial specialization).
As a result, the bug in 20_util/scoped_allocator/outermost.cc is not
diagnosed, because nothing in that test rebinds the allocator.

libstdc++-v3/ChangeLog:

	* include/bits/alloc_traits.h (__allocator_traits_base::__rebind):
	Add static assert for rebind requirement.
	* testsuite/20_util/allocator_traits/members/rebind_alloc.cc:
	Fix invalid rebind member in test allocator.
	* testsuite/20_util/allocator_traits/requirements/rebind_neg.cc:
	New test.
	* testsuite/20_util/scoped_allocator/outermost.cc: Add rebind to
	test allocator.
	* testsuite/23_containers/forward_list/48101_neg.cc: Prune new
	static assert error.
	* testsuite/23_containers/unordered_multiset/48101_neg.cc:
	Likewise.
	* testsuite/23_containers/unordered_set/48101_neg.cc:
	Likewise.
	* testsuite/23_containers/vector/52591.cc: Fix typo in rebind.
---
 libstdc++-v3/include/bits/alloc_traits.h      | 17 ++++++++++++++--
 .../allocator_traits/members/rebind_alloc.cc  | 11 +++++-----
 .../requirements/rebind_neg.cc                | 20 +++++++++++++++++++
 .../20_util/scoped_allocator/outermost.cc     |  8 ++++++++
 .../23_containers/forward_list/48101_neg.cc   |  1 +
 .../unordered_multiset/48101_neg.cc           |  1 +
 .../23_containers/unordered_set/48101_neg.cc  |  1 +
 .../testsuite/23_containers/vector/52591.cc   |  2 +-
 8 files changed, 52 insertions(+), 9 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/20_util/allocator_traits/requirements/rebind_neg.cc

diff --git a/libstdc++-v3/include/bits/alloc_traits.h b/libstdc++-v3/include/bits/alloc_traits.h
index 203988ab933..6eae409ab53 100644
--- a/libstdc++-v3/include/bits/alloc_traits.h
+++ b/libstdc++-v3/include/bits/alloc_traits.h
@@ -51,12 +51,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   struct __allocator_traits_base
   {
     template<typename _Tp, typename _Up, typename = void>
-      struct __rebind : __replace_first_arg<_Tp, _Up> { };
+      struct __rebind : __replace_first_arg<_Tp, _Up>
+      {
+	static_assert(is_same<
+	  typename __replace_first_arg<_Tp, typename _Tp::value_type>::type,
+			_Tp>::value,
+	  "allocator_traits<A>::rebind_alloc<A::value_type> must be A");
+      };
 
     template<typename _Tp, typename _Up>
       struct __rebind<_Tp, _Up,
 		      __void_t<typename _Tp::template rebind<_Up>::other>>
-      { using type = typename _Tp::template rebind<_Up>::other; };
+      {
+	using type = typename _Tp::template rebind<_Up>::other;
+
+	static_assert(is_same<
+	  typename _Tp::template rebind<typename _Tp::value_type>::other,
+			_Tp>::value,
+	  "allocator_traits<A>::rebind_alloc<A::value_type> must be A");
+      };
 
   protected:
     template<typename _Tp>
diff --git a/libstdc++-v3/testsuite/20_util/allocator_traits/members/rebind_alloc.cc b/libstdc++-v3/testsuite/20_util/allocator_traits/members/rebind_alloc.cc
index ca2a8044665..dc2b1afa2d5 100644
--- a/libstdc++-v3/testsuite/20_util/allocator_traits/members/rebind_alloc.cc
+++ b/libstdc++-v3/testsuite/20_util/allocator_traits/members/rebind_alloc.cc
@@ -24,17 +24,16 @@ using std::is_same;
 template<typename T, typename U>
   using Rebind = typename std::allocator_traits<T>::template rebind_alloc<U>;
 
-#if __STDC_HOSTED__
-template<typename T>
+template<typename T, typename = T>
   struct HasRebind {
     using value_type = T;
-    template<typename U> struct rebind { using other = std::allocator<U>; };
+    template<typename U> struct rebind { using other = HasRebind<U>; };
   };
 
-static_assert(is_same<Rebind<HasRebind<int>, long>,
-		      std::allocator<long>>::value,
+// Would get HasRebind<long, int> here if the first template argument is
+// replaced instead of using the nested rebind.
+static_assert(is_same<Rebind<HasRebind<int>, long>, HasRebind<long>>::value,
 	      "nested alias template is used");
-#endif
 
 template<typename T>
   struct NoRebind0 {
diff --git a/libstdc++-v3/testsuite/20_util/allocator_traits/requirements/rebind_neg.cc b/libstdc++-v3/testsuite/20_util/allocator_traits/requirements/rebind_neg.cc
new file mode 100644
index 00000000000..a446b592edf
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/allocator_traits/requirements/rebind_neg.cc
@@ -0,0 +1,20 @@
+// { dg-do compile { target c++11 } }
+#include <vector>
+
+// Custom allocator defined with std::allocator, but doesn't provide rebind.
+template<typename T> struct Alloc : std::allocator<T> { };
+
+std::vector<int, Alloc<int>> v; // { dg-error "here" "" { target c++17_down } }
+
+// Custom allocator that does provide rebind, but incorrectly.
+template<typename T> struct Alloc2
+{
+  using value_type = T;
+  template<typename U> struct rebind { using other = Alloc<U>; }; // not Alloc2
+  T* allocate(std::size_t n) { return std::allocator<T>().allocate(n); }
+  void deallocate(T* p, std::size_t n) { std::allocator<T>().deallocate(p, n); }
+};
+
+std::vector<int, Alloc2<int>> v2; // { dg-error "here" }
+
+// { dg-error "static assertion failed: .*rebind_alloc" "" { target *-*-* } 0 }
diff --git a/libstdc++-v3/testsuite/20_util/scoped_allocator/outermost.cc b/libstdc++-v3/testsuite/20_util/scoped_allocator/outermost.cc
index 7a5d41d2472..af4d29443f5 100644
--- a/libstdc++-v3/testsuite/20_util/scoped_allocator/outermost.cc
+++ b/libstdc++-v3/testsuite/20_util/scoped_allocator/outermost.cc
@@ -49,6 +49,14 @@ struct nested_alloc : A
   template<typename U>
     nested_alloc(nested_alloc<U>) { }
 
+  // Need to customize rebind, otherwise nested_alloc<alloc<T>> gets rebound
+  // to nested_alloc<U>.
+  template<typename U>
+    struct rebind
+    {
+      using other = typename std::allocator_traits<A>::template rebind_alloc<U>;
+    };
+
   A& outer_allocator() { return *this; }
 
   template<typename U, typename... Args>
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/48101_neg.cc b/libstdc++-v3/testsuite/23_containers/forward_list/48101_neg.cc
index d15918a0e74..195d32931c1 100644
--- a/libstdc++-v3/testsuite/23_containers/forward_list/48101_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/48101_neg.cc
@@ -28,3 +28,4 @@ test01()
 // { dg-error "non-const, non-volatile value_type" "" { target *-*-* } 0 }
 // { dg-prune-output "std::allocator<.* has no member named " }
 // { dg-prune-output "must have the same value_type as its allocator" }
+// { dg-prune-output "rebind_alloc" }
diff --git a/libstdc++-v3/testsuite/23_containers/unordered_multiset/48101_neg.cc b/libstdc++-v3/testsuite/23_containers/unordered_multiset/48101_neg.cc
index 989bc4e75c5..70babc682ae 100644
--- a/libstdc++-v3/testsuite/23_containers/unordered_multiset/48101_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/unordered_multiset/48101_neg.cc
@@ -34,3 +34,4 @@ test01()
 // { dg-prune-output "use of deleted function" }
 // { dg-prune-output "must have the same value_type as its allocator" }
 // { dg-prune-output "no match for call" }
+// { dg-prune-output "rebind_alloc" }
diff --git a/libstdc++-v3/testsuite/23_containers/unordered_set/48101_neg.cc b/libstdc++-v3/testsuite/23_containers/unordered_set/48101_neg.cc
index d5b01dbfaea..30225f3d6e1 100644
--- a/libstdc++-v3/testsuite/23_containers/unordered_set/48101_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/unordered_set/48101_neg.cc
@@ -34,3 +34,4 @@ test01()
 // { dg-prune-output "use of deleted function" }
 // { dg-prune-output "must have the same value_type as its allocator" }
 // { dg-prune-output "no match for call" }
+// { dg-prune-output "rebind_alloc" }
diff --git a/libstdc++-v3/testsuite/23_containers/vector/52591.cc b/libstdc++-v3/testsuite/23_containers/vector/52591.cc
index bd4050d51e6..ea80bb277c1 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/52591.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/52591.cc
@@ -60,7 +60,7 @@ void test02()
 template<typename T>
 struct A2 : std::allocator<T>
 {
-  template<typename U> struct rebind { typedef A1<U> other; };
+  template<typename U> struct rebind { typedef A2<U> other; };
 
   A2() = default;
   template<typename U> A2(const A2<U>&) { }
-- 
2.38.1