diff mbox

Constrain allocator_arg_t to only work with valid Allocators

Message ID 20150630144942.GC2856@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely June 30, 2015, 2:49 p.m. UTC 
This is what I have been experimenting with as an alternative to
making allocator_arg_t non-DefaultConstructible.

By replacing allocator_arg_t parameters with __alloc_arg_t<_Alloc> we
can constrain constructors to only participate in overload resolution
when _Alloc quacks sufficiently like an allocator.

This ensures that tuple<T,U>{ {} , U{} } is unambiguous (unless U
happens to be an allocator, or a reasonable forgery of one).

Making this change required fixing some tests where I was passing
fake allocator arguments to tuples, which violated the preconditions
before but was accepted. Rejecting those precondition violations at
compile-time suggests to me that this is a nice improvement. The
downside is that it probably hurts compilation times even more, as
every change related to allocators seems to.

Any comments or suggestions about this approach?

I'm also playing with another change to make allocator_traits<A>
SFINAE-friendly, by only defining the nested allocator_type member
when __is_allocator<A> is true. If it works I think that might be
worth standardising.
diff mbox

Patch

commit ea134c1ada93bbeca9f09a06513704639a752d39
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Jun 29 17:08:12 2015 +0100

    	* include/bits/uses_allocator.h (__uses_alloc_t, __use_alloc): Add
    	comments.
    	(__is_allocator): New trait.
    	(__alloc_arg_t): New SFINAE helper for detecting allocators.
    	* include/std/tuple (tuple): Replace allocator_arg_t with
    	__alloc_arg_t<_Alloc>.
    	* testsuite/20_util/tuple/cons/allocators.cc: Use a valid Allocator.
    	* testsuite/20_util/uses_allocator/cons_neg.cc: Likewise.
    	* testsuite/20_util/uses_allocator/construction.cc: Likewise.

diff --git a/libstdc++-v3/include/bits/uses_allocator.h b/libstdc++-v3/include/bits/uses_allocator.h
index f9ea7d6..293a315 100644
--- a/libstdc++-v3/include/bits/uses_allocator.h
+++ b/libstdc++-v3/include/bits/uses_allocator.h
@@ -38,6 +38,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   /// [allocator.tag]
   struct allocator_arg_t { };
 
+  /// A tag of type allocator_arg_t.
   constexpr allocator_arg_t allocator_arg = allocator_arg_t();
 
   template<typename _Tp, typename _Alloc, typename = __void_t<>>
@@ -58,14 +59,17 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   struct __uses_alloc_base { };
 
+  /// A tag type indicating construction without an allocator.
   struct __uses_alloc0 : __uses_alloc_base
   {
     struct _Sink { void operator=(const void*) { } } _M_a;
   };
 
+  /// A tag type indicating construction with allocator_arg_t.
   template<typename _Alloc>
     struct __uses_alloc1 : __uses_alloc_base { const _Alloc* _M_a; };
 
+  /// A tag type indicating construction with an allocator argument at the end.
   template<typename _Alloc>
     struct __uses_alloc2 : __uses_alloc_base { const _Alloc* _M_a; };
 
@@ -84,10 +88,12 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     struct __uses_alloc<false, _Tp, _Alloc, _Args...>
     : __uses_alloc0 { };
 
+  /// A tag type indicating whether/how to construct with an allocator.
   template<typename _Tp, typename _Alloc, typename... _Args>
     using __uses_alloc_t =
       __uses_alloc<uses_allocator<_Tp, _Alloc>::value, _Tp, _Alloc, _Args...>;
 
+  /// Make a tag type indicating how to use an allocator for construction.
   template<typename _Tp, typename _Alloc, typename... _Args>
     inline __uses_alloc_t<_Tp, _Alloc, _Args...>
     __use_alloc(const _Alloc& __a)
@@ -97,6 +103,34 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       return __ret;
     }
 
+  /// Check for required Allocator functions (only as an unevaluated operand).
+  template<typename _Alloc>
+    auto
+    __can_allocate(_Alloc* __a)
+    -> decltype(__a->deallocate(__a->allocate(1u), 1u));
+
+  /// Primary template handles all cases that don't look like Allocators.
+  template<typename _Alloc, typename = __void_t<>>
+    struct __is_allocator_impl
+    : false_type { };
+
+  /// Specialization recognizes types that define value_type and can allocate.
+  template<typename _Alloc>
+    struct __is_allocator_impl<_Alloc,
+			       __void_t<typename _Alloc::value_type,
+				        decltype(__can_allocate<_Alloc>(0))>>
+    : true_type { };
+
+  /// Detect whether a type might be an Allocator.
+  template<typename _Alloc>
+    struct __is_allocator : __is_allocator_impl<_Alloc>::type
+    { };
+
+  /// Alias for allocator_arg_t that is only valid if _Alloc is an Allocator.
+  template<typename _Alloc>
+    using __alloc_arg_t = typename enable_if<__is_allocator<_Alloc>::value,
+					     allocator_arg_t>::type;
+
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace std
 
diff --git a/libstdc++-v3/include/std/tuple b/libstdc++-v3/include/std/tuple
index 59b992a..a765bc8 100644
--- a/libstdc++-v3/include/std/tuple
+++ b/libstdc++-v3/include/std/tuple
@@ -640,7 +640,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       // Allocator-extended constructors.
 
       template<typename _Alloc>
-	tuple(allocator_arg_t __tag, const _Alloc& __a)
+	tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a)
 	: _Inherited(__tag, __a) { }
 
       template<typename _Alloc, typename _Dummy = void,
@@ -650,7 +650,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                  && _TCC<_Dummy>::template
                    _ImplicitlyConvertibleTuple<_Elements...>(),
                bool>::type=true>
-	tuple(allocator_arg_t __tag, const _Alloc& __a,
+	tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
 	      const _Elements&... __elements)
 	: _Inherited(__tag, __a, __elements...) { }
 
@@ -661,7 +661,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                  && !_TCC<_Dummy>::template
                    _ImplicitlyConvertibleTuple<_Elements...>(),
                bool>::type=false>
-	explicit tuple(allocator_arg_t __tag, const _Alloc& __a,
+	explicit tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
                        const _Elements&... __elements)
 	: _Inherited(__tag, __a, __elements...) { }
 
@@ -671,7 +671,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                   && _TMC<_UElements...>::template
                     _ImplicitlyMoveConvertibleTuple<_UElements...>(),
         bool>::type=true>
-	tuple(allocator_arg_t __tag, const _Alloc& __a,
+	tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
 	      _UElements&&... __elements)
 	: _Inherited(__tag, __a, std::forward<_UElements>(__elements)...)
        	{ }
@@ -682,17 +682,17 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                   && !_TMC<_UElements...>::template
                     _ImplicitlyMoveConvertibleTuple<_UElements...>(),
         bool>::type=false>
-	explicit tuple(allocator_arg_t __tag, const _Alloc& __a,
+	explicit tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
 	      _UElements&&... __elements)
 	: _Inherited(__tag, __a, std::forward<_UElements>(__elements)...)
         { }
 
       template<typename _Alloc>
-	tuple(allocator_arg_t __tag, const _Alloc& __a, const tuple& __in)
+	tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a, const tuple& __in)
 	: _Inherited(__tag, __a, static_cast<const _Inherited&>(__in)) { }
 
       template<typename _Alloc>
-	tuple(allocator_arg_t __tag, const _Alloc& __a, tuple&& __in)
+	tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a, tuple&& __in)
 	: _Inherited(__tag, __a, static_cast<_Inherited&&>(__in)) { }
 
       template<typename _Alloc, typename... _UElements, typename
@@ -701,7 +701,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                   && _TMC<_UElements...>::template
                     _ImplicitlyConvertibleTuple<_UElements...>(),
         bool>::type=true>
-	tuple(allocator_arg_t __tag, const _Alloc& __a,
+	tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
 	      const tuple<_UElements...>& __in)
 	: _Inherited(__tag, __a,
 	             static_cast<const _Tuple_impl<0, _UElements...>&>(__in))
@@ -713,7 +713,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                   && !_TMC<_UElements...>::template
                     _ImplicitlyConvertibleTuple<_UElements...>(),
         bool>::type=false>
-	explicit tuple(allocator_arg_t __tag, const _Alloc& __a,
+	explicit tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
 	      const tuple<_UElements...>& __in)
 	: _Inherited(__tag, __a,
 	             static_cast<const _Tuple_impl<0, _UElements...>&>(__in))
@@ -725,7 +725,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                   && _TMC<_UElements...>::template
                     _ImplicitlyMoveConvertibleTuple<_UElements...>(),
         bool>::type=true>
-	tuple(allocator_arg_t __tag, const _Alloc& __a,
+	tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
 	      tuple<_UElements...>&& __in)
 	: _Inherited(__tag, __a,
 	             static_cast<_Tuple_impl<0, _UElements...>&&>(__in))
@@ -737,7 +737,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                   && !_TMC<_UElements...>::template
                     _ImplicitlyMoveConvertibleTuple<_UElements...>(),
         bool>::type=false>
-	explicit tuple(allocator_arg_t __tag, const _Alloc& __a,
+	explicit tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
 	      tuple<_UElements...>&& __in)
 	: _Inherited(__tag, __a,
 	             static_cast<_Tuple_impl<0, _UElements...>&&>(__in))
@@ -936,7 +936,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       // Allocator-extended constructors.
 
       template<typename _Alloc>
-	tuple(allocator_arg_t __tag, const _Alloc& __a)
+	tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a)
 	: _Inherited(__tag, __a) { }
 
       template<typename _Alloc, typename _Dummy = void,
@@ -947,7 +947,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                    _ImplicitlyConvertibleTuple<_T1, _T2>(),
                bool>::type=true>
 
-	tuple(allocator_arg_t __tag, const _Alloc& __a,
+	tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
 	      const _T1& __a1, const _T2& __a2)
 	: _Inherited(__tag, __a, __a1, __a2) { }
 
@@ -959,7 +959,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                    _ImplicitlyConvertibleTuple<_T1, _T2>(),
                bool>::type=false>
 
-	explicit tuple(allocator_arg_t __tag, const _Alloc& __a,
+	explicit tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
 	      const _T1& __a1, const _T2& __a2)
 	: _Inherited(__tag, __a, __a1, __a2) { }
 
@@ -969,7 +969,8 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                   && _TMC::template
                     _ImplicitlyMoveConvertibleTuple<_U1, _U2>(),
 	bool>::type = true>
-	tuple(allocator_arg_t __tag, const _Alloc& __a, _U1&& __a1, _U2&& __a2)
+	tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a, _U1&& __a1,
+	      _U2&& __a2)
 	: _Inherited(__tag, __a, std::forward<_U1>(__a1),
 	             std::forward<_U2>(__a2)) { }
 
@@ -979,17 +980,17 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                   && !_TMC::template
                     _ImplicitlyMoveConvertibleTuple<_U1, _U2>(),
 	bool>::type = false>
-	explicit tuple(allocator_arg_t __tag, const _Alloc& __a,
+	explicit tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
                        _U1&& __a1, _U2&& __a2)
 	: _Inherited(__tag, __a, std::forward<_U1>(__a1),
 	             std::forward<_U2>(__a2)) { }
 
       template<typename _Alloc>
-	tuple(allocator_arg_t __tag, const _Alloc& __a, const tuple& __in)
+	tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a, const tuple& __in)
 	: _Inherited(__tag, __a, static_cast<const _Inherited&>(__in)) { }
 
       template<typename _Alloc>
-	tuple(allocator_arg_t __tag, const _Alloc& __a, tuple&& __in)
+	tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a, tuple&& __in)
 	: _Inherited(__tag, __a, static_cast<_Inherited&&>(__in)) { }
 
       template<typename _Alloc, typename _U1, typename _U2, typename
@@ -998,7 +999,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                   && _TMC::template
                     _ImplicitlyConvertibleTuple<_U1, _U2>(),
 	bool>::type = true>
-	tuple(allocator_arg_t __tag, const _Alloc& __a,
+	tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
 	      const tuple<_U1, _U2>& __in)
 	: _Inherited(__tag, __a,
 	             static_cast<const _Tuple_impl<0, _U1, _U2>&>(__in))
@@ -1010,7 +1011,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                   && !_TMC::template
                     _ImplicitlyConvertibleTuple<_U1, _U2>(),
 	bool>::type = false>
-	explicit tuple(allocator_arg_t __tag, const _Alloc& __a,
+	explicit tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
 	      const tuple<_U1, _U2>& __in)
 	: _Inherited(__tag, __a,
 	             static_cast<const _Tuple_impl<0, _U1, _U2>&>(__in))
@@ -1022,7 +1023,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                   && _TMC::template
                     _ImplicitlyMoveConvertibleTuple<_U1, _U2>(),
 	bool>::type = true>
-	tuple(allocator_arg_t __tag, const _Alloc& __a, tuple<_U1, _U2>&& __in)
+	tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a, tuple<_U1, _U2>&& __in)
 	: _Inherited(__tag, __a, static_cast<_Tuple_impl<0, _U1, _U2>&&>(__in))
 	{ }
 
@@ -1032,7 +1033,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                   && !_TMC::template
                     _ImplicitlyMoveConvertibleTuple<_U1, _U2>(),
 	bool>::type = false>
-	explicit tuple(allocator_arg_t __tag, const _Alloc& __a,
+	explicit tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
                        tuple<_U1, _U2>&& __in)
 	: _Inherited(__tag, __a, static_cast<_Tuple_impl<0, _U1, _U2>&&>(__in))
 	{ }
@@ -1043,7 +1044,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                   && _TMC::template
                     _ImplicitlyConvertibleTuple<_U1, _U2>(),
 	bool>::type = true>
-        tuple(allocator_arg_t __tag, const _Alloc& __a,
+        tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
 	      const pair<_U1, _U2>& __in)
 	: _Inherited(__tag, __a, __in.first, __in.second) { }
 
@@ -1053,7 +1054,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                   && !_TMC::template
                     _ImplicitlyConvertibleTuple<_U1, _U2>(),
 	bool>::type = false>
-        explicit tuple(allocator_arg_t __tag, const _Alloc& __a,
+        explicit tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
 	      const pair<_U1, _U2>& __in)
 	: _Inherited(__tag, __a, __in.first, __in.second) { }
 
@@ -1063,7 +1064,8 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                   && _TMC::template
                     _ImplicitlyMoveConvertibleTuple<_U1, _U2>(),
 	bool>::type = true>
-        tuple(allocator_arg_t __tag, const _Alloc& __a, pair<_U1, _U2>&& __in)
+        tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
+	      pair<_U1, _U2>&& __in)
 	: _Inherited(__tag, __a, std::forward<_U1>(__in.first),
 		     std::forward<_U2>(__in.second)) { }
 
@@ -1073,7 +1075,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
                   && !_TMC::template
                     _ImplicitlyMoveConvertibleTuple<_U1, _U2>(),
 	bool>::type = false>
-        explicit tuple(allocator_arg_t __tag, const _Alloc& __a,
+        explicit tuple(__alloc_arg_t<_Alloc> __tag, const _Alloc& __a,
                        pair<_U1, _U2>&& __in)
 	: _Inherited(__tag, __a, std::forward<_U1>(__in.first),
 		     std::forward<_U2>(__in.second)) { }
diff --git a/libstdc++-v3/testsuite/20_util/tuple/cons/allocators.cc b/libstdc++-v3/testsuite/20_util/tuple/cons/allocators.cc
index 8db7e11..566f2fc 100644
--- a/libstdc++-v3/testsuite/20_util/tuple/cons/allocators.cc
+++ b/libstdc++-v3/testsuite/20_util/tuple/cons/allocators.cc
@@ -22,8 +22,9 @@ 
 #include <memory>
 #include <tuple>
 #include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
 
-struct MyAlloc { };
+using MyAlloc = __gnu_test::SimpleAllocator<int>;
 
 // type that can't be constructed with an allocator
 struct CannotUse
diff --git a/libstdc++-v3/testsuite/20_util/uses_allocator/cons_neg.cc b/libstdc++-v3/testsuite/20_util/uses_allocator/cons_neg.cc
index 5027893..3e5e63a 100644
--- a/libstdc++-v3/testsuite/20_util/uses_allocator/cons_neg.cc
+++ b/libstdc++-v3/testsuite/20_util/uses_allocator/cons_neg.cc
@@ -22,8 +22,9 @@ 
 
 #include <memory>
 #include <tuple>
+#include <testsuite_allocator.h>
 
-struct MyAlloc { };
+using MyAlloc = __gnu_test::SimpleAllocator<int>;
 
 struct Type
 {
diff --git a/libstdc++-v3/testsuite/20_util/uses_allocator/construction.cc b/libstdc++-v3/testsuite/20_util/uses_allocator/construction.cc
index 0094a1e..af2c54e 100644
--- a/libstdc++-v3/testsuite/20_util/uses_allocator/construction.cc
+++ b/libstdc++-v3/testsuite/20_util/uses_allocator/construction.cc
@@ -22,8 +22,9 @@ 
 #include <memory>
 #include <tuple>
 #include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
 
-struct MyAlloc { };
+using MyAlloc = __gnu_test::SimpleAllocator<int>;
 
 // type that can't be constructed with an allocator
 struct CannotUse