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.

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.

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). 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.

Reply via email to