This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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?

-- 
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902
This small patch changes the prefix of warning messages that are promoted to
errors by -Werror, giving such diagnostics the format:

  x.c:1:12: error: warning: âxâ initialized and declared âexternâ

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.


gcc/ChangeLog:
2009-11-22  Simon Baldwin  <simonb@google.com>

	* diagnostic.h (struct diagnostic_info): Add init_kind field.
	* diagnostic.c (diagnostic_set_info_translated): Assign init_kind.
	(diagnostic_build_prefix): Add "warning:" after "error:" to the 
	prefix for diagnostics that were warnings and are now errors.

gcc/testsuite/ChangeLog:
2009-11-22  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)
@@ -124,6 +124,7 @@ diagnostic_set_info_translated (diagnost
   diagnostic->location = location;
   diagnostic->override_column = 0;
   diagnostic->kind = kind;
+  diagnostic->init_kind = kind;
   diagnostic->option_index = 0;
 }
 
@@ -148,18 +149,30 @@ diagnostic_build_prefix (diagnostic_info
 #undef DEFINE_DIAGNOSTIC_KIND
     "must-not-happen"
   };
-  const char *text = _(diagnostic_kind_text[diagnostic->kind]);
+  char *text;
+  char *message;
   expanded_location s = expand_location (diagnostic->location);
   if (diagnostic->override_column)
     s.column = diagnostic->override_column;
-  gcc_assert (diagnostic->kind < DK_LAST_DIAGNOSTIC_KIND);
 
-  return
+  gcc_assert (diagnostic->kind < DK_LAST_DIAGNOSTIC_KIND
+	      && diagnostic->init_kind < DK_LAST_DIAGNOSTIC_KIND);
+  if (diagnostic->kind == DK_ERROR
+      && diagnostic->init_kind == DK_WARNING)
+    text = build_message_string ("%s%s",
+				 diagnostic_kind_text[diagnostic->kind],
+				 diagnostic_kind_text[diagnostic->init_kind]);
+  else
+    text = build_message_string ("%s", diagnostic_kind_text[diagnostic->kind]);
+
+  message =
     (s.file == NULL
      ? build_message_string ("%s: %s", progname, text)
      : flag_show_column
      ? build_message_string ("%s:%d:%d: %s", s.file, s.line, s.column, text)
      : build_message_string ("%s:%d: %s", s.file, s.line, text));
+  free (text);
+  return message;
 }
 
 /* Take any action which is expected to happen after the diagnostic
@@ -322,7 +335,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,
Index: gcc/diagnostic.h
===================================================================
--- gcc/diagnostic.h	(revision 154244)
+++ gcc/diagnostic.h	(working copy)
@@ -47,6 +47,9 @@ typedef struct diagnostic_info
   tree abstract_origin;
   /* The kind of diagnostic it is about.  */
   diagnostic_t kind;
+  /* The value of kind when the diagnostic is initially created; kind
+     can be overwritten, but this field remains the same.  */
+  diagnostic_t init_kind;
   /* Which OPT_* directly controls this diagnostic.  */
   int option_index;
 } diagnostic_info;
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 "warning: 'x' initialized and declared 'extern'" }
+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 "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-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 "warning: negative integer implicitly converted to unsigned type" }
+int z = x;         // { dg-error "initializer element is not constant" }

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]