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.
