Add macros for diagnostic control, use for scanf %a tests

[Differences relative to version 1
<https://sourceware.org/ml/libc-alpha/2014-11/msg00459.html>: macros
are now named DIAG_*_NEEDS_COMMENT; the option name is now a string
argument to the macro rather than being stringized; the GCC version
number is now the first argument to DIAG_IGNORE_NEEDS_COMMENT rather
than the second, to reduce the risk that greppability would require
long source lines given the new longer macro name; the example uses of
the macros are now in scanf tests.]

In <https://sourceware.org/ml/libc-alpha/2014-11/msg00326.html>,
Roland requested internal macros for use of "#pragma GCC diagnostic".

This patch adds such macros and uses them to disable -Wformat warnings
for some code testing GNU scanf %as where GCC expects C99 scanf %a
(several other stdio tests currently use -Wno-format to disable
warnings).  Limitations in GCC's diagnostic pragmas require separate
macros before and after the code generating the warnings, rather than
a single macro taking that code as an argument.

The macros are named DIAG_*_NEEDS_COMMENT to emphasise to reviewers
the need for a comment accompanying any use of them (such comments may
however just appear once for several uses of the macros for the same
issue in the same file).  I put a GCC version in the arguments to
DIAG_IGNORE_NEEDS_COMMENT, as that seems something useful to grep for
when obsoleting support for an old GCC version and needing to decide
if warning-disabling code is still relevant.

These macros should be usable for replacing existing -Wno-* use in
makefiles (as also suggested by Roland), though I have no plans to
work on that (only on use of the macros in cases where warnings are
currently present that need disabling to use -Werror).

Tested for x86_64.

2014-11-25  Joseph Myers  <joseph@codesourcery.com>

	* include/libc-internal.h (DIAG_PUSH_NEEDS_COMMENT): New macro.
	(DIAG_POP_NEEDS_COMMENT): Likewise.
	(_DIAG_STR1): Likewise.
	(_DIAG_STR): Likewise.
	(DIAG_IGNORE_NEEDS_COMMENT): Likewise.
	* stdio-common/bug21.c: Include <libc-internal.h>.
	(do_test): Disable -Wformat around call to sscanf.
	* stdio-common/scanf14.c: Include <libc-internal.h>.
	(main): Disable -Wformat around some calls to scanf functions.

Joseph Myers wrote:
> +  /* GCC in C99 mode treats %a as the C99 format expecting float *,
> +     but glibc with _GNU_SOURCE treats %as as the GNU allocation
> +     extension, so resulting in "warning: format '%a' expects argument
> +     of type 'float *', but argument 3 has type 'char **'".  This
> +     applies to the other %as, %aS and %a[] formats below as well.  */
> +  DIAG_PUSH_NEEDS_COMMENT;
> +  DIAG_IGNORE_NEEDS_COMMENT (4.9, "-Wformat");
>     if (sscanf (" 1.25s x", "%as%2c", &sp, c) != 2)
>       FAIL ();
>     else
> @@ -32,10 +40,14 @@ main (void)
>         memset (sp, 'x', sizeof "1.25s");
>         free (sp);
>       }
> +  DIAG_POP_NEEDS_COMMENT;


This sort of thing looks intrusive.  Instead, how about defining a macro 
'C99_sscanf' that does the push and pop and commentary, and then simply 
replacing warned-about uses of sscanf with C99_scanf?  That'd be a lot easier to 
read, and it's more along the lines of Roland's original suggestion.

On Tue, 25 Nov 2014, Paul Eggert wrote:

> This sort of thing looks intrusive.  Instead, how about defining a macro
> 'C99_sscanf' that does the push and pop and commentary, and then simply
> replacing warned-about uses of sscanf with C99_scanf?  That'd be a lot easier
> to read, and it's more along the lines of Roland's original suggestion.

Again, it doesn't work to have the pragmas in the same macro as the code 
in which the diagnostics are suppressed; they need to be in separate 
macros for the suppression to be effective.

Joseph Myers wrote:
> it doesn't work to have the pragmas in the same macro as the code

Then how about using an inline function instead of a macro?  Something like this:

static inline __attribute__ ((__always_inline__)) int
C99_sscanf (char const *str, char const *format, ...)
{
   return sscanf (str, format, __builtin_va_arg_pack ());
}

This should silence the warning more accurately than the pragma does.

Similarly, if we're worried about fread_unlocked complaining about a zero-sized 
argument, perhaps we can write an inline wrapper for it; that should silence thw 
warning more accurately.

One might reasonably object that I'm going off the deep end here: after all, 
it's just a test case and we needn't care all that much about narrowly tailoring 
warning-suppression for test cases.  I'm sympathetic to that objection.  But in 
that case, why not just disable the warning for the entire test-case compilation 
unit?  That'd be simpler yet.  There'd be no need for pragma macros or for 
pushing or popping diagnostics, and it'd be simpler for others to follow what's 
going on.

I'm sorry if I appear to be niggling here, it's just that we've had a lot of 
experience dodging these warnings in GNU applications ...

On Wed, 26 Nov 2014, Paul Eggert wrote:

> Joseph Myers wrote:
> > it doesn't work to have the pragmas in the same macro as the code
> 
> Then how about using an inline function instead of a macro?  Something like
> this:
> 
> static inline __attribute__ ((__always_inline__)) int
> C99_sscanf (char const *str, char const *format, ...)
> {
>   return sscanf (str, format, __builtin_va_arg_pack ());
> }

I'm doubtful about hiding warnings in ways that rely on the compiler not 
being smart enough to see through them - it would be perfectly reasonable 
for the compiler to warn in this case by doing format checking after 
optimization.  Warnings should be disabled in ways that clearly make it 
incorrect for the compiler to warn.

> Similarly, if we're worried about fread_unlocked complaining about a
> zero-sized argument, perhaps we can write an inline wrapper for it; that
> should silence thw warning more accurately.

An inline wrapper would not be appropriate; these tests are specifically 
using arguments with side-effects, and arguments with unusual types, to 
verify that those work with the macro definitions the same way as they 
would work with a normal function call (i.e. they are evaluated at least 
once, and the floating-point arguments are converted to the correct 
argument types).  Using an inline wrapper would defect this testing.

Joseph Myers wrote:

> I'm doubtful about hiding warnings in ways that rely on the compiler not
> being smart enough to see through them ...
> An inline wrapper would not be appropriate; these tests are specifically
> using arguments with side-effects, and arguments with unusual types

Fair enough on both points, but this underscores the suggestion that for test 
cases let's do just put a simple pragma at the start of the file.  To grep it 
later we can use a stylized comment, like this:

/* (GCC 4.9) gcc -Werror -Wformat fails in C99 mode because ... */
#pragma GCC diagnostic ignored "-Wformat"

as it's not worth our time (or our readers' time) to get fancier on test cases.

Ping.  This patch 
<https://sourceware.org/ml/libc-alpha/2014-11/msg00736.html> is pending 
review.  Also pending review and depending on it are three other warning 
suppression patches 
<https://sourceware.org/ml/libc-alpha/2014-11/msg00752.html>, 
<https://sourceware.org/ml/libc-alpha/2014-11/msg00785.html> and 
<https://sourceware.org/ml/libc-alpha/2014-11/msg00791.html>, and the 
actual -Werror patch 
<https://sourceware.org/ml/libc-alpha/2014-11/msg00798.html>.

Ping^2.  This patch 
<https://sourceware.org/ml/libc-alpha/2014-11/msg00736.html> is still 
pending review.  If the use for the %a tests is controversial, review of 
the macro definitions on their own would still be helpful.

That's OK by me.  But it would be better if the explanations about how GCC
limitations influenced the macro API choices were in comments in the header
itself.

On Tue, 9 Dec 2014, Roland McGrath wrote:

> That's OK by me.  But it would be better if the explanations about how GCC
> limitations influenced the macro API choices were in comments in the header
> itself.

I've committed the patch with this comment.

/* The macros to control diagnostics are structured like this, rather
   than a single macro that both pushes and pops diagnostic state and
   takes the affected code as an argument, because the GCC pragmas
   work by disabling the diagnostic for a range of source locations
   and do not work when all the pragmas and the affected code are in a
   single macro expansion.  */