On Wed, 13 May 2026 at 16:14, Dragon Archer <[email protected]> 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. > > 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. > > 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. > 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.
