Now fixed as trivial, I set you as author.
François
On 08/07/2025 13:55, Jonathan Wakely wrote:
On Mon, 7 Jul 2025 at 11:12, Jonathan Wakely <jwak...@redhat.com> wrote:
On Sat, 5 Jul 2025 at 14:03, François Dumont <frs.dum...@gmail.com> wrote:
On 01/07/2025 22:51, Jonathan Wakely wrote:
On Mon, 16 Jun 2025 at 18:36, François Dumont <frs.dum...@gmail.com> wrote:
I eventually wonder if it is such a big deal to add the new symbols for
_GLIBCXX_DEBUG mode.
I like this version much more than the one trying to duplicate symbols with asm.
Here is the patch doing this. It avoids to add many const_cast which is what we
are trying to achieve here.
I'm still not really sure if this is worth it though - is it fixing a
bug or a correctness problem? (using const_cast is safe if the objects
aren't actually const)
All the new tests already pass, even without this patch. Are these
just tests for const member functions that we aren't currently testing
at all?
Those tests are showing the same UB that you fixed as part of your
PR116369 patch but this time with local_iterator. Even if tests are
passing without this patch it's still UB before it, do you prefer to
remove those tests then ?
Ah OK, so they are showing UB ... it's just that the compiler doesn't
actually complain about it.
Please make the const containers in those tests global variables,
instead of local variables inside main(). The compiler won't put local
variables in ROM so the test would never fail. It might put globals in
ROM (although not after your patch, because of the mutable members,
which is why the patch is actually fixing something).
Globally this patch is following your recommendations on PR116369 commit
where you were saying:
Ideally we would not need the const_cast at all. Instead, the _M_attach
member (and everything it calls) should be const-qualified. That would
work fine now, because the members that it ends up modifying are
mutable. Making that change would require a number of new exports from
the shared library, and would require retaining the old non-const
member
functions (maybe as symbol aliases) for backwards compatibility. That
might be worth changing at some point, but isn't done here.
In addition to what is said here I made the sequence pointer const too
as the added mutable allows that.
It was also the occasion to fix some types used in std::forward_list in
Debug mode.
Do you think it is useless eventually ?
I think it's worth doing, I was just concerned about the __asm__
solution used in the initial patches.
OK for trunk with the adjusted tests, thanks.
I see a large number of new test failures. It looks like this fix is needed:
--- a/libstdc++-v3/include/debug/forward_list
+++ b/libstdc++-v3/include/debug/forward_list
@@ -144,13 +144,13 @@ namespace __gnu_debug
//std::swap(_M_this()->_M_version, __other._M_version);
_Safe_iterator_base* __this_its = _M_this()->_M_iterators;
_S_swap_aux(__other, __other._M_iterators,
- _M_this(), _M_this()->_M_iterators);
+ *_M_this(), _M_this()->_M_iterators);
_Safe_iterator_base* __this_const_its = _M_this()->_M_const_iterators;
_S_swap_aux(__other, __other._M_const_iterators,
- _M_this(), _M_this()->_M_const_iterators);
- _S_swap_aux(_M_this(), __this_its,
+ *_M_this(), _M_this()->_M_const_iterators);
+ _S_swap_aux(*_M_this(), __this_its,
__other, __other._M_iterators);
- _S_swap_aux(_M_this(), __this_const_its,
+ _S_swap_aux(*_M_this(), __this_const_its,
__other, __other._M_const_iterators);
}
