On Fri, 29 Sept 2023 at 10:32, Jonathan Wakely <jwakely@redhat.com> wrote:
> > Thanks for the comments, here's an updated version of the patch.
> > Bootstrapped and regtested on x86_64-pc-linux-gnu.
>
> Great, I'll get this committed today - thanks!

That's done now.

> >
> > I'll note that there are some existing calls to `_M_use_local_data()`
> > already used only for their side effects without a cast to void, e.g.
> >
> >       /**
> >        *  @brief  Default constructor creates an empty string.
> >        */
> >       _GLIBCXX20_CONSTEXPR
> >       basic_string()
> >       _GLIBCXX_NOEXCEPT_IF(is_nothrow_default_constructible<_Alloc>::value)
> >       : _M_dataplus(_M_local_data())
> >       {
> >         _M_use_local_data();
> >         _M_set_length(0);
> >       }
> >
> > I haven't updated these, but should this be changed for consistency?
>
> Yes, good idea. I can do that.

I started to do that, and decided it made more sense to split out the
constexpr loop from _M_use_local_data() into a separate function,
_M_init_local_buf(). Then we can use that for all the places where we
don't care about the return value. That avoids the overhead of using
pointer_traits::pointer_to when we don't need the return value (which
is admittedly only going to be an issue for -O0 code, but I think it's
cleaner this way anyway).

Please see the attached patch and let me know what you think.


> Thanks again for fixing these. I think this might fix some bug reports
> about clang rejecting our std::string in constant expressions, so I'll
> check those.

Your patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110900
(so we should backport it to gcc-13 and gcc-12 too).