[libcxx] Fix copy/move issues caused by __tuple_leaf's converting constructor.

[EricWF]

Summary

This patch contains two fixes issues arising around __tuple_leafs converting constructor.

1. The first problem is that __tuple_leaf<T> uses is_constructible<T, U> to enable/disable it's converting constructor __tuple_leaf(U &&). Evaluating is_constructible under certain conditions can cause a hard compile error. Currently is_constructible<T, __tuple_leaf> is evaluated whenever __tuple_leaf is copied or moved. This patch prevents that from happening by only evaluating is_constructible when U != __tuple_leaf.
2. The second problem is that __tuple_leaf's specialization for empty types does have an implicitly generated move constructor because it declares a deleted assignment operator.

This causes the converting to be constructor during moves. The problem is solved by disabling the constructor when U == __tuple_leaf and giving __tuple_leaf a defaulted move constructor (so the defaulted copy constructor is not called).

There is also a small fix to two tests that needed extra brackets around an assert macro.

[EricWF]

Uploaded the correct version of the patch. Sorry for the spam.

[mclow.lists]

LGTM. Thanks!
Do you need me to commit this?

[EricWF]

Urg, "arc commit" is not playing nicely. Manually committed as r213888.

include/__tuple

 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
+// __lazy_and
+
+template <bool _Last, class ..._Preds>
+struct __lazy_and_impl;
+
+template <class ..._Preds>
+struct __lazy_and_impl<false, _Preds...> : false_type {};
+
+template <>
+struct __lazy_and_impl<true> : true_type {};
+
+template <class _Pred>
+struct __lazy_and_impl<true, _Pred> : integral_constant<bool, _Pred::type::value> {};
+
+template <class _Hp, class ..._Tp>
+struct __lazy_and_impl<true, _Hp, _Tp...> : __lazy_and_impl<_Hp::type::value, _Tp...> {};
+
+template <class _P1, class ..._Pr>
+struct __lazy_and : __lazy_and_impl<_P1::type::value, _Pr...> {};
+
+// __lazy_not
+
+template <class _Pred>
+struct __lazy_not : integral_constant<bool, !_Pred::type::value> {};
+
+
 template <class _Tp> class _LIBCPP_TYPE_VIS_ONLY tuple_size;
 
 template <class _Tp>

include/tuple

               "Attempted to default construct a reference element in a tuple");}
 
     template <class _Tp,
-              class = typename enable_if<is_constructible<_Hp, _Tp>::value>::type>
+              class = typename enable_if<
+                  __lazy_and<
+                      __lazy_not<is_same<typename decay<_Tp>::type, __tuple_leaf>>
+                    , is_constructible<_Hp, _Tp>
+                    >::value
+                >::type
+            >
         _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
         explicit __tuple_leaf(_Tp&& __t) _NOEXCEPT_((is_nothrow_constructible<_Hp, _Tp>::value))
             : value(_VSTD::forward<_Tp>(__t))

             : _Hp(__a) {}
 
     template <class _Tp,
-              class = typename enable_if<is_constructible<_Hp, _Tp>::value>::type>
+              class = typename enable_if<
+                  __lazy_and<
+                        __lazy_not<is_same<typename decay<_Tp>::type, __tuple_leaf>>
+                      , is_constructible<_Hp, _Tp>
+                    >::value
+                >::type
+            >
         _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
         explicit __tuple_leaf(_Tp&& __t) _NOEXCEPT_((is_nothrow_constructible<_Hp, _Tp>::value))
             : _Hp(_VSTD::forward<_Tp>(__t)) {}

         explicit __tuple_leaf(integral_constant<int, 2>, const _Alloc& __a, _Tp&& __t)
             : _Hp(_VSTD::forward<_Tp>(__t), __a) {}
 
+    __tuple_leaf(__tuple_leaf const &) = default;
+    __tuple_leaf(__tuple_leaf &&) = default;
+
     template <class _Tp>
         _LIBCPP_INLINE_VISIBILITY
         __tuple_leaf&

test/utilities/tuple/tuple.tuple/tuple.cnstr/const_Types.pass.cpp

 #include <string>
 #include <cassert>
 
+
+template <class ...>
+struct never {
+    enum { value = 0 };
+};
+
+struct NoValueCtor
+{
+    NoValueCtor() : id(++count) {}
+    NoValueCtor(NoValueCtor const & other) : id(other.id) { ++count; }
+
+    // The constexpr is required to make is_constructible instantiate this template.
+    // The explicit is needed to test-around a similar bug with is_convertible.
+    template <class T>
+    constexpr explicit NoValueCtor(T)
+    { static_assert(never<T>::value, "This should not be instantiated"); }
+
+    static int count;
+    int id;
+};
+
+int NoValueCtor::count = 0;
+
+
+struct NoValueCtorEmpty
+{
+    NoValueCtorEmpty() {}
+    NoValueCtorEmpty(NoValueCtorEmpty const &) {}
+
+    template <class T>
+    constexpr explicit NoValueCtorEmpty(T)
+    { static_assert(never<T>::value, "This should not be instantiated"); }
+};
+
 int main()
 {
     {

         assert(std::get<1>(t) == nullptr);
         assert(std::get<2>(t) == "text");
     }
+    // __tuple_leaf<T> uses is_constructible<T, U> to disable its explicit converting
+    // constructor overload __tuple_leaf(U &&). Evaluating is_constructible can cause a compile error.
+    // This overload is evaluated when __tuple_leafs copy or move ctor is called.
+    // This checks that is_constructible is not evaluated when U == __tuple_leaf.
+    {
+        std::tuple<int, NoValueCtor, int, int> t(1, NoValueCtor(), 2, 3);
+        assert(std::get<0>(t) == 1);
+        assert(std::get<1>(t).id == 1);
+        assert(std::get<2>(t) == 2);
+        assert(std::get<3>(t) == 3);
+    }
+    {
+        std::tuple<int, NoValueCtorEmpty, int, int> t(1, NoValueCtorEmpty(), 2, 3);
+        assert(std::get<0>(t) == 1);
+        assert(std::get<2>(t) == 2);
+        assert(std::get<3>(t) == 3);
+    }
     // extensions
     {
         std::tuple<int, char*, std::string> t(2);

test/utilities/tuple/tuple.tuple/tuple.cnstr/move.pass.cpp

 
 #include "../MoveOnly.h"
 
+struct ConstructsWithTupleLeaf
+{
+    ConstructsWithTupleLeaf() {}
+
+    ConstructsWithTupleLeaf(ConstructsWithTupleLeaf const &) { assert(false); }
+    ConstructsWithTupleLeaf(ConstructsWithTupleLeaf &&) {}
+
+    template <class T>
+    ConstructsWithTupleLeaf(T t)
+    { assert(false); }
+};
+
 int main()
 {
     {

         assert(std::get<1>(t) == 1);
         assert(std::get<2>(t) == 2);
     }
+    // A bug in tuple caused __tuple_leaf to use its explicit converting constructor
+    //  as its move constructor. This tests that ConstructsWithTupleLeaf is not called
+    // (w/ __tuple_leaf)
+    {
+        typedef std::tuple<ConstructsWithTupleLeaf> d_t;
+        d_t d((ConstructsWithTupleLeaf()));
+        d_t d2(static_cast<d_t &&>(d));
+    }
 }

test/utilities/tuple/tuple.tuple/tuple.elem/tuple.by.type1.fail.cpp

 #if _LIBCPP_STD_VER > 11
     typedef std::complex<float> cf;
     auto t1 = std::make_tuple<int, std::string> ( 42, "Hi" );
-    assert ( std::get<cf>(t1) == cf {1,2} );    // no such type
+    assert (( std::get<cf>(t1) == cf {1,2} )); // no such type
 #else
 #error
 #endif

test/utilities/utility/pairs/pair.astuple/pairs.by.type1.fail.cpp

 #if _LIBCPP_STD_VER > 11
     typedef std::complex<float> cf;
     auto t1 = std::make_pair<int, double> ( 42, 3.4 );
-    assert ( std::get<cf>(t1) == cf {1,2} );    // no such type
+    assert (( std::get<cf>(t1) == cf {1,2} ));  // no such type
 #else
 #error
 #endif