EricWF created this revision.
EricWF added reviewers: mclow.lists, eugenis, ldionne.
EricWF added a subscriber: cfe-commits.
This patch fixes the following bugs:
* PR23256 tuple<A> is_constructible attempts tuple<A> to A conversion:
* Fix this by disabling the UTypes... overloads using the
`__disable_tuple_args_ctor<...>` meta-function.
* PR22806 Move from destroyed object in tuple_cat of nested tuples
* Fix this by disabling the tuple-like overloads using the
`__disable_tuple_like_ctor<...>` meta-function.
The goal of the `__disable_*_ctor` metafunctions is to disable obviously
incorrect constructor overloads before they evaluate more SFINAE and cause a
hard compile error. The implementation of these traits uses NO SFINAE that
could result in a hard compile error. These must be safe to evaluate always.
http://reviews.llvm.org/D12502
Files:
include/__tuple
include/tuple
test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR22806_constrain_tuple_like_ctor.pass.cpp
test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR23256_constrain_UTypes_ctor.pass.cpp
Index: test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR23256_constrain_UTypes_ctor.pass.cpp
===================================================================
--- /dev/null
+++ test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR23256_constrain_UTypes_ctor.pass.cpp
@@ -0,0 +1,60 @@
+//===----------------------------------------------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// UNSUPPORTED: c++98, c++03
+
+// <tuple>
+
+// template <class... Types> class tuple;
+
+// template <class ...UTypes>
+// EXPLICIT(...) tuple(UTypes&&...)
+
+// Check that the UTypes... ctor is properly disabled before evaluating any
+// SFINAE when the tuple-like copy/move ctor should *clearly* be selected
+// instead. This happens 'sizeof...(UTypes) == 1' and the first element of
+// 'UTypes...' is an instance of the tuple itself. See PR23256.
+
+#include <tuple>
+#include <memory>
+#include <type_traits>
+
+
+class UnconstrainedCtor {
+public:
+ UnconstrainedCtor() : value_(0) {}
+
+ template <typename T>
+ constexpr UnconstrainedCtor(T value) : value_(static_cast<int>(value)) {
+ static_assert(std::is_same<int, T>::value, "");
+ }
+
+private:
+ int value_;
+};
+
+int main() {
+ typedef UnconstrainedCtor A;
+ {
+ static_assert(std::is_trivially_copy_constructible<std::tuple<A>>::value, "");
+ static_assert(std::is_trivially_move_constructible<std::tuple<A>>::value, "");
+ }
+ {
+ static_assert(std::is_constructible<
+ std::tuple<A>,
+ std::allocator_arg_t, std::allocator<void>,
+ std::tuple<A> const&
+ >::value, "");
+ static_assert(std::is_constructible<
+ std::tuple<A>,
+ std::allocator_arg_t, std::allocator<void>,
+ std::tuple<A> &&
+ >::value, "");
+ }
+}
Index: test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR22806_constrain_tuple_like_ctor.pass.cpp
===================================================================
--- /dev/null
+++ test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR22806_constrain_tuple_like_ctor.pass.cpp
@@ -0,0 +1,74 @@
+//===----------------------------------------------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// UNSUPPORTED: c++98, c++03
+
+// <tuple>
+
+// template <class... Types> class tuple;
+
+// template <class TupleLike>
+// tuple(TupleLike&&);
+// template <class Alloc, class TupleLike>
+// tuple(std::allocator_arg_t, Alloc const&, TupleLike&&);
+
+// Check that the tuple-like copy/move ctors are properly disabled when the
+// UTypes... constructor should be selected. See PR22806.
+
+#include <tuple>
+#include <memory>
+#include <cassert>
+
+#include "tracked_value.h"
+
+int main()
+{
+ { // rvalue reference
+ TrackedValue v;
+ std::tuple<TrackedValue> t1(v);
+ // This selects the wrong constructor and constructs std::tuple<Tracked>&&
+ // from a temporary.
+ std::tuple<std::tuple<TrackedValue>&&> t2(std::move(t1));
+ // This moves from the reference constructed from the temporary.
+ std::tuple<std::tuple<TrackedValue>> t3(std::move(t2));
+ }
+ { // const lvalue reference
+ TrackedValue v;
+ std::tuple<TrackedValue> t1(v);
+ std::tuple<std::tuple<TrackedValue> const &> t2(t1);
+ std::tuple<std::tuple<TrackedValue>> t3(t2);
+ }
+ { // lvalue reference
+ TrackedValue v;
+ std::tuple<TrackedValue> t1(v);
+ std::tuple<std::tuple<TrackedValue> &> t2(t1);
+ std::tuple<std::tuple<TrackedValue>> t3(t2);
+ }
+ { // rvalue reference via allocator ctor
+ TrackedValue v;
+ std::tuple<TrackedValue> t1(v);
+ std::allocator<void> a;
+ std::tuple<std::tuple<TrackedValue>&&> t2(std::allocator_arg, a, std::move(t1));
+ std::tuple<std::tuple<TrackedValue>> t3(std::move(t2));
+ }
+ { // const lvalue reference via allocator ctor
+ TrackedValue v;
+ std::tuple<TrackedValue> t1(v);
+ std::allocator<void> a;
+ std::tuple<std::tuple<TrackedValue> const &> t2(std::allocator_arg, a, t1);
+ std::tuple<std::tuple<TrackedValue>> t3(t2);
+ }
+ { // lvalue reference via allocator ctor
+ TrackedValue v;
+ std::tuple<TrackedValue> t1(v);
+ std::allocator<void> a;
+ std::tuple<std::tuple<TrackedValue> &> t2(std::allocator_arg, a, t1);
+ std::tuple<std::tuple<TrackedValue>> t3(t2);
+ }
+}
Index: include/tuple
===================================================================
--- include/tuple
+++ include/tuple
@@ -540,6 +540,8 @@
typename enable_if
<
sizeof...(_Up) <= sizeof...(_Tp) &&
+ __lazy_and<
+ __lazy_not<__disable_tuple_args_ctor<tuple, _Up...>>,
__tuple_convertible
<
tuple<_Up...>,
@@ -547,13 +549,14 @@
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,
+ >
+ >::type::value,
bool
>::type = false
>
@@ -578,6 +581,8 @@
typename enable_if
<
sizeof...(_Up) <= sizeof...(_Tp) &&
+ __lazy_and<
+ __lazy_not<__disable_tuple_args_ctor<tuple, _Up...>>,
__tuple_constructible
<
tuple<_Up...>,
@@ -585,21 +590,21 @@
sizeof...(_Up) < sizeof...(_Tp) ?
sizeof...(_Up) :
sizeof...(_Tp)>::type
- >::value &&
- !__tuple_convertible
+ >,
+ __lazy_not<__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,
+ >>::type::value,
bool
>::type =false
>
@@ -625,6 +630,8 @@
class = typename enable_if
<
sizeof...(_Up) <= sizeof...(_Tp) &&
+ __lazy_and<
+ __lazy_not<__disable_tuple_args_ctor<tuple, _Up...>>,
__tuple_convertible
<
tuple<_Up...>,
@@ -632,13 +639,13 @@
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
+ >>::value
>::type
>
_LIBCPP_INLINE_VISIBILITY
@@ -653,6 +660,7 @@
template <class _Tuple,
typename enable_if
<
+ !__disable_tuple_like_ctor<_Tuple, tuple>::value &&
__tuple_convertible<_Tuple, tuple>::value,
bool
>::type = false
@@ -664,6 +672,7 @@
template <class _Tuple,
typename enable_if
<
+ !__disable_tuple_like_ctor<_Tuple, tuple>::value &&
__tuple_constructible<_Tuple, tuple>::value &&
!__tuple_convertible<_Tuple, tuple>::value,
bool
@@ -677,6 +686,7 @@
template <class _Alloc, class _Tuple,
class = typename enable_if
<
+ !__disable_tuple_like_ctor<_Tuple, tuple>::value &&
__tuple_convertible<_Tuple, tuple>::value
>::type
>
Index: include/__tuple
===================================================================
--- include/__tuple
+++ include/__tuple
@@ -341,6 +341,91 @@
tuple_size<_Up>::value, _Tp, _Up>
{};
+
+/**
+ * A utility metafunction that gets the first element type of a tuple
+ * and returns that type with no ref qualifiers.
+ */
+template <class _Tuple>
+struct __first_tuple_elem : remove_reference<
+ typename tuple_element<0,
+ typename remove_reference<_Tuple>::type
+ >::type
+> {};
+
+/**
+ * A metafunction that determines if the tuple-like copy/move ctor should be
+ * disabled because it would construct a dangling reference to a nested tuple.
+ * See PR22806 - https://llvm.org/bugs/show_bug.cgi?id=22806.
+ *
+ * The goal of this meta-function is to disable the tuple-like ctor for
+ * instantiations like:
+ *
+ * std::tuple<std::tuple<ValueType>&&>::tuple(std::tuple<ValueType>&&);
+ *
+ * In this case the constructor would attempt to bind 'ValueType&&' to the first
+ * element of type 'std::tuple<ValueType>&&' which would create a temporary
+ * 'std::tuple<ValueType>' from 'ValueType&&' and assign it to a reference. The
+ * temporary would then go out of scope and the tuple would contain a dangling
+ * reference.
+ *
+ * This metafunction returns true if all of the following are true.
+ * 1. _From is "tuple-like". This is always true but we need it to guard other
+ * meta-programming.
+ *
+ * 2. _Tuple has size 1. This is the only case where the copy/move ctors can
+ * be a better match that the UTypes... ctors because a single argument
+ * ctor is a better match than a variadic one that deduces one argument.
+ *
+ * 3. The first element of _Tuple, 'TupleElem0', is a reference.
+ *
+ * 4. The first element of _From, 'FromElem0', is not an instance of
+ * or derived from 'TupleElem0'. If it were then the construction of
+ * 'TupleElem0' wouldn't result in a dangling reference.
+ */
+template <class _From, class _Tuple,
+ bool = __tuple_like<typename remove_reference<_From>::type>::value &&
+ tuple_size<_Tuple>::value == 1 &&
+ is_reference<typename tuple_element<0, _Tuple>::type>::value
+ >
+struct __disable_tuple_like_ctor : false_type {};
+
+template <class _From, class _Tuple>
+struct __disable_tuple_like_ctor<_From, _Tuple, true> : integral_constant<bool,
+ tuple_size<typename remove_reference<_From>::type>::value == 1 &&
+ !is_base_of<
+ typename __first_tuple_elem<_Tuple>::type,
+ typename __first_tuple_elem<_From>::type
+ >::value
+> {};
+
+
+/**
+ * A metafunction that returns 'true' when the tuple(UTypes&&...) constructor
+ * should not participate in overload resolution. This metafunction is meant to
+ * guard the other SFINAE on the tuple(UTypes&&...) from being evaluated when
+ * its clear that a tuple copy/move constructor should be chosen instead.
+ * See PR23256 - https://llvm.org/bugs/show_bug.cgi?id=23256.
+ *
+ * The metafunction returns true if and only if:
+ * 1. sizeof...(UTypes) == 1. The copy constructor only takes 1 argument
+ * so we obviously didn't intend to call it if we provide more that one
+ * argument.
+ *
+ * 2. The first element (and only) element type, 'UT0', is the same type as the
+ * tuple. Ex std::tuple<A>(std::tuple<A> const&).
+ *
+ */
+template <class _Tuple, class ..._Args>
+struct __disable_tuple_args_ctor : false_type {};
+
+template <class _Tuple, class _Arg0>
+struct __disable_tuple_args_ctor<_Tuple, _Arg0>
+ : is_same<
+ _Tuple,
+ typename remove_cv<typename remove_reference<_Arg0>::type>::type
+ > {};
+
#endif // _LIBCPP_HAS_NO_VARIADICS
_LIBCPP_END_NAMESPACE_STD
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
