[libcxx-commits] [libcxx] [libc++] Fix `tuple_cat` for element with unconstrained constructor (PR #122433)

A. Jiang via libcxx-commits libcxx-commits at lists.llvm.org
Tue Jan 14 02:26:25 PST 2025


https://github.com/frederick-vs-ja updated https://github.com/llvm/llvm-project/pull/122433

>From 3d416003d875ad088a0ab8f04e224177cb768048 Mon Sep 17 00:00:00 2001
From: "A. Jiang" <de34 at live.cn>
Date: Fri, 10 Jan 2025 17:25:19 +0800
Subject: [PATCH 1/3] [libc++] Fix `tuple_cat` for element with unconstrained
 constructor

Currently, when the result type is 1-`tuple`, `tuple_cat` possibly tests an undesired constructor of the element, due to conversion from the reference tuple to the result type. If the element type has an unconstrained constructor template, there can be extraneous hard error which shouldn't happen.

This patch introduces a helper function template `__tuple_cat_select_element_wise` to select the element-wise constructor template of `tuple`, which can avoid such error.
---
 libcxx/include/tuple                          | 19 ++++-
 .../tuple.creation/tuple_cat.pass.cpp         | 74 ++++++++++++++++++-
 2 files changed, 89 insertions(+), 4 deletions(-)

diff --git a/libcxx/include/tuple b/libcxx/include/tuple
index aca14ba408d314..9728c8c3b99314 100644
--- a/libcxx/include/tuple
+++ b/libcxx/include/tuple
@@ -1338,12 +1338,25 @@ struct __tuple_cat<tuple<_Types...>, __tuple_indices<_I0...>, __tuple_indices<_J
   }
 };
 
+template <class _TupleDst, class _TupleSrc, size_t... _Indices>
+inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 _TupleDst
+__tuple_cat_select_element_wise(_TupleSrc&& __src, __tuple_indices<_Indices...>) {
+  static_assert(tuple_size<_TupleDst>::value == tuple_size<_TupleSrc>::value,
+                "misuse of __tuple_cat_select_element_wise with tuples of different sizes");
+  return _TupleDst(std::get<_Indices>(std::forward<_TupleSrc>(__src))...);
+}
+
 template <class _Tuple0, class... _Tuples>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 typename __tuple_cat_return<_Tuple0, _Tuples...>::type
 tuple_cat(_Tuple0&& __t0, _Tuples&&... __tpls) {
-  using _T0 _LIBCPP_NODEBUG = __libcpp_remove_reference_t<_Tuple0>;
-  return __tuple_cat<tuple<>, __tuple_indices<>, typename __make_tuple_indices<tuple_size<_T0>::value>::type>()(
-      tuple<>(), std::forward<_Tuple0>(__t0), std::forward<_Tuples>(__tpls)...);
+  using _T0 _LIBCPP_NODEBUG          = __libcpp_remove_reference_t<_Tuple0>;
+  using _TRet _LIBCPP_NODEBUG        = typename __tuple_cat_return<_Tuple0, _Tuples...>::type;
+  using _T0Indices _LIBCPP_NODEBUG   = typename __make_tuple_indices<tuple_size<_T0>::value>::type;
+  using _TRetIndices _LIBCPP_NODEBUG = typename __make_tuple_indices<tuple_size<_TRet>::value>::type;
+  return std::__tuple_cat_select_element_wise<_TRet>(
+      __tuple_cat<tuple<>, __tuple_indices<>, _T0Indice>()(
+          tuple<>(), std::forward<_Tuple0>(__t0), std::forward<_Tuples>(__tpls)...),
+      _TRetIndice());
 }
 
 template <class... _Tp, class _Alloc>
diff --git a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.creation/tuple_cat.pass.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.creation/tuple_cat.pass.cpp
index 00c9d27ccc6d05..3f978f99c2cb47 100644
--- a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.creation/tuple_cat.pass.cpp
+++ b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.creation/tuple_cat.pass.cpp
@@ -31,6 +31,70 @@ template<typename ...Ts>
 void forward_as_tuple(Ts...) = delete;
 }
 
+// https://github.com/llvm/llvm-project/issues/41034
+struct Unconstrained {
+  int data;
+  template <typename Arg>
+  TEST_CONSTEXPR_CXX14 Unconstrained(Arg arg) : data(arg) {}
+};
+
+TEST_CONSTEXPR_CXX14 std::tuple<Unconstrained> test_cat_unary_lvalue() {
+  auto tup = std::tuple<Unconstrained>(Unconstrained(5));
+  return std::tuple_cat(tup);
+}
+
+TEST_CONSTEXPR_CXX14 std::tuple<Unconstrained> test_cat_unary_rvalue() {
+  return std::tuple_cat(std::tuple<Unconstrained>(Unconstrained(6)));
+}
+
+TEST_CONSTEXPR_CXX14 std::tuple<Unconstrained> test_cat_unary_and_nullary() {
+  return std::tuple_cat(std::tuple<Unconstrained>(Unconstrained(7)), std::tuple<>());
+}
+
+#if TEST_STD_VER >= 17
+constexpr auto test_cat_unary_lvalue_ctad() {
+  auto tup = std::tuple(Unconstrained(8));
+  return std::tuple_cat(tup);
+}
+
+constexpr auto test_cat_unary_rvalue_ctad() { return std::tuple_cat(std::tuple(Unconstrained(9))); }
+
+constexpr auto test_cat_unary_and_nullary_ctad() { return std::tuple_cat(std::tuple(Unconstrained(10)), std::tuple()); }
+#endif
+
+TEST_CONSTEXPR_CXX14 bool test_tuple_cat_with_unconstrained_constructor() {
+  {
+    auto tup = test_cat_unary_lvalue();
+    assert(std::get<0>(tup).data == 5);
+  }
+  {
+    auto tup = test_cat_unary_rvalue();
+    assert(std::get<0>(tup).data == 6);
+  }
+  {
+    auto tup = test_cat_unary_and_nullary();
+    assert(std::get<0>(tup).data == 7);
+  }
+#if TEST_STD_VER >= 17
+  {
+    auto tup = test_cat_unary_lvalue_ctad();
+    ASSERT_SAME_TYPE(decltype(tup), std::tuple<Unconstrained>);
+    assert(std::get<0>(tup).data == 8);
+  }
+  {
+    auto tup = test_cat_unary_rvalue_ctad();
+    ASSERT_SAME_TYPE(decltype(tup), std::tuple<Unconstrained>);
+    assert(std::get<0>(tup).data == 9);
+  }
+  {
+    auto tup = test_cat_unary_and_nullary_ctad();
+    ASSERT_SAME_TYPE(decltype(tup), std::tuple<Unconstrained>);
+    assert(std::get<0>(tup).data == 10);
+  }
+#endif
+  return true;
+}
+
 int main(int, char**)
 {
     {
@@ -270,5 +334,13 @@ int main(int, char**)
         assert(std::get<0>(t).i == 1);
         assert(std::get<0>(t2).i == 1);
     }
-  return 0;
+    // See https://github.com/llvm/llvm-project/issues/41034
+    {
+      test_tuple_cat_with_unconstrained_constructor();
+#if TEST_STD_VER >= 14
+      static_assert(test_tuple_cat_with_unconstrained_constructor(), "");
+#endif
+    }
+
+    return 0;
 }

>From 9deb7078b2b2d9bb8a64fed1adf0dec30bad1d32 Mon Sep 17 00:00:00 2001
From: "A. Jiang" <de34 at live.cn>
Date: Fri, 10 Jan 2025 17:33:43 +0800
Subject: [PATCH 2/3] Fix copy-pasta

---
 libcxx/include/tuple | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcxx/include/tuple b/libcxx/include/tuple
index 9728c8c3b99314..0c96786ae6d027 100644
--- a/libcxx/include/tuple
+++ b/libcxx/include/tuple
@@ -1354,9 +1354,9 @@ tuple_cat(_Tuple0&& __t0, _Tuples&&... __tpls) {
   using _T0Indices _LIBCPP_NODEBUG   = typename __make_tuple_indices<tuple_size<_T0>::value>::type;
   using _TRetIndices _LIBCPP_NODEBUG = typename __make_tuple_indices<tuple_size<_TRet>::value>::type;
   return std::__tuple_cat_select_element_wise<_TRet>(
-      __tuple_cat<tuple<>, __tuple_indices<>, _T0Indice>()(
+      __tuple_cat<tuple<>, __tuple_indices<>, _T0Indices>()(
           tuple<>(), std::forward<_Tuple0>(__t0), std::forward<_Tuples>(__tpls)...),
-      _TRetIndice());
+      _TRetIndices());
 }
 
 template <class... _Tp, class _Alloc>

>From b664f225249cec1865769571602a9a251507f471 Mon Sep 17 00:00:00 2001
From: "A. Jiang" <de34 at live.cn>
Date: Tue, 14 Jan 2025 18:26:15 +0800
Subject: [PATCH 3/3] Inline test cases

---
 .../tuple.creation/tuple_cat.pass.cpp         | 38 ++++---------------
 1 file changed, 8 insertions(+), 30 deletions(-)

diff --git a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.creation/tuple_cat.pass.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.creation/tuple_cat.pass.cpp
index 3f978f99c2cb47..4b5adb7141a8a8 100644
--- a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.creation/tuple_cat.pass.cpp
+++ b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.creation/tuple_cat.pass.cpp
@@ -38,56 +38,34 @@ struct Unconstrained {
   TEST_CONSTEXPR_CXX14 Unconstrained(Arg arg) : data(arg) {}
 };
 
-TEST_CONSTEXPR_CXX14 std::tuple<Unconstrained> test_cat_unary_lvalue() {
-  auto tup = std::tuple<Unconstrained>(Unconstrained(5));
-  return std::tuple_cat(tup);
-}
-
-TEST_CONSTEXPR_CXX14 std::tuple<Unconstrained> test_cat_unary_rvalue() {
-  return std::tuple_cat(std::tuple<Unconstrained>(Unconstrained(6)));
-}
-
-TEST_CONSTEXPR_CXX14 std::tuple<Unconstrained> test_cat_unary_and_nullary() {
-  return std::tuple_cat(std::tuple<Unconstrained>(Unconstrained(7)), std::tuple<>());
-}
-
-#if TEST_STD_VER >= 17
-constexpr auto test_cat_unary_lvalue_ctad() {
-  auto tup = std::tuple(Unconstrained(8));
-  return std::tuple_cat(tup);
-}
-
-constexpr auto test_cat_unary_rvalue_ctad() { return std::tuple_cat(std::tuple(Unconstrained(9))); }
-
-constexpr auto test_cat_unary_and_nullary_ctad() { return std::tuple_cat(std::tuple(Unconstrained(10)), std::tuple()); }
-#endif
-
 TEST_CONSTEXPR_CXX14 bool test_tuple_cat_with_unconstrained_constructor() {
   {
-    auto tup = test_cat_unary_lvalue();
+    auto tup_src = std::tuple<Unconstrained>(Unconstrained(5));
+    auto tup     = std::tuple_cat(tup_src);
     assert(std::get<0>(tup).data == 5);
   }
   {
-    auto tup = test_cat_unary_rvalue();
+    auto tup = std::tuple_cat(std::tuple<Unconstrained>(Unconstrained(6)));
     assert(std::get<0>(tup).data == 6);
   }
   {
-    auto tup = test_cat_unary_and_nullary();
+    auto tup = std::tuple_cat(std::tuple<Unconstrained>(Unconstrained(7)), std::tuple<>());
     assert(std::get<0>(tup).data == 7);
   }
 #if TEST_STD_VER >= 17
   {
-    auto tup = test_cat_unary_lvalue_ctad();
+    auto tup_src = std::tuple(Unconstrained(8));
+    auto tup     = std::tuple_cat(tup_src);
     ASSERT_SAME_TYPE(decltype(tup), std::tuple<Unconstrained>);
     assert(std::get<0>(tup).data == 8);
   }
   {
-    auto tup = test_cat_unary_rvalue_ctad();
+    auto tup = std::tuple_cat(std::tuple(Unconstrained(9)));
     ASSERT_SAME_TYPE(decltype(tup), std::tuple<Unconstrained>);
     assert(std::get<0>(tup).data == 9);
   }
   {
-    auto tup = test_cat_unary_and_nullary_ctad();
+    auto tup = std::tuple_cat(std::tuple(Unconstrained(10)), std::tuple());
     ASSERT_SAME_TYPE(decltype(tup), std::tuple<Unconstrained>);
     assert(std::get<0>(tup).data == 10);
   }



More information about the libcxx-commits mailing list