Re: [PATCH] RFA: Add a small indication to warnings that are promoted to errors

[Simon Baldwin <simonb at google dot com>]

2009/11/20 Richard Guenther <richard.guenther@gmail.com>:
> On Fri, Nov 20, 2009 at 4:13 PM, Simon Baldwin <simonb@google.com> wrote:
>> This small patch adds an indication "[was warning]" to gcc diagnostics where
>> warning messages are promoted to errors with -Werror.
>>
>> This helps to resolve possible confusion regarding diagnostics. ?For example,
>> if the following code is compiled with -Werror, both lines are currently
>> flagged as errors by gcc, with no hint that the first is not a real program
>> error:
>>
>> ?extern int foo = 42; ?// gcc error: 'foo' initialized and declared 'extern'
>> ? ? ? ? ? ? ? ? ? ? ? ?// okay: ISO/IEC 9899:1999, 6.9.2-4
>> ?static int bar = foo; // gcc error: initializer element is not constant
>> ? ? ? ? ? ? ? ? ? ? ? ?// error: ISO/IEC 9899:1999, 6.7.8-4
>>
>> Confirmed C dejagnu testsuite parity with the unpatched gcc, and verified
>> bootstrap of C on x86_64.
>
> We do say
>
> cc1: warnings being treated as errors

We say it for the overall -Werror, but not for more targeted
-Werror=sign-conversion, or similar specific -Werror= settings in the
absence of any overall -Werror.

Also, the "warnings being treated as errors" message, I've found,
tends to get overlooked.  Especially where there may be a lot (and
there can be an awful lot in C++) of other diagnostics between it and
the one of interest.


>
> though. ?Did you consider instead of appending [was warning] to
> simply make it print
>
> t.c:1: error: warning: ‘foo’ initialized and declared ‘extern’
>
> which would be consistent - the error is that there is a warning.

I discussed this patch off-list with Gaby before sending it out, and
this was one of my suggestions.  Gaby suggested the trailing "[was
warning]" instead.

Thanks,

--S


>>
>> gcc/ChangeLog:
>> 2009-11-19 ?Simon Baldwin ?<simonb@google.com>
>>
>> ? ? ? ?* diagnostic.c (diagnostic_report_diagnostic): Append '[was warning]'
>> ? ? ? ?to diagnostic's format if it is a promoted warning.
>>
>> gcc/testsuite/ChangeLog:
>> 2009-11-19 ?Simon Baldwin ?<simonb@google.com>
>>
>> ? ? ? ?* gcc.dg/was-warning-1.c: New.
>> ? ? ? ?* gcc.dg/was-warning-2.c: New.
>> ? ? ? ?* gcc.dg/was-warning-3.c: New.
>>
>>
>> Index: gcc/diagnostic.c
>> ===================================================================
>> --- gcc/diagnostic.c ? ?(revision 154244)
>> +++ gcc/diagnostic.c ? ?(working copy)
>> @@ -309,6 +309,7 @@ diagnostic_report_diagnostic (diagnostic
>> ?{
>> ? location_t location = diagnostic->location;
>> ? bool maybe_print_warnings_as_errors_message = false;
>> + ?bool is_warning_as_error = false;
>> ? const char *saved_format_spec;
>>
>> ? /* Give preference to being able to inhibit warnings, before they
>> @@ -322,7 +323,7 @@ diagnostic_report_diagnostic (diagnostic
>>
>> ? if (diagnostic->kind == DK_PEDWARN)
>> ? ? diagnostic->kind = pedantic_warning_kind ();
>> -
>> +
>> ? if (context->lock > 0)
>> ? ? {
>> ? ? ? /* If we're reporting an ICE in the middle of some other error,
>> @@ -357,6 +358,7 @@ diagnostic_report_diagnostic (diagnostic
>> ? ? ? ?{
>> ? ? ? ? ?diagnostic->kind = context->classify_diagnostic[diagnostic->option_index];
>> ? ? ? ? ?maybe_print_warnings_as_errors_message = false;
>> + ? ? ? ? is_warning_as_error = (diagnostic->kind == DK_ERROR);
>> ? ? ? ?}
>> ? ? ? /* This allows for future extensions, like temporarily disabling
>> ? ? ? ? warnings for ranges of source code. ?*/
>> @@ -407,6 +409,10 @@ diagnostic_report_diagnostic (diagnostic
>> ? ++diagnostic_kind_count (context, diagnostic->kind);
>>
>> ? saved_format_spec = diagnostic->message.format_spec;
>> + ?if (maybe_print_warnings_as_errors_message || is_warning_as_error)
>> + ? ?diagnostic->message.format_spec
>> + ? ? ?= ACONCAT ((diagnostic->message.format_spec, _(" [was warning]"), NULL));
>> +
>> ? if (context->show_option_requested && diagnostic->option_index)
>> ? ? diagnostic->message.format_spec
>> ? ? ? = ACONCAT ((diagnostic->message.format_spec,
>> Index: gcc/testsuite/gcc.dg/was-warning-1.c
>> ===================================================================
>> --- gcc/testsuite/gcc.dg/was-warning-1.c ? ? ? ?(revision 0)
>> +++ gcc/testsuite/gcc.dg/was-warning-1.c ? ? ? ?(revision 0)
>> @@ -0,0 +1,7 @@
>> +// { dg-do compile }
>> +// { dg-options "-Werror -Wsign-conversion -Wno-error=sign-conversion" }
>> +// { dg-message "warnings being treated as errors" "" { target "*-*-*" } 0 }
>> +
>> +extern int x = 0; ?// { dg-error "'x' initialized and declared 'extern' .was warning." }
>> +unsigned y = -1; ? // { dg-warning "negative integer implicitly converted to unsigned type" }
>> +int z = x; ? ? ? ? // { dg-error "initializer element is not constant" }
>> Index: gcc/testsuite/gcc.dg/was-warning-2.c
>> ===================================================================
>> --- gcc/testsuite/gcc.dg/was-warning-2.c ? ? ? ?(revision 0)
>> +++ gcc/testsuite/gcc.dg/was-warning-2.c ? ? ? ?(revision 0)
>> @@ -0,0 +1,6 @@
>> +// { dg-do compile }
>> +// { dg-options "-Werror=sign-conversion" }
>> +
>> +extern int x = 0; ?// { dg-warning "'x' initialized and declared 'extern'" }
>> +unsigned y = -1; ? // { dg-error "negative integer implicitly converted to unsigned type .was warning." }
>> +int z = x; ? ? ? ? // { dg-error "initializer element is not constant" }
>> Index: gcc/testsuite/gcc.dg/was-warning-3.c
>> ===================================================================
>> --- gcc/testsuite/gcc.dg/was-warning-3.c ? ? ? ?(revision 0)
>> +++ gcc/testsuite/gcc.dg/was-warning-3.c ? ? ? ?(revision 0)
>> @@ -0,0 +1,7 @@
>> +// { dg-do compile }
>> +// { dg-options "-Werror -Wsign-conversion" }
>> +// { dg-message "warnings being treated as errors" "" { target "*-*-*" } 0 }
>> +
>> +extern int x = 0; ?// { dg-error "'x' initialized and declared 'extern' .was warning." }
>> +unsigned y = -1; ? // { dg-error "negative integer implicitly converted to unsigned type .was warning." }
>> +int z = x; ? ? ? ? // { dg-error "initializer element is not constant" }
>>
>



-- 
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902