| Message ID | ffce06a5-59a8-56fd-b39e-a2bd38c609a3@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [_GLIBCXX_INLINE_VERSION] Fix <format> friend declarations | expand |
Hi Any news regarding this problem ? François On 23/08/2023 19:35, François Dumont wrote: > Hi > > The few tests that are failing in versioned namespace mode are due to > those friend declarations. > > This is a fix proposal even if I considered 2 other options: > > 1. Make __format::_Arg_store a struct and so do not bother with friend > declarations. > > 2. Consider it as a compiler bug and do nothing. In this case I think > we might still need this patch to avoid a non-working format library > in versioned namespace mode in gcc 14 if compiler bug is not fixed. > > I can also define _GLIBCXX_STD_V at <format> level to limit impact. > > libstdc++: [_GLIBCXX_INLINE_VERSION] Fix <format> friend declarations > > GCC do not consider the inline namespace in friend declarations. > We need > to explicit this namespace. > > libstdc++-v3/ChangeLog: > > * include/bits/c++config (_GLIBCXX_STD_V): New macro > giving current > std namespace with optionally the version namespace. > * include/std/format (std::__format::_Arg_store): Use > latter on friend > declarations. > > Tested under versioned mode. > > Ok to commit ? > > François
On Wed, 23 Aug 2023 at 18:35, François Dumont via Libstdc++ <libstdc++@gcc.gnu.org> wrote: > > Hi > > The few tests that are failing in versioned namespace mode are due to > those friend declarations. > > This is a fix proposal even if I considered 2 other options: > > 1. Make __format::_Arg_store a struct and so do not bother with friend > declarations. > > 2. Consider it as a compiler bug and do nothing. In this case I think we > might still need this patch to avoid a non-working format library in > versioned namespace mode in gcc 14 if compiler bug is not fixed. It definitely is a compiler bug, this is PR c++/59256. Please add a comment to the new macro definition, so we remember to remove it when it's not needed: #if _GLIBCXX_INLINE_VERSION // Needed because of PR c++/59526 # define _GLIBCXX_STD_V std::__8 #else # define _GLIBCXX_STD_V std #endif OK with that change, thanks. > > I can also define _GLIBCXX_STD_V at <format> level to limit impact. > > libstdc++: [_GLIBCXX_INLINE_VERSION] Fix <format> friend declarations > > GCC do not consider the inline namespace in friend declarations. We > need > to explicit this namespace. > > libstdc++-v3/ChangeLog: > > * include/bits/c++config (_GLIBCXX_STD_V): New macro giving > current > std namespace with optionally the version namespace. > * include/std/format (std::__format::_Arg_store): Use > latter on friend > declarations. > > Tested under versioned mode. > > Ok to commit ? > > François
On Tue, 12 Sept 2023 at 17:47, Jonathan Wakely <jwakely@redhat.com> wrote: > > On Wed, 23 Aug 2023 at 18:35, François Dumont via Libstdc++ > <libstdc++@gcc.gnu.org> wrote: > > > > Hi > > > > The few tests that are failing in versioned namespace mode are due to > > those friend declarations. > > > > This is a fix proposal even if I considered 2 other options: > > > > 1. Make __format::_Arg_store a struct and so do not bother with friend > > declarations. > > > > 2. Consider it as a compiler bug and do nothing. In this case I think we > > might still need this patch to avoid a non-working format library in > > versioned namespace mode in gcc 14 if compiler bug is not fixed. > > It definitely is a compiler bug, this is PR c++/59256. > > Please add a comment to the new macro definition, so we remember to > remove it when it's not needed: > > > #if _GLIBCXX_INLINE_VERSION > // Needed because of PR c++/59526 > # define _GLIBCXX_STD_V std::__8 > #else > # define _GLIBCXX_STD_V std > #endif > > > OK with that change, thanks. Actually, are you sure the friend std::basic_format_args declaration needs to change? I only see errors for the friend function, not the friend class. So this seems to fix it: --- a/libstdc++-v3/include/std/format +++ b/libstdc++-v3/include/std/format @@ -3437,7 +3437,13 @@ namespace __format template<typename _Ctx, typename... _Argz> friend auto - std::make_format_args(_Argz&&...) noexcept; +#if _GLIBCXX_INLINE_VERSION + // Needed for PR c++/59526 + std::__8:: +#else + std:: +#endif + make_format_args(_Argz&&...) noexcept; // For a sufficiently small number of arguments we only store values. // basic_format_args can get the types from the _Args pack. > > > > > > I can also define _GLIBCXX_STD_V at <format> level to limit impact. > > > > libstdc++: [_GLIBCXX_INLINE_VERSION] Fix <format> friend declarations > > > > GCC do not consider the inline namespace in friend declarations. We > > need > > to explicit this namespace. > > > > libstdc++-v3/ChangeLog: > > > > * include/bits/c++config (_GLIBCXX_STD_V): New macro giving > > current > > std namespace with optionally the version namespace. > > * include/std/format (std::__format::_Arg_store): Use > > latter on friend > > declarations. > > > > Tested under versioned mode. > > > > Ok to commit ? > > > > François
It's working and what's I've committed. Thanks On 12/09/2023 19:04, Jonathan Wakely wrote: > On Tue, 12 Sept 2023 at 17:47, Jonathan Wakely <jwakely@redhat.com> wrote: >> On Wed, 23 Aug 2023 at 18:35, François Dumont via Libstdc++ >> <libstdc++@gcc.gnu.org> wrote: >>> Hi >>> >>> The few tests that are failing in versioned namespace mode are due to >>> those friend declarations. >>> >>> This is a fix proposal even if I considered 2 other options: >>> >>> 1. Make __format::_Arg_store a struct and so do not bother with friend >>> declarations. >>> >>> 2. Consider it as a compiler bug and do nothing. In this case I think we >>> might still need this patch to avoid a non-working format library in >>> versioned namespace mode in gcc 14 if compiler bug is not fixed. >> It definitely is a compiler bug, this is PR c++/59256. >> >> Please add a comment to the new macro definition, so we remember to >> remove it when it's not needed: >> >> >> #if _GLIBCXX_INLINE_VERSION >> // Needed because of PR c++/59526 >> # define _GLIBCXX_STD_V std::__8 >> #else >> # define _GLIBCXX_STD_V std >> #endif >> >> >> OK with that change, thanks. > Actually, are you sure the friend std::basic_format_args declaration > needs to change? > > I only see errors for the friend function, not the friend class. So > this seems to fix it: > > --- a/libstdc++-v3/include/std/format > +++ b/libstdc++-v3/include/std/format > @@ -3437,7 +3437,13 @@ namespace __format > > template<typename _Ctx, typename... _Argz> > friend auto > - std::make_format_args(_Argz&&...) noexcept; > +#if _GLIBCXX_INLINE_VERSION > + // Needed for PR c++/59526 > + std::__8:: > +#else > + std:: > +#endif > + make_format_args(_Argz&&...) noexcept; > > // For a sufficiently small number of arguments we only store values. > // basic_format_args can get the types from the _Args pack. > > > > >> >>> I can also define _GLIBCXX_STD_V at <format> level to limit impact. >>> >>> libstdc++: [_GLIBCXX_INLINE_VERSION] Fix <format> friend declarations >>> >>> GCC do not consider the inline namespace in friend declarations. We >>> need >>> to explicit this namespace. >>> >>> libstdc++-v3/ChangeLog: >>> >>> * include/bits/c++config (_GLIBCXX_STD_V): New macro giving >>> current >>> std namespace with optionally the version namespace. >>> * include/std/format (std::__format::_Arg_store): Use >>> latter on friend >>> declarations. >>> >>> Tested under versioned mode. >>> >>> Ok to commit ? >>> >>> François
On Wed, 13 Sept 2023 at 21:47, François Dumont <frs.dumont@gmail.com> wrote: > > It's working and what's I've committed. Nice, thanks! > > Thanks > > On 12/09/2023 19:04, Jonathan Wakely wrote: > > On Tue, 12 Sept 2023 at 17:47, Jonathan Wakely <jwakely@redhat.com> wrote: > >> On Wed, 23 Aug 2023 at 18:35, François Dumont via Libstdc++ > >> <libstdc++@gcc.gnu.org> wrote: > >>> Hi > >>> > >>> The few tests that are failing in versioned namespace mode are due to > >>> those friend declarations. > >>> > >>> This is a fix proposal even if I considered 2 other options: > >>> > >>> 1. Make __format::_Arg_store a struct and so do not bother with friend > >>> declarations. > >>> > >>> 2. Consider it as a compiler bug and do nothing. In this case I think we > >>> might still need this patch to avoid a non-working format library in > >>> versioned namespace mode in gcc 14 if compiler bug is not fixed. > >> It definitely is a compiler bug, this is PR c++/59256. > >> > >> Please add a comment to the new macro definition, so we remember to > >> remove it when it's not needed: > >> > >> > >> #if _GLIBCXX_INLINE_VERSION > >> // Needed because of PR c++/59526 > >> # define _GLIBCXX_STD_V std::__8 > >> #else > >> # define _GLIBCXX_STD_V std > >> #endif > >> > >> > >> OK with that change, thanks. > > Actually, are you sure the friend std::basic_format_args declaration > > needs to change? > > > > I only see errors for the friend function, not the friend class. So > > this seems to fix it: > > > > --- a/libstdc++-v3/include/std/format > > +++ b/libstdc++-v3/include/std/format > > @@ -3437,7 +3437,13 @@ namespace __format > > > > template<typename _Ctx, typename... _Argz> > > friend auto > > - std::make_format_args(_Argz&&...) noexcept; > > +#if _GLIBCXX_INLINE_VERSION > > + // Needed for PR c++/59526 > > + std::__8:: > > +#else > > + std:: > > +#endif > > + make_format_args(_Argz&&...) noexcept; > > > > // For a sufficiently small number of arguments we only store values. > > // basic_format_args can get the types from the _Args pack. > > > > > > > > > >> > >>> I can also define _GLIBCXX_STD_V at <format> level to limit impact. > >>> > >>> libstdc++: [_GLIBCXX_INLINE_VERSION] Fix <format> friend declarations > >>> > >>> GCC do not consider the inline namespace in friend declarations. We > >>> need > >>> to explicit this namespace. > >>> > >>> libstdc++-v3/ChangeLog: > >>> > >>> * include/bits/c++config (_GLIBCXX_STD_V): New macro giving > >>> current > >>> std namespace with optionally the version namespace. > >>> * include/std/format (std::__format::_Arg_store): Use > >>> latter on friend > >>> declarations. > >>> > >>> Tested under versioned mode. > >>> > >>> Ok to commit ? > >>> > >>> François >
On Wed, 13 Sept 2023 at 21:50, Jonathan Wakely <jwakely@redhat.com> wrote: > > On Wed, 13 Sept 2023 at 21:47, François Dumont <frs.dumont@gmail.com> wrote: > > > > It's working and what's I've committed. > > Nice, thanks! > > > > > > Thanks > > > > On 12/09/2023 19:04, Jonathan Wakely wrote: > > > On Tue, 12 Sept 2023 at 17:47, Jonathan Wakely <jwakely@redhat.com> wrote: > > >> On Wed, 23 Aug 2023 at 18:35, François Dumont via Libstdc++ > > >> <libstdc++@gcc.gnu.org> wrote: > > >>> Hi > > >>> > > >>> The few tests that are failing in versioned namespace mode are due to > > >>> those friend declarations. > > >>> > > >>> This is a fix proposal even if I considered 2 other options: > > >>> > > >>> 1. Make __format::_Arg_store a struct and so do not bother with friend > > >>> declarations. > > >>> > > >>> 2. Consider it as a compiler bug and do nothing. In this case I think we > > >>> might still need this patch to avoid a non-working format library in > > >>> versioned namespace mode in gcc 14 if compiler bug is not fixed. > > >> It definitely is a compiler bug, this is PR c++/59256. > > >> > > >> Please add a comment to the new macro definition, so we remember to > > >> remove it when it's not needed: > > >> > > >> > > >> #if _GLIBCXX_INLINE_VERSION > > >> // Needed because of PR c++/59526 > > >> # define _GLIBCXX_STD_V std::__8 > > >> #else > > >> # define _GLIBCXX_STD_V std > > >> #endif > > >> > > >> > > >> OK with that change, thanks. > > > Actually, are you sure the friend std::basic_format_args declaration > > > needs to change? > > > > > > I only see errors for the friend function, not the friend class. So > > > this seems to fix it: > > > > > > --- a/libstdc++-v3/include/std/format > > > +++ b/libstdc++-v3/include/std/format > > > @@ -3437,7 +3437,13 @@ namespace __format > > > > > > template<typename _Ctx, typename... _Argz> > > > friend auto > > > - std::make_format_args(_Argz&&...) noexcept; > > > +#if _GLIBCXX_INLINE_VERSION > > > + // Needed for PR c++/59526 Except that should be 59256 :-( My fault, I'll fix it. > > > + std::__8:: > > > +#else > > > + std:: > > > +#endif > > > + make_format_args(_Argz&&...) noexcept; > > > > > > // For a sufficiently small number of arguments we only store values. > > > // basic_format_args can get the types from the _Args pack. > > > > > > > > > > > > > > >> > > >>> I can also define _GLIBCXX_STD_V at <format> level to limit impact. > > >>> > > >>> libstdc++: [_GLIBCXX_INLINE_VERSION] Fix <format> friend declarations > > >>> > > >>> GCC do not consider the inline namespace in friend declarations. We > > >>> need > > >>> to explicit this namespace. > > >>> > > >>> libstdc++-v3/ChangeLog: > > >>> > > >>> * include/bits/c++config (_GLIBCXX_STD_V): New macro giving > > >>> current > > >>> std namespace with optionally the version namespace. > > >>> * include/std/format (std::__format::_Arg_store): Use > > >>> latter on friend > > >>> declarations. > > >>> > > >>> Tested under versioned mode. > > >>> > > >>> Ok to commit ? > > >>> > > >>> François > >
diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config index 0a41cdd29a9..a917fb58225 100644 --- a/libstdc++-v3/include/bits/c++config +++ b/libstdc++-v3/include/bits/c++config @@ -449,6 +449,7 @@ _GLIBCXX_END_NAMESPACE_VERSION // of some nested namespace within it corresponding to the active mode. // _GLIBCXX_STD_A // _GLIBCXX_STD_C +// _GLIBCXX_STD_V // // Macros for opening/closing conditional namespaces. // _GLIBCXX_BEGIN_NAMESPACE_ALGO @@ -477,6 +478,12 @@ _GLIBCXX_END_NAMESPACE_VERSION # define _GLIBCXX_END_NAMESPACE_ALGO #endif +#if _GLIBCXX_INLINE_VERSION +# define _GLIBCXX_STD_V std::__8 +#else +# define _GLIBCXX_STD_V std +#endif + // GLIBCXX_ABI Deprecated // Define if compatibility should be provided for -mlong-double-64. #undef _GLIBCXX_LONG_DOUBLE_COMPAT diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format index f3d9ae152f9..94417c321e4 100644 --- a/libstdc++-v3/include/std/format +++ b/libstdc++-v3/include/std/format @@ -3429,11 +3429,11 @@ namespace __format template<typename _Context, typename... _Args> class __format::_Arg_store { - friend std::basic_format_args<_Context>; + friend _GLIBCXX_STD_V::basic_format_args<_Context>; template<typename _Ctx, typename... _Argz> friend auto - std::make_format_args(_Argz&&...) noexcept; + _GLIBCXX_STD_V::make_format_args(_Argz&&...) noexcept; // For a sufficiently small number of arguments we only store values. // basic_format_args can get the types from the _Args pack.