[PATCH] libstdc++: replace assert with __glibcxx_assert (PR libstdc++/125228)

[Dragon Archer]

On Wed, 13 May 2026 at 16:46, Jonathan Wakely <jwakely@redhat.com> wrote:
> On Wed, 13 May 2026 at 16:14, Dragon Archer <dragon-archer@outlook.com> wrote:
> >
> > Thank you for your review and thoughtful questions.
> >
> > > Seems reasonable to me (though pragma push/pop_macro don't seem strictly
> > > necessary as Jonathan pointed out).
> > >
> > > Though in theory I guess there's a risk of a performance regression by
> > > removing an assert in Ryu, if the compiler relies on an assert condition
> > > to be true in order to safely apply an important optimization.
> >
> > I understand the concern. In my view:
> >
> > 1. Replacing an unconditional `assert` (which always evaluates the condition at runtime) with `__glibcxx_assert` (which is completely removed when `_GLIBCXX_ASSERTIONS` is not defined) actually **removes** runtime checks in non‑debug builds. This performance benefit likely outweighs any hypothetical optimization that the compiler might have derived from the assertion.
> >
> > 2. If we are worried about missing optimization hints, we could consider using `__builtin_assume` – that would be more semantically precise. However, that would require guarding it with `#ifdef _GLIBCXX_ASSERTIONS` and defining a fallback. I personally prefer keeping the current approach with `__glibcxx_assert` because it is already used throughout libstdc++ and matches the debugging philosophy.
>
> I don't want such changes in the Ryu code, it will just make it
> difficult to rebase on upstream. However, we might be able to replace
> it with Teju Jagua anyway.

As discussed in PR/125228, this patch only changes
libstdc++-v3/src/c++11/debug.cc and libstdc++-v3/src/c++17/floating_to_chars.cc,
both are pure GCC files that are not shared with upstream.
So there should be no rebase issues.

> >
> > 3. If the community later decides that `__glibcxx_assert` should also serve as an optimization hint in release mode, they could define it to `__builtin_assume` instead of empty when `_GLIBCXX_ASSERTIONS` is **not** defined. That would be a separate change, and will automatically be effective for this patch as well.
>
> That won't happen while I have any say in the matter.
>

I didn't want to propose that change either.

> >
> > So for this patch, I believe using `__glibcxx_assert` is the right and simple solution.
> >
> > > Just wondering, is there a legit problem/concern with __FILE__ appearing
> > > in the final library or is it a matter of QoI (which I agree with)?
> >
> > It is primarily a QoI improvement. The presence of `__FILE__` strings increases binary size and may leak internal build paths (which is a minor privacy/security concern for some users).
>
> Users who care about that can easily compile in /tmp or another
> non-secret path. You can also compile with --enable-cxx-flags=-DNDEBUG
> to disable those assertions in your own builds.
>

Yes, that's true, but I think this patch is clean enough and can be
made available in upstream without causing any issues. It is a small
change that improves the quality of the library without affecting
functionality (and may even improve performance in non‑debug builds).

> > Eliminating them makes the library cleaner, but there is no functional correctness issue. BTW, I think the removal of runtime checks will improve perfomance slightly.
> >
> > Again, thank you for your time and feedback.