On Thu, 14 May 2026 at 08:15, Dragon Archer <[email protected]> wrote: > > On 2026/05/14 04:25, Jonathan Wakely wrote: > > On Wed, 13 May 2026 at 15:29, Patrick Palka <[email protected]> wrote: > >> > >> On Fri, 8 May 2026, Dragon Archer wrote: > >> > >>> PR libstdc++/125228 > >>> > >>> This patch replaces the uses of `assert` in ryu and debug.cc with > >>> `__glibcxx_assert`, and removed their direct dependency on `<cassert>`. > >>> To avoid modifying the third-party ryu headers, this patch use > >>> `#pragma push_macro` / `#pragma pop_macro` to locally > >>> redefine `assert` to `__glibcxx_assert` when including the ryu headers. > >>> > >>> libstdc++-v3/ChangeLog: > >>> > >>> PR libstdc++/125228 > >>> * src/c++11/debug.cc: Replace assert with __glibcxx_assert, > >>> and remove the include of <cassert>. > >>> * src/c++17/floating_to_chars.cc: Likewise, but redefine > >>> assert as __glibcxx_assert using pragma push/pop_macro. > >>> > >>> -- > >>> > >>> From 76f866652f9a7e2fc4faf012f6b356e9a85ecb8d Mon Sep 17 00:00:00 2001 > >>> From: dragon-archer <[email protected]> > >>> Date: Fri, 8 May 2026 19:14:49 +0800 > >>> Subject: [PATCH] libstdc++: Use __glibcxx_assert instead of assert > >>> > >>> Unlike `__glibcxx_assert` which is guarded > >>> by `_GLIBCXX_ASSERTIONS` and enabled only in Debug > >>> build of libstdc++, `assert` is either always enabled, or > >>> always disabled if manually defining `NDEBUG` before > >>> `#include <cassert>` or `#include <assert.h>`. This not > >>> only makes `assert` inflexible, but also introduces extra > >>> runtime overhead and/or increased binary size in Release > >>> builds. > >>> > >>> Uses of `assert` without `NDEBUG` introduces `__FILE__` > >>> into the final library, and can be easily found using > >>> `string` > >> > >> Seems reasonable to me (though pragma push/pop_macro don't seem strictly > >> necessary as Jonathan pointed out). > > > > I'm a little concerned that the use of the push/pop_macro pragmas > > gives the impression that we do expect 'assert' to be used elsewhere > > in that file, which isn't true. > > > > I think it would be sufficient and express the intent better to do: > > > > #define assert __glibcxx_assert > > ... include the ryu files ... > > #undef assert > > > > If for some reason assert has already been defined at that point, > > we'll get "warning: 'assert' redefined" to tell us something > > unexpected is happening, and we can investigate. > > > > Sounds reasonable. I'v updated the patch below and adopt this change.
Great, thanks! I'll push this to git for you. > > --- > From 7bcd2dcb248e22908cafa01bb412c7f3bad9bba2 Mon Sep 17 00:00:00 2001 > From: dragon-archer <[email protected]> > Date: Fri, 8 May 2026 19:14:49 +0800 > Subject: [PATCH] libstdc++: use __glibcxx_assert instead of assert > [PR125228] > > This patch replaces the uses of `assert` in ryu and debug.cc with > `__glibcxx_assert`, and removed their direct dependency on `<cassert>`. > To avoid modifying the third-party ryu headers, this patch define > `assert` to `__glibcxx_assert` when including the ryu headers. > > libstdc++-v3/ChangeLog: > > PR libstdc++/125228 > * src/c++11/debug.cc: Replace assert with __glibcxx_assert, > and remove the include of <cassert>. > * src/c++17/floating_to_chars.cc: Likewise, but define assert > as __glibcxx_assert to avoid modifying the ryu headers. > --- > libstdc++-v3/src/c++11/debug.cc | 33 ++++++++++----------- > libstdc++-v3/src/c++17/floating_to_chars.cc | 3 +- > 2 files changed, 18 insertions(+), 18 deletions(-) > > diff --git a/libstdc++-v3/src/c++11/debug.cc > b/libstdc++-v3/src/c++11/debug.cc > index 7049d5f1238..9a36bdfabb9 100644 > --- a/libstdc++-v3/src/c++11/debug.cc > +++ b/libstdc++-v3/src/c++11/debug.cc > @@ -33,7 +33,6 @@ > #include <debug/safe_local_iterator.h> > #include <debug/vector> > > -#include <cassert> > #include <cstdio> // for std::fprintf, stderr > #include <cstdlib> // for std::abort > #include <cctype> // for std::isspace. > @@ -888,7 +887,7 @@ namespace > void > print_named_name(PrintContext& ctx, const _Parameter::_Named& named) > { > - assert(named._M_name); > + __glibcxx_assert(named._M_name); > pretty_print(ctx, named._M_name, print_word); > } > > @@ -985,7 +984,7 @@ namespace > print_iterator_state(ctx, iterator); > else if (__builtin_strcmp(fname, "sequence") == 0) > { > - assert(iterator._M_sequence); > + __glibcxx_assert(iterator._M_sequence); > print_address(ctx, iterator._M_sequence); > } > else if (__builtin_strcmp(fname, "seq_type") == 0) > @@ -999,43 +998,43 @@ namespace > void > print_field(PrintContext& ctx, const _Parameter& param, const char* > fname) > { > - assert(param._M_kind != _Parameter::__unused_param); > + __glibcxx_assert(param._M_kind != _Parameter::__unused_param); > > const auto& variant = param._M_variant; > switch (param._M_kind) > { > case _Parameter::__iterator: > if (!print_iterator_field(ctx, fname, variant._M_iterator)) > - assert(false); > + __glibcxx_assert(false); > break; > > case _Parameter::__sequence: > if (!print_instance_field(ctx, fname, variant._M_sequence)) > - assert(false); > + __glibcxx_assert(false); > break; > > case _Parameter::__integer: > if (!print_named_field(ctx, fname, variant._M_integer)) > - assert(false); > + __glibcxx_assert(false); > break; > > case _Parameter::__string: > if (!print_named_field(ctx, fname, variant._M_string)) > - assert(false); > + __glibcxx_assert(false); > break; > > case _Parameter::__instance: > if (!print_instance_field(ctx, fname, variant._M_instance)) > - assert(false); > + __glibcxx_assert(false); > break; > > case _Parameter::__iterator_value_type: > if (!print_type_field(ctx, fname, variant._M_iterator_value_type)) > - assert(false); > + __glibcxx_assert(false); > break; > > default: > - assert(false); > + __glibcxx_assert(false); > break; > } > } > @@ -1198,9 +1197,9 @@ namespace > } > > // Get the parameter number > - assert(*str >= '1' && *str <= '9'); > + __glibcxx_assert(*str >= '1' && *str <= '9'); > size_t param_index = *str - '0' - 1; > - assert(param_index < num_parameters); > + __glibcxx_assert(param_index < num_parameters); > const auto& param = parameters[param_index]; > > // '.' separates the parameter number from the field > @@ -1208,7 +1207,7 @@ namespace > ++str; > if (*str != '.') > { > - assert(*str == ';'); > + __glibcxx_assert(*str == ';'); > ++str; > if (param._M_kind == _Parameter::__integer) > print_integer(ctx, param._M_variant._M_integer._M_value); > @@ -1226,8 +1225,8 @@ namespace > ++str; > while (*str != ';') > { > - assert(*str); > - assert(field_idx < max_field_len - 1); > + __glibcxx_assert(*str); > + __glibcxx_assert(field_idx < max_field_len - 1); > field[field_idx++] = *str++; > } > ++str; > @@ -1382,7 +1381,7 @@ namespace __gnu_debug > print_literal(ctx, "Error: "); > > // Print the error message > - assert(_M_text); > + __glibcxx_assert(_M_text); > print_string(ctx, _M_text, -1, _M_parameters, _M_num_parameters); > print_literal(ctx, ".\n"); > > diff --git a/libstdc++-v3/src/c++17/floating_to_chars.cc > b/libstdc++-v3/src/c++17/floating_to_chars.cc > index 9bddffd70ff..c5dce2177e3 100644 > --- a/libstdc++-v3/src/c++17/floating_to_chars.cc > +++ b/libstdc++-v3/src/c++17/floating_to_chars.cc > @@ -26,7 +26,6 @@ > > #include <bit> > #include <cfenv> > -#include <cassert> > #include <cmath> > #include <cstdio> > #include <cstring> > @@ -106,6 +105,7 @@ namespace > > namespace ryu > { > +#define assert __glibcxx_assert > #include "ryu/common.h" > #include "ryu/digit_table.h" > #include "ryu/d2s_intrinsics.h" > @@ -123,6 +123,7 @@ namespace > # include "ryu/ryu_generic_128.h" > # include "ryu/generic_128.c" > } // namespace generic128 > +#undef assert > > using generic128::floating_decimal_128; > using generic128::generic_binary_to_decimal; > -- > 2.54.0.windows.1 > > > >> > >> 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. > >> > >> 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)? > >> > >>> > >>> This is a long standing issue, dating from at least GCC > >>> 8.5.0 to latest GCC 16.1.0 > >>> --- > >>> libstdc++-v3/src/c++11/debug.cc | 33 ++++++++++----------- > >>> libstdc++-v3/src/c++17/floating_to_chars.cc | 5 +++- > >>> 2 files changed, 20 insertions(+), 18 deletions(-) > >>> > >>> diff --git a/libstdc++-v3/src/c++11/debug.cc > >>> b/libstdc++-v3/src/c++11/debug.cc > >>> index 7049d5f1238..9a36bdfabb9 100644 > >>> --- a/libstdc++-v3/src/c++11/debug.cc > >>> +++ b/libstdc++-v3/src/c++11/debug.cc > >>> @@ -33,7 +33,6 @@ > >>> #include <debug/safe_local_iterator.h> > >>> #include <debug/vector> > >>> > >>> -#include <cassert> > >>> #include <cstdio> // for std::fprintf, stderr > >>> #include <cstdlib> // for std::abort > >>> #include <cctype> // for std::isspace. > >>> @@ -888,7 +887,7 @@ namespace > >>> void > >>> print_named_name(PrintContext& ctx, const _Parameter::_Named& named) > >>> { > >>> - assert(named._M_name); > >>> + __glibcxx_assert(named._M_name); > >>> pretty_print(ctx, named._M_name, print_word); > >>> } > >>> > >>> @@ -985,7 +984,7 @@ namespace > >>> print_iterator_state(ctx, iterator); > >>> else if (__builtin_strcmp(fname, "sequence") == 0) > >>> { > >>> - assert(iterator._M_sequence); > >>> + __glibcxx_assert(iterator._M_sequence); > >>> print_address(ctx, iterator._M_sequence); > >>> } > >>> else if (__builtin_strcmp(fname, "seq_type") == 0) > >>> @@ -999,43 +998,43 @@ namespace > >>> void > >>> print_field(PrintContext& ctx, const _Parameter& param, const char* > >>> fname) > >>> { > >>> - assert(param._M_kind != _Parameter::__unused_param); > >>> + __glibcxx_assert(param._M_kind != _Parameter::__unused_param); > >>> > >>> const auto& variant = param._M_variant; > >>> switch (param._M_kind) > >>> { > >>> case _Parameter::__iterator: > >>> if (!print_iterator_field(ctx, fname, variant._M_iterator)) > >>> - assert(false); > >>> + __glibcxx_assert(false); > >>> break; > >>> > >>> case _Parameter::__sequence: > >>> if (!print_instance_field(ctx, fname, variant._M_sequence)) > >>> - assert(false); > >>> + __glibcxx_assert(false); > >>> break; > >>> > >>> case _Parameter::__integer: > >>> if (!print_named_field(ctx, fname, variant._M_integer)) > >>> - assert(false); > >>> + __glibcxx_assert(false); > >>> break; > >>> > >>> case _Parameter::__string: > >>> if (!print_named_field(ctx, fname, variant._M_string)) > >>> - assert(false); > >>> + __glibcxx_assert(false); > >>> break; > >>> > >>> case _Parameter::__instance: > >>> if (!print_instance_field(ctx, fname, variant._M_instance)) > >>> - assert(false); > >>> + __glibcxx_assert(false); > >>> break; > >>> > >>> case _Parameter::__iterator_value_type: > >>> if (!print_type_field(ctx, fname, variant._M_iterator_value_type)) > >>> - assert(false); > >>> + __glibcxx_assert(false); > >>> break; > >>> > >>> default: > >>> - assert(false); > >>> + __glibcxx_assert(false); > >>> break; > >>> } > >>> } > >>> @@ -1198,9 +1197,9 @@ namespace > >>> } > >>> > >>> // Get the parameter number > >>> - assert(*str >= '1' && *str <= '9'); > >>> + __glibcxx_assert(*str >= '1' && *str <= '9'); > >>> size_t param_index = *str - '0' - 1; > >>> - assert(param_index < num_parameters); > >>> + __glibcxx_assert(param_index < num_parameters); > >>> const auto& param = parameters[param_index]; > >>> > >>> // '.' separates the parameter number from the field > >>> @@ -1208,7 +1207,7 @@ namespace > >>> ++str; > >>> if (*str != '.') > >>> { > >>> - assert(*str == ';'); > >>> + __glibcxx_assert(*str == ';'); > >>> ++str; > >>> if (param._M_kind == _Parameter::__integer) > >>> print_integer(ctx, param._M_variant._M_integer._M_value); > >>> @@ -1226,8 +1225,8 @@ namespace > >>> ++str; > >>> while (*str != ';') > >>> { > >>> - assert(*str); > >>> - assert(field_idx < max_field_len - 1); > >>> + __glibcxx_assert(*str); > >>> + __glibcxx_assert(field_idx < max_field_len - 1); > >>> field[field_idx++] = *str++; > >>> } > >>> ++str; > >>> @@ -1382,7 +1381,7 @@ namespace __gnu_debug > >>> print_literal(ctx, "Error: "); > >>> > >>> // Print the error message > >>> - assert(_M_text); > >>> + __glibcxx_assert(_M_text); > >>> print_string(ctx, _M_text, -1, _M_parameters, _M_num_parameters); > >>> print_literal(ctx, ".\n"); > >>> > >>> diff --git a/libstdc++-v3/src/c++17/floating_to_chars.cc > >>> b/libstdc++-v3/src/c++17/floating_to_chars.cc > >>> index 9bddffd70ff..264e520e187 100644 > >>> --- a/libstdc++-v3/src/c++17/floating_to_chars.cc > >>> +++ b/libstdc++-v3/src/c++17/floating_to_chars.cc > >>> @@ -26,7 +26,6 @@ > >>> > >>> #include <bit> > >>> #include <cfenv> > >>> -#include <cassert> > >>> #include <cmath> > >>> #include <cstdio> > >>> #include <cstring> > >>> @@ -106,6 +105,9 @@ namespace > >>> > >>> namespace ryu > >>> { > >>> +#pragma push_macro("assert") > >>> +#undef assert > >>> +#define assert __glibcxx_assert > >>> #include "ryu/common.h" > >>> #include "ryu/digit_table.h" > >>> #include "ryu/d2s_intrinsics.h" > >>> @@ -123,6 +125,7 @@ namespace > >>> # include "ryu/ryu_generic_128.h" > >>> # include "ryu/generic_128.c" > >>> } // namespace generic128 > >>> +#pragma pop_macro("assert") > >>> > >>> using generic128::floating_decimal_128; > >>> using generic128::generic_binary_to_decimal; > >>> -- > >>> 2.54.0.windows.1 > >>> > >>> > >>> > >> > > >
