[libc++] Add tombstone traits to use in optional, variant #98498
Conversation
| _LIBCPP_HIDE_FROM_ABI constexpr bool __is_engaged() const noexcept { | ||
| if (__libcpp_is_constant_evaluated()) | ||
| return !__builtin_constant_p(__tombstone_.__is_disengaged_ == _TombstoneLayout::__disengaged_value_); | ||
| return __tombstone_.__is_disengaged_ != _TombstoneLayout::__disengaged_value_; |
There was a problem hiding this comment.
This needs a comment explaining that this is technically UB. Or better, go through reinterpret_cast<std::byte*> or something to make it well defined.
|
|
||
| _LIBCPP_HIDE_FROM_ABI constexpr bool __is_engaged() const noexcept { | ||
| if (__libcpp_is_constant_evaluated()) | ||
| return !__builtin_constant_p(__tombstone_.__is_disengaged_ == _TombstoneLayout::__disengaged_value_); |
There was a problem hiding this comment.
We should get the compiler folks to actually stamp this kind of usage of __builtin_constant_p, just to make sure we don't get ourselves in trouble.
| _LIBCPP_BEGIN_NAMESPACE_STD | ||
|
|
||
| template <class> | ||
| struct __tombstone_memory_layout; |
There was a problem hiding this comment.
Since this is the class that we intend types to specialize, this should actually be named __tombstone_traits. It's a bit like iterator_traits or __segmented_iterator_traits.
We can find another name for the current __tombstone_traits class.
| }; | ||
|
|
||
| template <class _Tp, class _Payload> | ||
| struct __tombstone_traits { |
There was a problem hiding this comment.
| struct __tombstone_traits { | |
| struct __tombstoned_value { |
?? Not super pretty but it explains what it does -- it adds a tombstone state to an object.
| using _IsDisengagedT = remove_cv_t<decltype(_TombstoneLayout::__disengaged_value_)>; | ||
|
|
||
| _LIBCPP_NO_UNIQUE_ADDRESS _Payload __payload_; | ||
| char __padding_[_TombstoneLayout::__is_disengaged_offset_ - __datasizeof_v<_Payload>]; |
There was a problem hiding this comment.
static_assert(disengaged-offset >= datasizeof(Payload)) to catch misuse?
| // - vector<T> with alignof(T) == 1 | ||
|
|
||
| template <class _Tp> | ||
| struct __tombstone_memory_layout<__enable_specialization_if<is_fundamental_v<_Tp> && alignof(_Tp) >= 2, _Tp*>> |
There was a problem hiding this comment.
This needs a comment explaining why is_fundamental is important.
| static constexpr size_t __is_disengaged_offset_ = 0; | ||
| }; | ||
|
|
||
| struct __tombstone_pointer_layout { |
There was a problem hiding this comment.
I'm not a fan of defining __tombstone_pointer_layout since there are unwritten assumptions you're making in that type (namely the alignment of the pointed-to type).
| @@ -686,6 +686,12 @@ template <class _CharT, class _Traits> | |||
| inline constexpr bool ranges::enable_borrowed_range<basic_string_view<_CharT, _Traits> > = true; | |||
| #endif // _LIBCPP_STD_VER >= 20 | |||
|
|
|||
| template <class _CharT, class _Traits> | |||
| struct __tombstone_memory_layout<basic_string_view<_CharT, _Traits>> { | |||
| static constexpr size_t __disengaged_value_ = basic_string_view<_CharT, _Traits>::npos; | |||
There was a problem hiding this comment.
I think npos is a valid size for a string_view. It's somewhat academic but I don't think this specific optimization is valid.
No description provided.