Re: [PATCH BZ#19239] Improve deprecation warnings

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 11/10/2016 10:09, Zack Weinberg wrote:
> By using __glibc_macro_warning instead of __attribute_deprecated__,
> we get the deprecation warnings whenever the macros are expanded, not
> just when they compile to a function call.  This is important for
> unintentional uses like the test case in #19239.  It's also simpler,
> because __REDIRECT is no longer required.
> 
> __SYSMACROS_DM(1) is tricksy: by writing the full text of the
> deprecation message directly inside the call to __SYSMACROS_DM1,
> we prevent all those bare identifiers from being macro-expanded
> (because they're the immediate subject of stringification) while
> still being able to substitute ‘symbol’.  (Neither _Pragma itself
> nor #pragma GCC warning permits string-literal concatenation.)
> 
> OK?

I am assuming you tested some platform and verified that it actually
fixed the issue.  Patch looks ok with some comments about the new
macro definition.

> 
> zw
> 
> 
> 	BZ#19239
>         * misc/sys/cdefs.h (__glibc_macro_warning):
>         Use #pragma GCC warning for clang >=3.5 as well.
>         * misc/sys/sysmacros.h: Use __glibc_macro_warning instead of
>         __attribute_deprecated_msg__ to diagnose deprecated definitions via
>         sys/types.h.
> 
> diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
> index 50e00e6..49b0956 100644
> --- a/misc/sys/cdefs.h
> +++ b/misc/sys/cdefs.h
> @@ -474,7 +474,7 @@
> 
>  /* __glibc_macro_warning (MESSAGE) issues warning MESSAGE.  This is
>     intended for use in preprocessor macros.  */
> -#if __GNUC_PREREQ (4,8)
> +#if __GNUC_PREREQ (4,8) || __glibc_clang_prereq (3,5)
>  # define __glibc_macro_warning1(message) _Pragma (#message)
>  # define __glibc_macro_warning(message) \
>    __glibc_macro_warning1 (GCC warning message)
> diff --git a/misc/sys/sysmacros.h b/misc/sys/sysmacros.h
> index 086e9af..5600672 100644
> --- a/misc/sys/sysmacros.h
> +++ b/misc/sys/sysmacros.h
> @@ -40,54 +40,39 @@
>  #include <bits/types.h>
>  #include <bits/sysmacros.h>
> 
> -/* The extra "\n " moves gcc's [-Wdeprecated-declarations] annotation
> -   onto the next line.  */
> -#define __SYSMACROS_DEPRECATION_MSG(symbol)				     \
> -  "\n  In the GNU C Library, `" #symbol "' is defined by
> <sys/sysmacros.h>." \
> -  "\n  For historical compatibility, it is currently defined by"	     \
> -  "\n  <sys/types.h> as well, but we plan to remove this soon."		     \
> -  "\n  To use `" #symbol "', include <sys/sysmacros.h> directly."	     \
> -  "\n  If you did not intend to use a system-defined macro `" #symbol
> "',"   \
> -  "\n  you should #undef it after including <sys/types.h>."		     \
> -  "\n "
> +/* Caution: if you change the whitespace in and around this macro,
> +   it may malfunction.  Newline placement is tuned for GCC 6.  */

This comments is somewhat confusing, what does it mean to be 'tuned'
for GCC 6? It is just that stylist it will be better formatted on
GCC 6? Will it work correctly on older GCC releases? What kind of
whitespaces constraints do we need to be aware of?

> +#define __SYSMACROS_DM(symbol) __SYSMACROS_DM1 \
> + (In the GNU C Library, #symbol is defined\n\
> +  by <sys/sysmacros.h>. For historical compatibility, it is\n\
> +  currently defined by <sys/types.h> as well, but we plan to\n\
> +  remove this soon.  To use #symbol, include <sys/sysmacros.h>\n\
> +  directly.  If you did not intend to use a system-defined macro\n\
> +  #symbol, you should undefine it after including <sys/types.h>.)
> +
> +/* This macro is variadic because the deprecation message above
> +   contains commas.  */
> +#define __SYSMACROS_DM1(...) __glibc_macro_warning (#__VA_ARGS__)
> 
>  #define __SYSMACROS_DECL_TEMPL(rtype, name, proto)			     \
>    extern rtype gnu_dev_##name proto __THROW __attribute_const__;
> 
> -#define __SYSMACROS_FST_DECL_TEMPL(rtype, name, proto)			     \
> -  extern rtype __REDIRECT_NTH (__##name##_from_sys_types, proto,	     \
> -			       gnu_dev_##name)				     \
> -       __attribute_const__						     \
> -       __attribute_deprecated_msg__ (__SYSMACROS_DEPRECATION_MSG (name));
> -
>  #define __SYSMACROS_IMPL_TEMPL(rtype, name, proto)			     \
>    __extension__ __extern_inline __attribute_const__ rtype		     \
>    __NTH (gnu_dev_##name proto)
> 
> -#define __SYSMACROS_FST_IMPL_TEMPL(rtype, name, proto)			     \
> -  __extension__ __extern_inline __attribute_const__ rtype		     \
> -  __NTH (__##name##_from_sys_types proto)
> -
>  __BEGIN_DECLS
> 
>  __SYSMACROS_DECLARE_MAJOR (__SYSMACROS_DECL_TEMPL)
>  __SYSMACROS_DECLARE_MINOR (__SYSMACROS_DECL_TEMPL)
>  __SYSMACROS_DECLARE_MAKEDEV (__SYSMACROS_DECL_TEMPL)
> 
> -__SYSMACROS_DECLARE_MAJOR (__SYSMACROS_FST_DECL_TEMPL)
> -__SYSMACROS_DECLARE_MINOR (__SYSMACROS_FST_DECL_TEMPL)
> -__SYSMACROS_DECLARE_MAKEDEV (__SYSMACROS_FST_DECL_TEMPL)
> -
>  #ifdef __USE_EXTERN_INLINES
> 
>  __SYSMACROS_DEFINE_MAJOR (__SYSMACROS_IMPL_TEMPL)
>  __SYSMACROS_DEFINE_MINOR (__SYSMACROS_IMPL_TEMPL)
>  __SYSMACROS_DEFINE_MAKEDEV (__SYSMACROS_IMPL_TEMPL)
> 
> -__SYSMACROS_DEFINE_MAJOR (__SYSMACROS_FST_IMPL_TEMPL)
> -__SYSMACROS_DEFINE_MINOR (__SYSMACROS_FST_IMPL_TEMPL)
> -__SYSMACROS_DEFINE_MAKEDEV (__SYSMACROS_FST_IMPL_TEMPL)
> -
>  #endif
> 
>  __END_DECLS
> @@ -96,9 +81,7 @@ __END_DECLS
> 
>  #ifndef __SYSMACROS_NEED_IMPLEMENTATION
>  # undef __SYSMACROS_DECL_TEMPL
> -# undef __SYSMACROS_FST_DECL_TEMPL
>  # undef __SYSMACROS_IMPL_TEMPL
> -# undef __SYSMACROS_FST_IMPL_TEMPL
>  # undef __SYSMACROS_DECLARE_MAJOR
>  # undef __SYSMACROS_DECLARE_MINOR
>  # undef __SYSMACROS_DECLARE_MAKEDEV
> @@ -108,9 +91,9 @@ __END_DECLS
>  #endif
> 
>  #ifdef __SYSMACROS_DEPRECATED_INCLUSION
> -# define major(dev) __major_from_sys_types (dev)
> -# define minor(dev) __minor_from_sys_types (dev)
> -# define makedev(maj, min) __makedev_from_sys_types (maj, min)
> +# define major(dev) __SYSMACROS_DM (major) gnu_dev_major (dev)
> +# define minor(dev) __SYSMACROS_DM (minor) gnu_dev_minor (dev)
> +# define makedev(maj, min) __SYSMACROS_DM (makedev) gnu_dev_makedev
> (maj, min)
>  #else
>  # define major(dev) gnu_dev_major (dev)
>  # define minor(dev) gnu_dev_minor (dev)