Re: [PATCH 1/2] libstdc++: Normalise _GLIBCXX20_DEPRECATED macro

[Jonathan Wakely <jwakely@redhat.com>]

On Wed, 28 Dec 2022 at 14:28, Nathaniel Shead via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> These two patches implement P1413 (deprecate std::aligned_storage and
> std::aligned_union) for C++23. Tested on x86_64-linux.
>
> -- >8 --
>
> Updates _GLIBCXX20_DEPRECATED to be defined and behave the same as the
> versions for other standards (e.g. _GLIBCXX17_DEPRECATED).
>
> libstdc++-v3/ChangeLog:
>
>         * doc/doxygen/user.cfg.in (PREDEFINED): Update macros.
>         * include/bits/c++config (_GLIBCXX20_DEPRECATED): Make
>         consistent with other 'deprecated' macros.
>         * include/std/type_traits (is_pod, is_pod_v): Use
>         _GLIBCXX20_DEPRECATED_SUGGEST instead.
>
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>  libstdc++-v3/doc/doxygen/user.cfg.in | 4 ++--
>  libstdc++-v3/include/bits/c++config  | 6 +++---
>  libstdc++-v3/include/std/type_traits | 4 ++--
>  3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/libstdc++-v3/doc/doxygen/user.cfg.in b/libstdc++-v3/doc/doxygen/user.cfg.in
> index 834ad9e4fd5..fc46e722529 100644
> --- a/libstdc++-v3/doc/doxygen/user.cfg.in
> +++ b/libstdc++-v3/doc/doxygen/user.cfg.in
> @@ -2394,8 +2394,8 @@ PREDEFINED             = __cplusplus=202002L \
>                           "_GLIBCXX11_DEPRECATED_SUGGEST(E)= " \
>                           "_GLIBCXX17_DEPRECATED= " \
>                           "_GLIBCXX17_DEPRECATED_SUGGEST(E)= " \
> -                         "_GLIBCXX20_DEPRECATED(E)= " \
> -                         "_GLIBCXX20_DEPRECATED(E)= " \
> +                         "_GLIBCXX20_DEPRECATED= " \
> +                         "_GLIBCXX20_DEPRECATED_SUGGEST(E)= " \

Oops, good catch, that should definitely be fixed.

>                           _GLIBCXX17_INLINE=inline \
>                           _GLIBCXX_CHRONO_INT64_T=int64_t \
>                           _GLIBCXX_DEFAULT_ABI_TAG \
> diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
> index 50406066afe..d2b0cfa15ce 100644
> --- a/libstdc++-v3/include/bits/c++config
> +++ b/libstdc++-v3/include/bits/c++config
> @@ -84,7 +84,7 @@
>  //   _GLIBCXX14_DEPRECATED_SUGGEST( string-literal )
>  //   _GLIBCXX17_DEPRECATED
>  //   _GLIBCXX17_DEPRECATED_SUGGEST( string-literal )
> -//   _GLIBCXX20_DEPRECATED( string-literal )
> +//   _GLIBCXX20_DEPRECATED
>  //   _GLIBCXX20_DEPRECATED_SUGGEST( string-literal )
>  #ifndef _GLIBCXX_USE_DEPRECATED
>  # define _GLIBCXX_USE_DEPRECATED 1
> @@ -124,10 +124,10 @@
>  #endif
>
>  #if defined(__DEPRECATED) && (__cplusplus >= 202002L)
> -# define _GLIBCXX20_DEPRECATED(MSG) [[deprecated(MSG)]]
> +# define _GLIBCXX20_DEPRECATED [[__deprecated__]]
>  # define _GLIBCXX20_DEPRECATED_SUGGEST(ALT) _GLIBCXX_DEPRECATED_SUGGEST(ALT)
>  #else
> -# define _GLIBCXX20_DEPRECATED(MSG)
> +# define _GLIBCXX20_DEPRECATED

I think this inconsistency was actually deliberate...

>  # define _GLIBCXX20_DEPRECATED_SUGGEST(ALT)
>  #endif
>
> diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
> index 5dc9e1b2921..2f4d4bb8d4d 100644
> --- a/libstdc++-v3/include/std/type_traits
> +++ b/libstdc++-v3/include/std/type_traits
> @@ -815,7 +815,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>    // Could use is_standard_layout && is_trivial instead of the builtin.
>    template<typename _Tp>
>      struct
> -    _GLIBCXX20_DEPRECATED("use is_standard_layout && is_trivial instead")
> +    _GLIBCXX20_DEPRECATED_SUGGEST("is_standard_layout && is_trivial")

This doesn't quite work. The SUGGEST macro will enclose its argument
in single quotes, so here we get:

is_pod is deprecated, use 'is_standard_layout && is_trivial' instead.

That makes it look like 'is_standard_layout && is_trivial' is a piece
of valid code, but it's not. It would need to use the variable
templates not the class templates, and would need template argument
lists:

is_pod is deprecated, use 'is_standard_layout_v<T> && is_trivial_v<T>' instead.

Then we have the question of whether it's OK to use 'T' here when that
is neither the template parameter in the <type_traits> header (that's
'_Tp') nor the user's code (that will be some class type that we can't
know in the attribute's string-literal).

So I think my preference would be to leave _GLIBCXX20_DEPRECATED as
is, and always require a message. If possible, we should always say
something user-friendly, e.g. for std::aligned_storage we could use
something like:

_GLIBCXX23_DEPRECATED("use an aligned array of bytes instead")

We don't want to use the SUGGEST macro here because we don't want the
single quotes. But I don't see an obvious message we could use for
std::aligned_union ... so because I'm too lazy to spend any longer
thinking about it, I think I'll just merge both your patches. Thanks
for contributing them!