Skip to content

Conversation

philnik777
Copy link
Contributor

This fixes a bug reported in #151654 (comment).

Copy link

github-actions bot commented Aug 20, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@alexfh alexfh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the prompt fix!

@philnik777 philnik777 marked this pull request as ready for review August 21, 2025 17:44
@philnik777 philnik777 requested a review from a team as a code owner August 21, 2025 17:44
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Aug 21, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 21, 2025

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

This fixes a bug reported in #151654 (comment).


Full diff: https://github.com/llvm/llvm-project/pull/154517.diff

2 Files Affected:

  • (modified) libcxx/include/tuple (+21-19)
  • (added) libcxx/test/std/utilities/tuple/tuple.tuple/move_ctor_sfinae.compile.pass.cpp (+29)
diff --git a/libcxx/include/tuple b/libcxx/include/tuple
index b046e2aed43e2..bef718684023c 100644
--- a/libcxx/include/tuple
+++ b/libcxx/include/tuple
@@ -516,6 +516,7 @@ struct __tuple_impl;
 
 struct __forward_args {};
 struct __value_init {};
+struct __from_tuple {};
 
 template <size_t... _Indx, class... _Tp>
 struct _LIBCPP_DECLSPEC_EMPTY_BASES
@@ -538,7 +539,7 @@ struct _LIBCPP_DECLSPEC_EMPTY_BASES
       : __tuple_leaf<_Indx, _Tp>(__uses_alloc_ctor<_Tp, _Alloc, _Args>(), __alloc, std::forward<_Args>(__args))... {}
 
   template <class _Tuple>
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 __tuple_impl(_Tuple&& __t) noexcept(
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 __tuple_impl(__from_tuple, _Tuple&& __t) noexcept(
       (__all<is_nothrow_constructible<
            _Tp,
            typename tuple_element<_Indx, typename __make_tuple_types<_Tuple>::type>::type>::value...>::value))
@@ -547,7 +548,8 @@ struct _LIBCPP_DECLSPEC_EMPTY_BASES
                 std::get<_Indx>(__t)))... {}
 
   template <class _Alloc, class _Tuple>
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 __tuple_impl(allocator_arg_t, const _Alloc& __a, _Tuple&& __t)
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14
+  __tuple_impl(allocator_arg_t, const _Alloc& __a, __from_tuple, _Tuple&& __t)
       : __tuple_leaf<_Indx, _Tp>(
             __uses_alloc_ctor<_Tp,
                               _Alloc,
@@ -673,13 +675,13 @@ public:
             template <class...> class _And                                  = _And,
             __enable_if_t< _And<is_copy_constructible<_Tp>...>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 tuple(allocator_arg_t, const _Alloc& __alloc, const tuple& __t)
-      : __base_(allocator_arg_t(), __alloc, __t) {}
+      : __base_(allocator_arg_t(), __alloc, __from_tuple(), __t) {}
 
   template <class _Alloc,
             template <class...> class _And                                  = _And,
             __enable_if_t< _And<is_move_constructible<_Tp>...>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 tuple(allocator_arg_t, const _Alloc& __alloc, tuple&& __t)
-      : __base_(allocator_arg_t(), __alloc, std::move(__t)) {}
+      : __base_(allocator_arg_t(), __alloc, __from_tuple(), std::move(__t)) {}
 
   // tuple(const tuple<U...>&) constructors (including allocator_arg_t variants)
 
@@ -712,7 +714,7 @@ public:
   _LIBCPP_HIDE_FROM_ABI
   _LIBCPP_CONSTEXPR_SINCE_CXX14 explicit(_Not<_Lazy<_And, is_convertible<const _Up&, _Tp>...> >::value)
       tuple(const tuple<_Up...>& __t) noexcept(_And<is_nothrow_constructible<_Tp, const _Up&>...>::value)
-      : __base_(__t) {}
+      : __base_(__from_tuple(), __t) {}
 
   template <class... _Up,
             class _Alloc,
@@ -720,33 +722,33 @@ public:
   _LIBCPP_HIDE_FROM_ABI
   _LIBCPP_CONSTEXPR_SINCE_CXX20 explicit(_Not<_Lazy<_And, is_convertible<const _Up&, _Tp>...> >::value)
       tuple(allocator_arg_t, const _Alloc& __a, const tuple<_Up...>& __t)
-      : __base_(allocator_arg_t(), __a, __t) {}
+      : __base_(allocator_arg_t(), __a, __from_tuple(), __t) {}
 
 #    if _LIBCPP_STD_VER >= 23
   // tuple(tuple<U...>&) constructors (including allocator_arg_t variants)
 
   template <class... _Up, enable_if_t< _EnableCtorFromUTypesTuple<tuple<_Up...>&>::value>* = nullptr>
   _LIBCPP_HIDE_FROM_ABI constexpr explicit(!_Lazy<_And, is_convertible<_Up&, _Tp>...>::value) tuple(tuple<_Up...>& __t)
-      : __base_(__t) {}
+      : __base_(__from_tuple(), __t) {}
 
   template <class _Alloc, class... _Up, enable_if_t< _EnableCtorFromUTypesTuple<tuple<_Up...>&>::value>* = nullptr>
   _LIBCPP_HIDE_FROM_ABI constexpr explicit(!_Lazy<_And, is_convertible<_Up&, _Tp>...>::value)
       tuple(allocator_arg_t, const _Alloc& __alloc, tuple<_Up...>& __t)
-      : __base_(allocator_arg_t(), __alloc, __t) {}
+      : __base_(allocator_arg_t(), __alloc, __from_tuple(), __t) {}
 #    endif // _LIBCPP_STD_VER >= 23
 
   // tuple(tuple<U...>&&) constructors (including allocator_arg_t variants)
   template <class... _Up, __enable_if_t< _And< _EnableCtorFromUTypesTuple<tuple<_Up...>&&> >::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 explicit(_Not<_Lazy<_And, is_convertible<_Up, _Tp>...> >::value)
       tuple(tuple<_Up...>&& __t) noexcept(_And<is_nothrow_constructible<_Tp, _Up>...>::value)
-      : __base_(std::move(__t)) {}
+      : __base_(__from_tuple(), std::move(__t)) {}
 
   template <class _Alloc,
             class... _Up,
             __enable_if_t< _And< _EnableCtorFromUTypesTuple<tuple<_Up...>&&> >::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 explicit(_Not<_Lazy<_And, is_convertible<_Up, _Tp>...> >::value)
       tuple(allocator_arg_t, const _Alloc& __a, tuple<_Up...>&& __t)
-      : __base_(allocator_arg_t(), __a, std::move(__t)) {}
+      : __base_(allocator_arg_t(), __a, __from_tuple(), std::move(__t)) {}
 
 #    if _LIBCPP_STD_VER >= 23
   // tuple(const tuple<U...>&&) constructors (including allocator_arg_t variants)
@@ -754,14 +756,14 @@ public:
   template <class... _Up, enable_if_t< _EnableCtorFromUTypesTuple<const tuple<_Up...>&&>::value>* = nullptr>
   _LIBCPP_HIDE_FROM_ABI constexpr explicit(!_Lazy<_And, is_convertible<const _Up&&, _Tp>...>::value)
       tuple(const tuple<_Up...>&& __t)
-      : __base_(std::move(__t)) {}
+      : __base_(__from_tuple(), std::move(__t)) {}
 
   template <class _Alloc,
             class... _Up,
             enable_if_t< _EnableCtorFromUTypesTuple<const tuple<_Up...>&&>::value>* = nullptr>
   _LIBCPP_HIDE_FROM_ABI constexpr explicit(!_Lazy<_And, is_convertible<const _Up&&, _Tp>...>::value)
       tuple(allocator_arg_t, const _Alloc& __alloc, const tuple<_Up...>&& __t)
-      : __base_(allocator_arg_t(), __alloc, std::move(__t)) {}
+      : __base_(allocator_arg_t(), __alloc, __from_tuple(), std::move(__t)) {}
 #    endif // _LIBCPP_STD_VER >= 23
 
   // tuple(const pair<U1, U2>&) constructors (including allocator_arg_t variants)
@@ -796,7 +798,7 @@ public:
   _LIBCPP_HIDE_FROM_ABI
   _LIBCPP_CONSTEXPR_SINCE_CXX14 explicit(_Not<_BothImplicitlyConvertible<const pair<_Up1, _Up2>&> >::value)
       tuple(const pair<_Up1, _Up2>& __p) noexcept(_NothrowConstructibleFromPair<const pair<_Up1, _Up2>&>::value)
-      : __base_(__p) {}
+      : __base_(__from_tuple(), __p) {}
 
   template <class _Alloc,
             class _Up1,
@@ -814,7 +816,7 @@ public:
   template <class _U1, class _U2, enable_if_t< _EnableCtorFromPair<pair<_U1, _U2>&>::value>* = nullptr>
   _LIBCPP_HIDE_FROM_ABI constexpr explicit(!_BothImplicitlyConvertible<pair<_U1, _U2>&>::value)
       tuple(pair<_U1, _U2>& __p)
-      : __base_(__p) {}
+      : __base_(__from_tuple(), __p) {}
 
   template <class _Alloc,
             class _U1,
@@ -822,7 +824,7 @@ public:
             enable_if_t< _EnableCtorFromPair<std::pair<_U1, _U2>&>::value>* = nullptr>
   _LIBCPP_HIDE_FROM_ABI constexpr explicit(!_BothImplicitlyConvertible<pair<_U1, _U2>&>::value)
       tuple(allocator_arg_t, const _Alloc& __alloc, pair<_U1, _U2>& __p)
-      : __base_(allocator_arg_t(), __alloc, __p) {}
+      : __base_(allocator_arg_t(), __alloc, __from_tuple(), __p) {}
 #    endif
 
   // tuple(pair<U1, U2>&&) constructors (including allocator_arg_t variants)
@@ -834,7 +836,7 @@ public:
   _LIBCPP_HIDE_FROM_ABI
   _LIBCPP_CONSTEXPR_SINCE_CXX14 explicit(_Not<_BothImplicitlyConvertible<pair<_Up1, _Up2>&&> >::value)
       tuple(pair<_Up1, _Up2>&& __p) noexcept(_NothrowConstructibleFromPair<pair<_Up1, _Up2>&&>::value)
-      : __base_(std::move(__p)) {}
+      : __base_(__from_tuple(), std::move(__p)) {}
 
   template <class _Alloc,
             class _Up1,
@@ -844,7 +846,7 @@ public:
   _LIBCPP_HIDE_FROM_ABI
   _LIBCPP_CONSTEXPR_SINCE_CXX20 explicit(_Not<_BothImplicitlyConvertible<pair<_Up1, _Up2>&&> >::value)
       tuple(allocator_arg_t, const _Alloc& __a, pair<_Up1, _Up2>&& __p)
-      : __base_(allocator_arg_t(), __a, std::move(__p)) {}
+      : __base_(allocator_arg_t(), __a, __from_tuple(), std::move(__p)) {}
 
 #    if _LIBCPP_STD_VER >= 23
   // tuple(const pair<U1, U2>&&) constructors (including allocator_arg_t variants)
@@ -852,7 +854,7 @@ public:
   template <class _U1, class _U2, enable_if_t< _EnableCtorFromPair<const pair<_U1, _U2>&&>::value>* = nullptr>
   _LIBCPP_HIDE_FROM_ABI constexpr explicit(!_BothImplicitlyConvertible<const pair<_U1, _U2>&&>::value)
       tuple(const pair<_U1, _U2>&& __p)
-      : __base_(std::move(__p)) {}
+      : __base_(__from_tuple(), std::move(__p)) {}
 
   template <class _Alloc,
             class _U1,
@@ -860,7 +862,7 @@ public:
             enable_if_t< _EnableCtorFromPair<const pair<_U1, _U2>&&>::value>* = nullptr>
   _LIBCPP_HIDE_FROM_ABI constexpr explicit(!_BothImplicitlyConvertible<const pair<_U1, _U2>&&>::value)
       tuple(allocator_arg_t, const _Alloc& __alloc, const pair<_U1, _U2>&& __p)
-      : __base_(allocator_arg_t(), __alloc, std::move(__p)) {}
+      : __base_(allocator_arg_t(), __alloc, __from_tuple(), std::move(__p)) {}
 #    endif // _LIBCPP_STD_VER >= 23
 
   // [tuple.assign]
diff --git a/libcxx/test/std/utilities/tuple/tuple.tuple/move_ctor_sfinae.compile.pass.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/move_ctor_sfinae.compile.pass.cpp
new file mode 100644
index 0000000000000..6eeaae9fb5c8f
--- /dev/null
+++ b/libcxx/test/std/utilities/tuple/tuple.tuple/move_ctor_sfinae.compile.pass.cpp
@@ -0,0 +1,29 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// Ensure that tuple's move constructor properly SFINAES.
+// This is a regression test for https://github.com/llvm/llvm-project/pull/151654#issuecomment-3205410955
+
+// UNSUPPORTED: c++03
+
+#include <tuple>
+#include <variant>
+#include <type_traits>
+
+struct S {
+    S(const S&) = delete;
+    S& operator=(const S&) = delete;
+    S(S&&) = default;
+    S& operator=(S&&) = default;
+};
+
+using T = std::tuple<const std::variant<S>>;
+
+void func() {
+  (void)std::is_trivially_move_constructible<T>::value;
+}

@alexfh
Copy link
Contributor

alexfh commented Aug 21, 2025

Please ensure premerge checks pass (now there are complaints from clang-format and test failures).

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
View the diff from clang-format here.

diff --git a/libcxx/test/std/utilities/tuple/tuple.tuple/move_ctor_sfinae.compile.pass.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/move_ctor_sfinae.compile.pass.cpp
index 6eeaae9fb..27306de8e 100644
--- a/libcxx/test/std/utilities/tuple/tuple.tuple/move_ctor_sfinae.compile.pass.cpp
+++ b/libcxx/test/std/utilities/tuple/tuple.tuple/move_ctor_sfinae.compile.pass.cpp
@@ -16,14 +16,12 @@
 #include <type_traits>
 
 struct S {
-    S(const S&) = delete;
-    S& operator=(const S&) = delete;
-    S(S&&) = default;
-    S& operator=(S&&) = default;
+  S(const S&)            = delete;
+  S& operator=(const S&) = delete;
+  S(S&&)                 = default;
+  S& operator=(S&&)      = default;
 };
 
 using T = std::tuple<const std::variant<S>>;
 
-void func() {
-  (void)std::is_trivially_move_constructible<T>::value;
-}
+void func() { (void)std::is_trivially_move_constructible<T>::value; }

@philnik777 philnik777 force-pushed the tuple_add_tags branch 2 times, most recently from b44eba9 to 365420c Compare August 22, 2025 09:40
@philnik777 philnik777 merged commit d9a192c into llvm:main Aug 23, 2025
73 of 78 checks passed
@philnik777 philnik777 deleted the tuple_add_tags branch August 23, 2025 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants