On Wed, 17 Dec 2025 at 09:46, Jonathan Wakely <[email protected]> wrote:
>
> On Wed, 17 Dec 2025 at 08:56, Tomasz Kamiński <[email protected]> wrote:
> >
> > The r16-6177-g866bc8a9214b1d introduced type-constraint on _Urbg template
> > parameter in __glibcxx_concepts, with was inconsistent with declaration in
> > bits/random.h and definition in bits/random.tcc causing the missing symbol
> > errors in tests.
> >
> > Furthermore, this made the mangled name of generate_canonical in C++20
> > mode different from older standard and previous versions of GCC.
>
> Which isn't really a problem since it's a function template, so old
> code has its own instantiation of the old definition.
>
> In fact, maybe it's better to have a different mangled name, so that
> the new definition is a distinct symbol and the linker won't choose a
> symbol generated from the old definition, giving surprising results in
> a program where some TUs are compiled by GCC 16 and some TUs are
> compiled by GCC 15. If all calls to generate_canonical use the old
> definition, even for TUs compiled with GCC 16, that would be
> surprising.
>
> So I agree that we need the declaration and definition to match
> (obviously!) and that we should not use a type constraint on the new
> definition (the standard doesn't say it has any Constraints, so
> there's no reason to add that, the static_assert is better). But I
> think we should put the new definition in an inline namespace.
>
> In <chrono> we do something similar for system_clock and steady_clock:
>
> _GLIBCXX_BEGIN_INLINE_ABI_NAMESPACE(_V2)
> struct system_clock
> ...
> _GLIBCXX_END_INLINE_ABI_NAMESPACE(_V2)
>
> also in <condition_variable> and <bits/algo.h>.
>
> We should do that for the new generate_canonical, ensuring that the
> declaration in <bits/random.h> matches it, like so:
>
> --- a/libstdc++-v3/include/bits/random.h
> +++ b/libstdc++-v3/include/bits/random.h
> @@ -51,6 +51,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
> // std::uniform_random_bit_generator is defined in <bits/uniform_int_dist.h>
>
> +#ifndef _GLIBCXX_USE_OLD_GENERATE_CANONICAL
> +_GLIBCXX_BEGIN_INLINE_ABI_NAMESPACE(_V2)
> +#endif
> /**
> * @brief A function template for converting the output of a (integral)
> * uniform random number generator to a floatng point result in the range
> @@ -60,6 +63,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> typename _UniformRandomNumberGenerator>
> _RealType
> generate_canonical(_UniformRandomNumberGenerator& __g);
> +#ifndef _GLIBCXX_USE_OLD_GENERATE_CANONICAL
> +_GLIBCXX_END_INLINE_ABI_NAMESPACE(_V2)
> +#endif
>
> /// @cond undocumented
> // Implementation-space details.
>
>
>
>
> >
> > libstdc++-v3/ChangeLog:
> >
> > * include/bits/random.tcc (generate_canonical)
> > [!_GLIBCXX_USE_OLD_GENERATE_CANONICAL]: Use static_assert
> > instead of type-constraint on template parameter.
> > ---
> > The missing error was much more prosaic in their root cause, than I
> > have originally suspected.
> >
> > Testing on x86_64-linux. OK for trunk?
Please push this fix, then we can add the inline namespace separately.
> >
> > libstdc++-v3/include/bits/random.tcc | 11 ++++-------
> > 1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/libstdc++-v3/include/bits/random.tcc
> > b/libstdc++-v3/include/bits/random.tcc
> > index 38e8645c88c..d0aed028ed6 100644
> > --- a/libstdc++-v3/include/bits/random.tcc
> > +++ b/libstdc++-v3/include/bits/random.tcc
> > @@ -3619,12 +3619,6 @@ namespace __detail
> > template <> const bool __is_rand_dist_float_v<long double> = true;
> > #endif
> >
> > -#ifdef __glibcxx_concepts
> > -# define _Uniform_random_bit_generator uniform_random_bit_generator
> > -#else
> > -# define _Uniform_random_bit_generator typename
> > -#endif
> > -
> > // Note, this works even when (__range + 1) overflows:
> > template <typename _Rng>
> > constexpr bool __is_power_of_2_less_1(_Rng __range)
> > @@ -3646,10 +3640,13 @@ namespace __detail
> > * @since C++11
> > */
> > template<typename _RealT, size_t __digits,
> > - _Uniform_random_bit_generator _Urbg>
> > + typename _Urbg>
> > _RealT
> > generate_canonical(_Urbg& __urng)
> > {
> > +#ifdef __glibcxx_concepts
> > + static_assert(uniform_random_bit_generator<_Urbg>);
> > +#endif
> > static_assert(__is_rand_dist_float_v<_RealT>,
> > "template argument must be floating point");
> > static_assert(__digits != 0 && _Urbg::max() > _Urbg::min(),
> > --
> > 2.52.0
> >
