[libc++] Remove SFINAE on __tuple_impl constructors #151654
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
philnik777
force-pushed
the
simplify_tuple
branch
2 times, most recently
from
b749b78 to
d8b4b28
Compare
philnik777
force-pushed
the
simplify_tuple
branch
from
d8b4b28 to
26bbd44
Compare
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) ChangesThe SFINAE isn't required, since the primary This also moves the Full diff: https://github.com/llvm/llvm-project/pull/151654.diff 7 Files Affected:
diff --git a/libcxx/include/__fwd/tuple.h b/libcxx/include/__fwd/tuple.h
index fb922b29f3d3f..39ed94d9806e5 100644
--- a/libcxx/include/__fwd/tuple.h
+++ b/libcxx/include/__fwd/tuple.h
@@ -26,6 +26,11 @@ struct tuple_element;
template <class...>
class tuple;
+template <size_t _Ip, class... _Tp>
+struct tuple_element<_Ip, tuple<_Tp...> > {
+ using type _LIBCPP_NODEBUG = __type_pack_element<_Ip, _Tp...>;
+};
+
template <class>
struct tuple_size;
diff --git a/libcxx/include/__memory/uses_allocator_construction.h b/libcxx/include/__memory/uses_allocator_construction.h
index 49ddf99d9cc95..6733f5cf6fc35 100644
--- a/libcxx/include/__memory/uses_allocator_construction.h
+++ b/libcxx/include/__memory/uses_allocator_construction.h
@@ -17,6 +17,7 @@
#include <__type_traits/remove_cv.h>
#include <__utility/declval.h>
#include <__utility/pair.h>
+#include <__utility/piecewise_construct.h>
#include <tuple>
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
diff --git a/libcxx/include/__memory_resource/polymorphic_allocator.h b/libcxx/include/__memory_resource/polymorphic_allocator.h
index 6e7a9afc25deb..9a351199b5b16 100644
--- a/libcxx/include/__memory_resource/polymorphic_allocator.h
+++ b/libcxx/include/__memory_resource/polymorphic_allocator.h
@@ -18,6 +18,7 @@
#include <__new/exceptions.h>
#include <__new/placement_new_delete.h>
#include <__utility/exception_guard.h>
+#include <__utility/piecewise_construct.h>
#include <limits>
#include <tuple>
diff --git a/libcxx/include/__tuple/sfinae_helpers.h b/libcxx/include/__tuple/sfinae_helpers.h
index 9fe5e84e2f3ca..f314381d0a48d 100644
--- a/libcxx/include/__tuple/sfinae_helpers.h
+++ b/libcxx/include/__tuple/sfinae_helpers.h
@@ -10,20 +10,6 @@
#define _LIBCPP___TUPLE_SFINAE_HELPERS_H
#include <__config>
-#include <__cstddef/size_t.h>
-#include <__fwd/tuple.h>
-#include <__tuple/make_tuple_types.h>
-#include <__tuple/tuple_element.h>
-#include <__tuple/tuple_like_ext.h>
-#include <__tuple/tuple_size.h>
-#include <__tuple/tuple_types.h>
-#include <__type_traits/conjunction.h>
-#include <__type_traits/enable_if.h>
-#include <__type_traits/integral_constant.h>
-#include <__type_traits/is_constructible.h>
-#include <__type_traits/is_same.h>
-#include <__type_traits/remove_cvref.h>
-#include <__type_traits/remove_reference.h>
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
@@ -33,35 +19,6 @@ _LIBCPP_BEGIN_NAMESPACE_STD
#ifndef _LIBCPP_CXX03_LANG
-struct __tuple_sfinae_base {
- template <template <class, class...> class _Trait, class... _LArgs, class... _RArgs>
- static auto __do_test(__tuple_types<_LArgs...>,
- __tuple_types<_RArgs...>) -> __all<__enable_if_t<_Trait<_LArgs, _RArgs>::value, bool>{true}...>;
- template <template <class...> class>
- static auto __do_test(...) -> false_type;
-
- template <class _FromArgs, class _ToArgs>
- using __constructible _LIBCPP_NODEBUG = decltype(__do_test<is_constructible>(_ToArgs{}, _FromArgs{}));
-};
-
-// __tuple_constructible
-
-template <class _Tp,
- class _Up,
- bool = __tuple_like_ext<__libcpp_remove_reference_t<_Tp> >::value,
- bool = __tuple_like_ext<_Up>::value>
-struct __tuple_constructible : public false_type {};
-
-template <class _Tp, class _Up>
-struct __tuple_constructible<_Tp, _Up, true, true>
- : public __tuple_sfinae_base::__constructible< typename __make_tuple_types<_Tp>::type,
- typename __make_tuple_types<_Up>::type > {};
-
-template <size_t _Ip, class... _Tp>
-struct tuple_element<_Ip, tuple<_Tp...> > {
- using type _LIBCPP_NODEBUG = typename tuple_element<_Ip, __tuple_types<_Tp...> >::type;
-};
-
struct _LIBCPP_EXPORTED_FROM_ABI __check_tuple_constructor_fail {
static _LIBCPP_HIDE_FROM_ABI constexpr bool __enable_explicit_default() { return false; }
static _LIBCPP_HIDE_FROM_ABI constexpr bool __enable_implicit_default() { return false; }
diff --git a/libcxx/include/module.modulemap.in b/libcxx/include/module.modulemap.in
index 5857a83b5fe14..b07a153eacfae 100644
--- a/libcxx/include/module.modulemap.in
+++ b/libcxx/include/module.modulemap.in
@@ -2168,7 +2168,10 @@ module std [system] {
module is_valid_range { header "__utility/is_valid_range.h" }
module move { header "__utility/move.h" }
module no_destroy { header "__utility/no_destroy.h" }
- module pair { header "__utility/pair.h" }
+ module pair {
+ header "__utility/pair.h"
+ export std.utility.piecewise_construct
+ }
module piecewise_construct { header "__utility/piecewise_construct.h" }
module priority_tag { header "__utility/priority_tag.h" }
module private_constructor_tag { header "__utility/private_constructor_tag.h" }
diff --git a/libcxx/include/tuple b/libcxx/include/tuple
index 1946034aeecbe..b854d615f7192 100644
--- a/libcxx/include/tuple
+++ b/libcxx/include/tuple
@@ -227,7 +227,6 @@ template <class... Types>
# include <__tuple/find_index.h>
# include <__tuple/ignore.h>
# include <__tuple/make_tuple_types.h>
-# include <__tuple/sfinae_helpers.h>
# include <__tuple/tuple_element.h>
# include <__tuple/tuple_like_ext.h>
# include <__tuple/tuple_size.h>
@@ -240,7 +239,6 @@ template <class... Types>
# include <__type_traits/disjunction.h>
# include <__type_traits/enable_if.h>
# include <__type_traits/invoke.h>
-# include <__type_traits/is_arithmetic.h>
# include <__type_traits/is_assignable.h>
# include <__type_traits/is_constructible.h>
# include <__type_traits/is_convertible.h>
@@ -267,7 +265,6 @@ template <class... Types>
# include <__utility/forward.h>
# include <__utility/integer_sequence.h>
# include <__utility/move.h>
-# include <__utility/piecewise_construct.h>
# include <__utility/swap.h>
# include <version>
@@ -471,7 +468,7 @@ struct _LIBCPP_DECLSPEC_EMPTY_BASES
allocator_arg_t, const _Alloc& __alloc, __forward_args, _Args&&... __args)
: __tuple_leaf<_Indx, _Tp>(__uses_alloc_ctor<_Tp, _Alloc, _Args>(), __alloc, std::forward<_Args>(__args))... {}
- template <class _Tuple, __enable_if_t<__tuple_constructible<_Tuple, tuple<_Tp...> >::value, int> = 0>
+ template <class _Tuple>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 __tuple_impl(_Tuple&& __t) noexcept(
(__all<is_nothrow_constructible<
_Tp,
@@ -480,7 +477,7 @@ struct _LIBCPP_DECLSPEC_EMPTY_BASES
std::forward<typename tuple_element<_Indx, typename __make_tuple_types<_Tuple>::type>::type>(
std::get<_Indx>(__t)))... {}
- template <class _Alloc, class _Tuple, __enable_if_t<__tuple_constructible<_Tuple, tuple<_Tp...> >::value, int> = 0>
+ template <class _Alloc, class _Tuple>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 __tuple_impl(allocator_arg_t, const _Alloc& __a, _Tuple&& __t)
: __tuple_leaf<_Indx, _Tp>(
__uses_alloc_ctor<_Tp,
diff --git a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.verify.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.verify.cpp
index 2edeeefde0b8f..8a8b049275d7f 100644
--- a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.verify.cpp
+++ b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.verify.cpp
@@ -23,4 +23,4 @@
using T = std::tuple<int, long, void*>;
using E1 = typename std::tuple_element<1, T &>::type; // expected-error{{undefined template}}
using E2 = typename std::tuple_element<3, T>::type;
-using E3 = typename std::tuple_element<4, T const>::type; // expected-error@*:* 2 {{static assertion failed}}
+using E3 = typename std::tuple_element<4, T const>::type; // expected-error@*:* 2 {{out of bounds index}}
|
|
Hi @philnik777, The code above started generating an error and causing a Clang crash: |
|
@alexfh Could you file a bug for the crash? Re. the libc++ part the fix should be fairly trivial. We can just add another tag, which avoids the instantiation when evaluating |
I'll wait till your libc++ fix lands and will check if the crash goes away to see if it's a crash-on-valid or crash-on-invalid. |
|
Aha, I've reduced the example to: template<class>
struct Trait;
template<class T>
constexpr bool TraitV = Trait<T>::value;
template<class T>
struct S {
S() noexcept(TraitV<T>) = default;
};
template<class T>
struct S2 : S<T> {};
S2<char> s;This should be crash-on-invalid . |
philnik777
added a commit
that referenced
this pull request
Aug 23, 2025
This fixes a bug reported in #151654 (comment).
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Aug 23, 2025
…s (#154517) This fixes a bug reported in llvm/llvm-project#151654 (comment).
Thanks for reducing this! The crash doesn't reproduce after #154517, so it's indeed a crash-on-invalid. Filed #155320 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The SFINAE isn't required, since the primary
tupleclass already does the SFINAE checks. This removes a bit of code that was only used for these constraints.This also moves the
tuple_elementspecialization fortupleto__fwd/tuple.hto avoid a dependency on__tuple/sfinae_helpers.h(which should be moved in a follow-up).