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