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

[Simon Baldwin <simonb at google dot com>]

2009/11/22 Richard Guenther <richard.guenther@gmail.com>:
> On Sun, Nov 22, 2009 at 7:00 PM, Simon Baldwin <simonb@google.com> wrote:
>> 2009/11/20 Richard Guenther <richard.guenther@gmail.com>:
>>> On Fri, Nov 20, 2009 at 4:37 PM, Simon Baldwin <simonb@google.com> wrote:
>>>> 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.
>>>
>>> I don't like the suffix much - so much for the bikeshedding ;)
>>
>> Indeed.
>>
>> Attached below is an alternative version of this patch which
>> implements "error: warning:" prefixes for promoted warnings. ?How
>> about this one?
>
> I like the result, but I can't approve the patch.

Thanks for the vote.

I've just noted that this version drops the localization of prefixes.
I've fixed this by

-  static const char *const diagnostic_kind_text[] = {
+  static const char *const diagnostic_prefix[] = {
...
+  if (diagnostic->kind == DK_ERROR
+      && diagnostic->init_kind == DK_WARNING)
+    text = build_message_string ("%s%s",
+				 _(diagnostic_prefix[diagnostic->kind]),
+				 _(diagnostic_prefix[diagnostic->init_kind]));
+  else
+    text = build_message_string ("%s", _(diagnostic_prefix[diagnostic->kind]));

Lots of traps for the unwary even in simpler patches, aren't there?

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