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.
