[libcxx] Remove the "reduced-arity-initialization" extension from the uses-allocator constructors

Summary:
A default uses-allocator constructor has been added since that overload was previously provided by the extended constructor.

Since Clang does implicit conversion checking after substitution this constructor has to deduce the allocator_arg_t parameter so that it can prevent the evaluation of "is_default_constructible" if the first argument doesn't match. See http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1391 for more information.

This patch fixes PR24779 (https://llvm.org/bugs/show_bug.cgi?id=24779)

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D19006

llvm-svn: 266409

3 files changed, 116 insertions, 70 deletions

diff --git a/libcxx/include/tuple b/libcxx/include/tuple
index cb1e27d93cf..a39eeda60b7 100644
--- a/libcxx/include/tuple
+++ b/libcxx/include/tuple
@@ -384,6 +384,9 @@ struct __all
     : is_same<__all<_Pred...>, __all<(_Pred, true)...>>
 { };
 
+template <class ..._Tp>
+struct __lazy_all : __all<_Tp::value...> {};
+
 template <class _Tp>
 struct __all_default_constructible;
 
@@ -523,6 +526,19 @@ public:
     _LIBCPP_CONSTEXPR tuple()
         _NOEXCEPT_(__all<is_nothrow_default_constructible<_Tp>::value...>::value) {}
 
+    template <class _AllocArgT, class _Alloc, bool _Dummy = true, class = typename enable_if<
+        __lazy_and<
+            is_base_of<allocator_arg_t, _AllocArgT>,
+            __lazy_all<__dependent_type<is_default_constructible<_Tp>, _Dummy>...>
+       >::value
+    >::type>
+    _LIBCPP_INLINE_VISIBILITY
+    tuple(_AllocArgT, _Alloc const& __a)
+      : base_(allocator_arg_t(), __a,
+                    __tuple_indices<>(), __tuple_types<>(),
+                    typename __make_tuple_indices<sizeof...(_Tp), 0>::type(),
+                    __tuple_types<_Tp...>()) {}
+
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
     explicit tuple(const _Tp& ... __t) _NOEXCEPT_((__all<is_nothrow_copy_constructible<_Tp>::value...>::value)) 
         : base_(typename __make_tuple_indices<sizeof...(_Tp)>::type(),
@@ -631,21 +647,8 @@ public:
     template <class _Alloc, class ..._Up,
               class = typename enable_if
                       <
-                         sizeof...(_Up) <= sizeof...(_Tp) &&
-                         __tuple_convertible
-                         <
-                            tuple<_Up...>,
-                            typename __make_tuple_types<tuple,
-                                     sizeof...(_Up) < sizeof...(_Tp) ?
-                                        sizeof...(_Up) :
-                                        sizeof...(_Tp)>::type
-                         >::value &&
-                         __all_default_constructible<
-                            typename __make_tuple_types<tuple, sizeof...(_Tp),
-                                sizeof...(_Up) < sizeof...(_Tp) ?
-                                    sizeof...(_Up) :
-                                    sizeof...(_Tp)>::type
-                         >::value
+                         sizeof...(_Up) == sizeof...(_Tp) &&
+                         __tuple_convertible<tuple<_Up...>, tuple>::value
                       >::type
              >
         _LIBCPP_INLINE_VISIBILITY
diff --git a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/alloc.pass.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/alloc.pass.cpp
index 39776822cbd..c69163de1fd 100644
--- a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/alloc.pass.cpp
+++ b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/alloc.pass.cpp
@@ -24,12 +24,28 @@
 #include "../alloc_first.h"
 #include "../alloc_last.h"
 
+template <class T = void>
+struct NonDefaultConstructible {
+  constexpr NonDefaultConstructible() {
+      static_assert(!std::is_same<T, T>::value, "Default Ctor instantiated");
+  }
+
+  explicit constexpr NonDefaultConstructible(int) {}
+};
+
+
+struct DerivedFromAllocArgT : std::allocator_arg_t {};
+
 int main()
 {
     {
         std::tuple<> t(std::allocator_arg, A1<int>());
     }
     {
+        DerivedFromAllocArgT tag;
+        std::tuple<> t(tag, A1<int>());
+    }
+    {
         std::tuple<int> t(std::allocator_arg, A1<int>());
         assert(std::get<0>(t) == 0);
     }
@@ -78,4 +94,29 @@ int main()
         assert(!alloc_last::allocator_constructed);
         assert(std::get<2>(t) == alloc_last());
     }
+    {
+        // Test that allocator construction is selected when the user provides
+        // a custom tag type which derives from allocator_arg_t.
+        DerivedFromAllocArgT tag;
+        alloc_first::allocator_constructed = false;
+        alloc_last::allocator_constructed = false;
+
+        std::tuple<DefaultOnly, alloc_first, alloc_last> t(tag, A1<int>(5));
+
+        assert(std::get<0>(t) == DefaultOnly());
+        assert(alloc_first::allocator_constructed);
+        assert(std::get<1>(t) == alloc_first());
+        assert(alloc_last::allocator_constructed);
+        assert(std::get<2>(t) == alloc_last());
+    }
+    {
+        // Test that the uses-allocator default constructor does not evaluate
+        // it's SFINAE when it otherwise shouldn't be selected. Do this by
+        // using 'NonDefaultConstructible' which will cause a compile error
+        // if std::is_default_constructible is evaluated on it.
+        using T = NonDefaultConstructible<>;
+        T v(42);
+        std::tuple<T, T> t(v, v);
+        std::tuple<T, T> t2(42, 42);
+    }
 }
diff --git a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/alloc_UTypes.pass.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/alloc_UTypes.pass.cpp
index 3929965cd27..394af00231e 100644
--- a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/alloc_UTypes.pass.cpp
+++ b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/alloc_UTypes.pass.cpp
@@ -24,16 +24,29 @@
 #include "../alloc_first.h"
 #include "../alloc_last.h"
 
-struct NoDefault { NoDefault() = delete; };
+template <class T = void>
+struct DefaultCtorBlowsUp {
+  constexpr DefaultCtorBlowsUp() {
+      static_assert(!std::is_same<T, T>::value, "Default Ctor instantiated");
+  }
 
-// Make sure the _Up... constructor SFINAEs out when the types that
-// are not explicitly initialized are not all default constructible.
-// Otherwise, std::is_constructible would return true but instantiating
-// the constructor would fail.
-void test_default_constructible_extension_sfinae()
+  explicit constexpr DefaultCtorBlowsUp(int x) : value(x) {}
+
+  int value;
+};
+
+
+struct DerivedFromAllocArgT : std::allocator_arg_t {};
+
+
+// Make sure the _Up... constructor SFINAEs out when the number of initializers
+// is less that the number of elements in the tuple. Previously libc++ would
+// offer these constructers as an extension but they broke conforming code.
+void test_uses_allocator_sfinae_evaluation()
 {
+     using BadDefault = DefaultCtorBlowsUp<>;
     {
-        typedef std::tuple<MoveOnly, NoDefault> Tuple;
+        typedef std::tuple<MoveOnly, MoveOnly, BadDefault> Tuple;
 
         static_assert(!std::is_constructible<
             Tuple,
@@ -42,11 +55,11 @@ void test_default_constructible_extension_sfinae()
 
         static_assert(std::is_constructible<
             Tuple,
-            std::allocator_arg_t, A1<int>, MoveOnly, NoDefault
+            std::allocator_arg_t, A1<int>, MoveOnly, MoveOnly, BadDefault
         >::value, "");
     }
     {
-        typedef std::tuple<MoveOnly, MoveOnly, NoDefault> Tuple;
+        typedef std::tuple<MoveOnly, MoveOnly, BadDefault, BadDefault> Tuple;
 
         static_assert(!std::is_constructible<
             Tuple,
@@ -55,36 +68,7 @@ void test_default_constructible_extension_sfinae()
 
         static_assert(std::is_constructible<
             Tuple,
-            std::allocator_arg_t, A1<int>, MoveOnly, MoveOnly, NoDefault
-        >::value, "");
-    }
-    {
-        // Same idea as above but with a nested tuple
-        typedef std::tuple<MoveOnly, NoDefault> Tuple;
-        typedef std::tuple<MoveOnly, Tuple, MoveOnly, MoveOnly> NestedTuple;
-
-        static_assert(!std::is_constructible<
-            NestedTuple,
-            std::allocator_arg_t, A1<int>, MoveOnly, MoveOnly, MoveOnly, MoveOnly
-        >::value, "");
-
-        static_assert(std::is_constructible<
-            NestedTuple,
-            std::allocator_arg_t, A1<int>, MoveOnly, Tuple, MoveOnly, MoveOnly
-        >::value, "");
-    }
-    {
-        typedef std::tuple<MoveOnly, int> Tuple;
-        typedef std::tuple<MoveOnly, Tuple, MoveOnly, MoveOnly> NestedTuple;
-
-        static_assert(std::is_constructible<
-            NestedTuple,
-            std::allocator_arg_t, A1<int>, MoveOnly, MoveOnly, MoveOnly, MoveOnly
-        >::value, "");
-
-        static_assert(std::is_constructible<
-            NestedTuple,
-            std::allocator_arg_t, A1<int>, MoveOnly, Tuple, MoveOnly, MoveOnly
+            std::allocator_arg_t, A1<int>, MoveOnly, MoveOnly, BadDefault, BadDefault
         >::value, "");
     }
 }
@@ -96,12 +80,23 @@ int main()
         assert(std::get<0>(t) == 0);
     }
     {
+        using T = DefaultCtorBlowsUp<>;
+        std::tuple<T> t(std::allocator_arg, A1<int>(), T(42));
+        assert(std::get<0>(t).value == 42);
+    }
+    {
         std::tuple<MoveOnly, MoveOnly> t(std::allocator_arg, A1<int>(),
                                          MoveOnly(0), MoveOnly(1));
         assert(std::get<0>(t) == 0);
         assert(std::get<1>(t) == 1);
     }
     {
+        using T = DefaultCtorBlowsUp<>;
+        std::tuple<T, T> t(std::allocator_arg, A1<int>(), T(42), T(43));
+        assert(std::get<0>(t).value == 42);
+        assert(std::get<1>(t).value == 43);
+    }
+    {
         std::tuple<MoveOnly, MoveOnly, MoveOnly> t(std::allocator_arg, A1<int>(),
                                                    MoveOnly(0),
                                                    1, 2);
@@ -110,6 +105,13 @@ int main()
         assert(std::get<2>(t) == 2);
     }
     {
+        using T = DefaultCtorBlowsUp<>;
+        std::tuple<T, T, T> t(std::allocator_arg, A1<int>(), T(1), T(2), T(3));
+        assert(std::get<0>(t).value == 1);
+        assert(std::get<1>(t).value == 2);
+        assert(std::get<2>(t).value == 3);
+    }
+    {
         alloc_first::allocator_constructed = false;
         alloc_last::allocator_constructed = false;
         std::tuple<int, alloc_first, alloc_last> t(std::allocator_arg,
@@ -120,22 +122,22 @@ int main()
         assert(alloc_last::allocator_constructed);
         assert(std::get<2>(t) == alloc_last(3));
     }
-    // extensions
-    {
-        std::tuple<MoveOnly, MoveOnly, MoveOnly> t(std::allocator_arg, A1<int>(),
-                                                   0, 1);
-        assert(std::get<0>(t) == 0);
-        assert(std::get<1>(t) == 1);
-        assert(std::get<2>(t) == MoveOnly());
-    }
     {
-        std::tuple<MoveOnly, MoveOnly, MoveOnly> t(std::allocator_arg, A1<int>(),
-                                                   0);
-        assert(std::get<0>(t) == 0);
-        assert(std::get<1>(t) == MoveOnly());
-        assert(std::get<2>(t) == MoveOnly());
+        // Check that uses-allocator construction is still selected when
+        // given a tag type that derives from allocator_arg_t.
+        DerivedFromAllocArgT tag;
+        alloc_first::allocator_constructed = false;
+        alloc_last::allocator_constructed = false;
+        std::tuple<int, alloc_first, alloc_last> t(tag,
+                                                   A1<int>(5), 1, 2, 3);
+        assert(std::get<0>(t) == 1);
+        assert(alloc_first::allocator_constructed);
+        assert(std::get<1>(t) == alloc_first(2));
+        assert(alloc_last::allocator_constructed);
+        assert(std::get<2>(t) == alloc_last(3));
     }
-    // Check that SFINAE is properly applied with the default reduced arity
-    // constructor extensions.
-    test_default_constructible_extension_sfinae();
+    // Stress test the SFINAE on the uses-allocator constructors and
+    // ensure that the "reduced-arity-initialization" extension is not offered
+    // for these constructors.
+    test_uses_allocator_sfinae_evaluation();
 }