[PATCH BZ#19239] Improve deprecation warnings

[PATCH BZ#19239] Improve deprecation warnings

[Zack Weinberg]

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?

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.  */
+#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)

Re: [PATCH BZ#19239] Improve deprecation warnings

[Adhemerval Zanella]

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)

Re: [PATCH BZ#19239] Improve deprecation warnings

[Zack Weinberg]

On Thu, Oct 13, 2016 at 5:40 PM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> 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.
>
> 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.

Yes, I ran the test suite on x86_64-linux and I manually verified the
test case in #19329.

>> +/* 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?

It will work correctly with any GCC release (that supports #pragma GCC
message), it just might not be word-wrapped as neatly.

After thinking about it some more, the fragility is not about
whitespace, it's about tokens.  I have changed the comment to read

/* Caution: The text of this deprecation message is unquoted, so that
   #symbol can be substituted.  (It is converted to a string by
   __SYSMACROS_DM1.)  This means the message must be a sequence of
   complete pp-tokens; in particular, English contractions (it's,
   can't) cannot be used.

   The message has been manually word-wrapped to fit in 80 columns
   when output by GCC 5 and 6.  The first line is shorter to leave
   some room for the "foo.c:23: warning:" annotation.  */

and I've also added a note to cdefs.h above the definition of
__glibc_macro_warning

/* __glibc_macro_warning (MESSAGE) issues warning MESSAGE.  This is
   intended for use in preprocessor macros.

   Note: MESSAGE must be a _single_ string; concatenation of string
   literals is not supported.  */

and I'm going to go ahead and check it in like that.

zw