On Wed, 11 Jun 2025 at 10:47, Tomasz Kaminski <tkami...@redhat.com> wrote:
>
>
>
> On Wed, Jun 11, 2025 at 11:39 AM Jonathan Wakely <jwak...@redhat.com> wrote:
>>
>> On Wed, 11 Jun 2025 at 10:24, Tomasz Kaminski <tkami...@redhat.com> wrote:
>> >
>> >
>> >
>> > On Tue, Jun 10, 2025 at 12:37 PM Jonathan Wakely <jwak...@redhat.com>
>> > wrote:
>> >>
>> >> By using std::is_trivially_destructible instead of the old
>> >> __has_trivial_destructor built-in we no longer need the static_assert to
>> >> deal with types with deleted destructors. All non-destructible types,
>> >> including those with deleted destructors, will now give user-friendly
>> >> diagnostics that clearly explain the problem.
>> >>
>> >> Also combine the _Destroy_aux and _Destroy_n_aux class templates used
>> >> for C++98 into one, so that we perform fewer expensive class template
>> >> instantiations.
>> >>
>> >> libstdc++-v3/ChangeLog:
>> >>
>> >> PR libstdc++/120390
>> >> * include/bits/stl_construct.h (_Destroy_aux::__destroy_n): New
>> >> static member function.
>> >> (_Destroy_aux<true>::__destroy_n): Likewise.
>> >> (_Destroy_n_aux): Remove.
>> >> (_Destroy(ForwardIterator, ForwardIterator)): Remove
>> >> static_assert. Use is_trivially_destructible instead of
>> >> __has_trivial_destructor.
>> >> (_Destroy_n): Likewise. Use _Destroy_aux::__destroy_n instead of
>> >> _Destroy_n_aux::__destroy_n.
>> >> *
>> >> testsuite/20_util/specialized_algorithms/memory_management_tools/destroy_neg.cc:
>> >> Adjust dg-error strings. Move destroy_n tests to ...
>> >> *
>> >> testsuite/20_util/specialized_algorithms/memory_management_tools/destroy_n_neg.cc:
>> >> New test.
>> >> * testsuite/23_containers/vector/cons/destructible_debug_neg.cc:
>> >> Adjust dg-error strings.
>> >> * testsuite/23_containers/vector/cons/destructible_neg.cc:
>> >> Likewise.
>> >> ---
>> >>
>> >> Tested x86_64-linux.
>> >
>> > LGTM.
>> > Not directly related to the patch, but I have noticed that for c++98, for
>> > trivially destructible
>> > types we make sure that we traverse range (we call advance) however in
>> > C++11+ mode
>> > we do not do that. Something changed in standard?
>>
>> I think they should be equivalent. For C++98 the
>> _Destroy_aux<true>::__destroy_n function calls advance and returns the
>> result:
>>
>>
>> >> @@ -189,6 +198,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> >> template<typename _ForwardIterator>
>> >> static void
>> >> __destroy(_ForwardIterator, _ForwardIterator) { }
>> >> +
>> >> + template<typename _ForwardIterator, typename _Size>
>> >> + static _ForwardIterator
>> >> + __destroy_n(_ForwardIterator __first, _Size __count)
>> >> + {
>> >> + std::advance(__first, __count);
>> >> + return __first;
>> >> + }
>> >> };
>> >> #endif
>>
>> For C++11 we do the same directly in the _Destroy_n function:
>>
>> >> @@ -260,10 +247,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> >> typedef typename iterator_traits<_ForwardIterator>::value_type
>> >> _Value_type;
>> >> #if __cplusplus >= 201103L
>> >> - // A deleted destructor is trivial, this ensures we reject such
>> >> types:
>> >> - static_assert(is_destructible<_Value_type>::value,
>> >> - "value type is destructible");
>> >> - if constexpr (!__has_trivial_destructor(_Value_type))
>> >> + if constexpr (!is_trivially_destructible<_Value_type>::value)
>> >> for (; __count > 0; (void)++__first, --__count)
>> >> std::_Destroy(std::__addressof(*__first));
>> >> #if __cpp_constexpr_dynamic_alloc // >= C++20
>> >> @@ -275,7 +259,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> >> std::advance(__first, __count);
>> >> return __first;
>>
>> ^ here.
>
> Thanks, I completely missed this in the else branch.
All the #if / #else groups make it quite messy :-(
