On Mon, 23 Jun 2025 at 18:19, François Dumont <frs.dum...@gmail.com> wrote:
>
> Hi
>
> Even if you have no time to review this for now could you only answer the 
> question below that is to say:
>
> Should the current _GLIBCXX_INLINE_VERSION abi be preserved ?

No, that's not necessary


>
> Thanks
>
> On 16/06/2025 19:36, François Dumont wrote:
>
> I eventually wonder if it is such a big deal to add the new symbols for 
> _GLIBCXX_DEBUG mode.
>
> Here is the patch doing this. It avoids to add many const_cast which is what 
> we are trying to achieve here.
>
> I've updated the PR keeping 2 commits so that if this last step is not good I 
> can just drop it.
>
> I even updated the versioned namespace mode breaking this mode abi, I think 
> it's fine, no ?
>
> François
>
>
>
> On 08/06/2025 22:00, François Dumont wrote:
>
> Here is a new attempt preserving symbols.
>
>     libstdc++: Make debug iterator pointer sequence const [PR116369]
>
>     In revision a35dd276cbf6236e08bcf6e56e62c2be41cf6e3c the debug sequence
>     have been made mutable to allow attach iterators to const containers.
>     This change completes this fix by also declaring debug unordered container
>     members mutable.
>
>     Additionally the debug iterator sequence is now a pointer-to-const and so
>     _Safe_sequence_base _M_attach and all other methods are const qualified.
>     Not-const methods exported are preserved for abi backward compatibility. 
> The
>     new const methods are calling the latter thanks to a safe use of 
> const_cast.
>
>     libstdc++-v3/ChangeLog:
>
>             PR c++/116369
>             * include/debug/safe_base.h
>             (_Safe_iterator_base::_M_sequence): Declare as pointer-to-const.
>             (_Safe_iterator_base::_M_attach, _M_attach_single): New, take 
> pointer-to-const
>             _Safe_sequence_base.
>             (_Safe_sequence_base::_M_detach_all, _M_detach_singular, 
> _M_revalidate_singular)
>             (_M_swap, _M_get_mutex): New, const qualified.
>             (_Safe_sequence_base::_M_attach, _M_attach_single, _M_detach, 
> _M_detach_single):
>             const qualify.
>             * include/debug/safe_container.h (_Safe_container<>::_M_cont): 
> Add const qualifier.
>             (_Safe_container<>::_M_swap_base): New.
>             (_Safe_container(_Safe_container&&, const _Alloc&, 
> std::false_type)):
>             Adapt to use latter.
>             (_Safe_container<>::operator=(_Safe_container&&)): Likewise.
>             (_Safe_container<>::_M_swap): Likewise and take parameter as 
> const reference.
>             * include/debug/safe_unordered_base.h
>             (_Safe_local_iterator_base::_M_safe_container): New.
>             (_Safe_local_iterator_base::_Safe_local_iterator_base): Take
>             _Safe_unordered_container_base as pointer-to-const.
>             (_Safe_unordered_container_base::_M_attach, _M_attach_single): 
> New, take
>             container as _Safe_unordered_container_base pointer-to-const.
>             (_Safe_unordered_container_base::_M_local_iterators, 
> _M_const_local_iterators):
>             Add mutable.
>             (_Safe_unordered_container_base::_M_detach_all, _M_swap): New, 
> const qualify.
>             (_Safe_unordered_container_base::_M_attach_local, 
> _M_attach_local_single)
>             (_M_detach_local, _M_detach_local_single): Add const qualifier.
>             * include/debug/safe_iterator.h (_Safe_iterator<>::_M_attach, 
> _M_attach_single):
>             Take _Safe_sequence_base as pointer-to-const.
>             (_Safe_iterator<>::_M_get_sequence): Add const_cast and comment 
> about it.
>             * include/debug/safe_local_iterator.h (_Safe_local_iterator<>): 
> Replace usages
>             of _M_sequence member by _M_safe_container().
>             (_Safe_local_iterator<>::_M_attach, _M_attach_single): Take
>             _Safe_unordered_container_base as pointer-to-const.
>             (_Safe_local_iterator<>::_M_get_sequence): Rename into...
>             (_Safe_local_iterator<>::_M_get_ucontainer): ...this. Add 
> necessary const_cast and
>             comment to explain it.
>             (_Safe_local_iterator<>::_M_is_begin, _M_is_end): Adapt.
>             * include/debug/safe_local_iterator.tcc: Adapt.
>             * include/debug/safe_sequence.h
>             (_Safe_sequence<>::_M_invalidate_if, _M_transfer_from_if): Add 
> const qualifier.
>             * include/debug/safe_sequence.tcc: Adapt.
>             * include/debug/deque (std::__debug::deque::erase): Adapt to use 
> new const
>             qualified methods.
>             * include/debug/formatter.h: Adapt.
>             * include/debug/forward_list (_Safe_forward_list::_M_this): Add 
> const
>             qualification.
>             (_Safe_forward_list::_M_swap_aux): Rename into...
>             (_Safe_forward_list::_S_swap_aux): ...this and take sequence as 
> const reference.
>             (forward_list<>::resize): Adapt to use const methods.
>             * include/debug/list (list<>::resize): Likewise.
>             * src/c++11/debug.cc: Adapt to const qualification.
>             * testsuite/util/testsuite_containers.h
>             (forward_members_unordered::forward_members_unordered): Add check 
> on local_iterator
>             conversion to const_local_iterator.
>             (forward_members::forward_members): Add check on iterator 
> conversion to
>             const_iterator.
>             * testsuite/23_containers/unordered_map/const_container.cc: New 
> test case.
>             * testsuite/23_containers/unordered_multimap/const_container.cc: 
> New test case.
>             * testsuite/23_containers/unordered_multiset/const_container.cc: 
> New test case.
>             * testsuite/23_containers/unordered_set/const_container.cc: New 
> test case.
>             * testsuite/23_containers/vector/debug/mutex_association.cc: 
> Adapt.
>
> Tested under Linux x86_64.
>
> Ok to commit ?
>
> François
>
>
> On 26/05/2025 19:07, François Dumont wrote:
>
> Ok, I'll give it another try.
>
> Trying to use the same approach for targets using gnu.ver and others thought, 
> seems more reasonable to me.
>
> François
>
>
> On 22/05/2025 09:28, Jonathan Wakely wrote:
>
>
>
> On Thu, 22 May 2025, 08:26 Jonathan Wakely, <jwakely....@gmail.com> wrote:
>>
>>
>>
>> On Thu, 15 May 2025, 06:26 François Dumont, <frs.dum...@gmail.com> wrote:
>>>
>>> Got
>>>
>>> On 14/05/2025 18:46, Jonathan Wakely wrote:
>>> > On Wed, 14 May 2025 at 17:31, François Dumont <frs.dum...@gmail.com> 
>>> > wrote:
>>> >> On 12/05/2025 23:03, Jonathan Wakely wrote:
>>> >>> On 31/03/25 22:20 +0200, François Dumont wrote:
>>> >>>> Hi
>>> >>>>
>>> >>>> Following this previous patch
>>> >>>> https://gcc.gnu.org/pipermail/libstdc++/2024-August/059418.html I've
>>> >>>> completed it for the _Safe_unordered_container_base type and
>>> >>>> implemented the rest of the change to store the safe iterator
>>> >>>> sequence as a pointer-to-const.
>>> >>>>
>>> >>>>      libstdc++: Make debug iterator pointer sequence const [PR116369]
>>> >>>>
>>> >>>>      In revision a35dd276cbf6236e08bcf6e56e62c2be41cf6e3c the debug
>>> >>>> sequence
>>> >>>>      have been made mutable to allow attach iterators to const
>>> >>>> containers.
>>> >>>>      This change completes this fix by also declaring debug unordered
>>> >>>> container
>>> >>>>      members mutable.
>>> >>>>
>>> >>>>      Additionally the debug iterator sequence is now a
>>> >>>> pointer-to-const and so
>>> >>>>      _Safe_sequence_base _M_attach and all other methods are const
>>> >>>> qualified.
>>> >>>>      Symbols export are maintained thanks to __asm directives.
>>> >>>>
>>> >>> I can't compile this, it seems to be missing changes to
>>> >>> safe_local_iterator.tcc:
>>> >>>
>>> >>> In file included from
>>> >>> /home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/safe_local_iterator.h:444,
>>> >>>                   from
>>> >>> /home/jwakely/src/gcc/gcc/libstdc++-v3/src/c++11/debug.cc:33:
>>> >>> /home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/safe_local_iterator.tcc:
>>> >>> In member function ‘typename
>>> >>> __gnu_debug::_Distance_traits<_Iterator>::__type
>>> >>> __gnu_debug::_Safe_local_iterator<_Iterator,
>>> >>> _Sequence>::_M_get_distance_to(const
>>> >>> __gnu_debug::_Safe_local_iterator<_Iterator, _Sequence>&) const’:
>>> >>> /home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/safe_local_iterator.tcc:47:17:
>>> >>> error: there are no arguments to ‘_M_get_sequence’ that depend on a
>>> >>> template parameter, so a declaration of ‘_M_get_sequence’ must be
>>> >>> available [-Wtemplate-body]
>>> >>>     47 | _M_get_sequence()->bucket_size(bucket()),
>>> >>>        |                 ^~~~~~~~~~~~~~~
>>> >>> /home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/safe_local_iterator.tcc:47:17:
>>> >>> note: (if you use ‘-fpermissive’, G++ will accept your code, but
>>> >>> allowing the use of an undeclared name is deprecated)
>>> >>> /home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/safe_local_iterator.tcc:59:18:
>>> >>> error: there are no arguments to ‘_M_get_sequence’ that depend on a
>>> >>> template parameter, so a declaration of ‘_M_get_sequence’ must be
>>> >>> available [-Wtemplate-body]
>>> >>>     59 | -_M_get_sequence()->bucket_size(bucket()),
>>> >>>        |                  ^~~~~~~~~~~~~~~
>>> >>>
>>> >> Yes, sorry, I had already spotted this problem, but only updated the PR
>>> >> and not re-sending patch here.
>>> >>
>>> >>
>>> >>>> Also available as a PR
>>> >>>>
>>> >>>> https://forge.sourceware.org/gcc/gcc-TEST/pulls/47
>>> >>>>
>>> >>>>      /** Detach all singular iterators.
>>> >>>>       *  @post for all iterators i attached to this sequence,
>>> >>>>       *   i->_M_version == _M_version.
>>> >>>>       */
>>> >>>>      void
>>> >>>> -    _M_detach_singular();
>>> >>>> +    _M_detach_singular() const
>>> >>>> + __asm("_ZN11__gnu_debug19_Safe_sequence_base18_M_detach_singularEv");
>>> >>> Does this work on all targets?
>>> >> No idea ! I thought the symbol name used here just had to match the
>>> >> entries in config/abi/pre/gnu.ver.
>>> > That linker script is not used for all targets.
>>>
>>> Ok, got it, I only need to use this when symbol versioning is activated.
>>
>>
>> I don't think that's right. For targets that don't use gnu.ver we still want 
>> to preserve the same symbols. They just aren't versioned on those targets.
>> And e.g. Solaris uses versioning, but a different format, not gnu.ver, and I 
>> don't remember it the same macro is defined.
>>
>> Isn't it possible to do this without asm somehow? At least as a fallback for 
>> targets that don't use gnu.ver
>
>
> Basically this needs more research, and then testing on other targets.
>
>
>>
>>
>>>
>>> I think this new patch should do it if so.
>>>
>>> François
>>>

Reply via email to