On Tue, 27 May 2025 at 13:26, Tomasz Kaminski <tkami...@redhat.com> wrote:
>
>
>
> On Fri, May 23, 2025 at 7:00 PM Jonathan Wakely <jwak...@redhat.com> wrote:
>>
>> Since r16-154-gc91eb5a5c13f14 std::addressof is no less efficient than
>> std::__addressof, so change some uses of the latter to the former.
>>
>> We can't change them all, because some uses need to compile as C++98
>> which only has std::__addressof.
>>
>> libstdc++-v3/ChangeLog:
>>
>> * include/bits/stl_construct.h: Replace std::__addressof with
>> std::addressof.
>> * include/bits/stl_uninitialized.h: Likewise.
>> ---
>>
>> I'm undecided whether it's better to use the standard std::addressof for
>> simplicity, or to stick with std::__addressof in files where there's a
>> mix of C++98 code and >= C++11 code.
>
> After giving this a bit of thought. If I touch this files, then most likely we
> are implementing the new standard features, and in the patch I will just
> default to std::addressof, as in any other case. So preferring to use
> std::addressof when available seems like easier to maintain policy.
>
>>
>>
>> Obviously in files that don't need to compile as C++98 (such as
>> <optional>) we could just use std::addressof.
>>
>> Tested x86_64-linux.
>>
>>
>> libstdc++-v3/include/bits/stl_construct.h | 2 +-
>> libstdc++-v3/include/bits/stl_uninitialized.h | 28 +++++++++----------
>> 2 files changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/libstdc++-v3/include/bits/stl_construct.h
>> b/libstdc++-v3/include/bits/stl_construct.h
>> index 23b8fb754710..a53274e33c0c 100644
>> --- a/libstdc++-v3/include/bits/stl_construct.h
>> +++ b/libstdc++-v3/include/bits/stl_construct.h
>> @@ -82,7 +82,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> if constexpr (__cplusplus > 201703L && is_array_v<_Tp>)
>> {
>> for (auto& __x : *__location)
>> - std::destroy_at(std::__addressof(__x));
>> + std::destroy_at(std::addressof(__x));
>> }
>> else
>> __location->~_Tp();
>
> There are calls to __addressof in _Destroy at lines 212, 216, 268, 272 in
> calls that are >= C++11.
> If we update them, we should also change all of them in file.
I have another patch locally which replaced those ones (and in my tree
that patch is actually first).
I'll finish the other changes to stl_construct.h that I'm working on,
and then after that I'll replace all >= C++11 uses of __addressof.
>
>>
>> diff --git a/libstdc++-v3/include/bits/stl_uninitialized.h
>> b/libstdc++-v3/include/bits/stl_uninitialized.h
>> index b1428db48b00..bde787c2beaa 100644
>> --- a/libstdc++-v3/include/bits/stl_uninitialized.h
>> +++ b/libstdc++-v3/include/bits/stl_uninitialized.h
>
> Looks like all of ones that could be updated in this file, are already.
>>
>> @@ -839,7 +839,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> {
>> _UninitDestroyGuard<_ForwardIterator> __guard(__first);
>> for (; __first != __last; ++__first)
>> - std::_Construct(std::__addressof(*__first));
>> + std::_Construct(std::addressof(*__first));
>> __guard.release();
>> }
>> };
>> @@ -856,7 +856,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> return;
>>
>> typename iterator_traits<_ForwardIterator>::value_type* __val
>> - = std::__addressof(*__first);
>> + = std::addressof(*__first);
>> std::_Construct(__val);
>> if (++__first != __last)
>> std::fill(__first, __last, *__val);
>> @@ -873,7 +873,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> {
>> _UninitDestroyGuard<_ForwardIterator> __guard(__first);
>> for (; __n > 0; --__n, (void) ++__first)
>> - std::_Construct(std::__addressof(*__first));
>> + std::_Construct(std::addressof(*__first));
>> __guard.release();
>> return __first;
>> }
>> @@ -890,7 +890,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> if (__n > 0)
>> {
>> typename iterator_traits<_ForwardIterator>::value_type* __val
>> - = std::__addressof(*__first);
>> + = std::addressof(*__first);
>> std::_Construct(__val);
>> ++__first;
>> __first = std::fill_n(__first, __n - 1, *__val);
>> @@ -955,7 +955,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> __alloc);
>> typedef __gnu_cxx::__alloc_traits<_Allocator> __traits;
>> for (; __first != __last; ++__first)
>> - __traits::construct(__alloc, std::__addressof(*__first));
>> + __traits::construct(__alloc, std::addressof(*__first));
>> __guard.release();
>> }
>>
>> @@ -980,7 +980,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> __alloc);
>> typedef __gnu_cxx::__alloc_traits<_Allocator> __traits;
>> for (; __n > 0; --__n, (void) ++__first)
>> - __traits::construct(__alloc, std::__addressof(*__first));
>> + __traits::construct(__alloc, std::addressof(*__first));
>> __guard.release();
>> return __first;
>> }
>> @@ -1007,7 +1007,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> {
>> _UninitDestroyGuard<_ForwardIterator> __guard(__first);
>> for (; __first != __last; ++__first)
>> - std::_Construct_novalue(std::__addressof(*__first));
>> + std::_Construct_novalue(std::addressof(*__first));
>> __guard.release();
>> }
>> };
>> @@ -1033,7 +1033,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> {
>> _UninitDestroyGuard<_ForwardIterator> __guard(__first);
>> for (; __n > 0; --__n, (void) ++__first)
>> - std::_Construct_novalue(std::__addressof(*__first));
>> + std::_Construct_novalue(std::addressof(*__first));
>> __guard.release();
>> return __first;
>> }
>> @@ -1089,7 +1089,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> {
>> _UninitDestroyGuard<_ForwardIterator> __guard(__result);
>> for (; __n > 0; --__n, (void) ++__first, ++__result)
>> - std::_Construct(std::__addressof(*__result), *__first);
>> + std::_Construct(std::addressof(*__result), *__first);
>> __guard.release();
>> return __result;
>> }
>> @@ -1112,7 +1112,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> {
>> _UninitDestroyGuard<_ForwardIterator> __guard(__result);
>> for (; __n > 0; --__n, (void) ++__first, ++__result)
>> - std::_Construct(std::__addressof(*__result), *__first);
>> + std::_Construct(std::addressof(*__result), *__first);
>> __guard.release();
>> return {__first, __result};
>> }
>> @@ -1276,11 +1276,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> noexcept(noexcept(std::allocator_traits<_Allocator>::construct(__alloc,
>> __dest, std::move(*__orig)))
>> && noexcept(std::allocator_traits<_Allocator>::destroy(
>> - __alloc, std::__addressof(*__orig))))
>> + __alloc, std::addressof(*__orig))))
>> {
>> typedef std::allocator_traits<_Allocator> __traits;
>> __traits::construct(__alloc, __dest, std::move(*__orig));
>> - __traits::destroy(__alloc, std::__addressof(*__orig));
>> + __traits::destroy(__alloc, std::addressof(*__orig));
>> }
>>
>> // This class may be specialized for specific types.
>> @@ -1308,8 +1308,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> "relocation is only possible for values of the same type");
>> _ForwardIterator __cur = __result;
>> for (; __first != __last; ++__first, (void)++__cur)
>> - std::__relocate_object_a(std::__addressof(*__cur),
>> - std::__addressof(*__first), __alloc);
>> + std::__relocate_object_a(std::addressof(*__cur),
>> + std::addressof(*__first), __alloc);
>> return __cur;
>> }
>>
>> --
>> 2.49.0
>>
