[obvious/diagnostics] Honor override_column when printing the caret
diff mbox

Message ID CAESRpQCNBMH=OufWBoSwN+QqdnbCOpWvtjYDpovki0+PTvqOCw@mail.gmail.com
State New
Headers show

Commit Message

Manuel López-Ibáñez Dec. 5, 2014, 12:06 a.m. UTC
On 3 December 2014 at 09:48, Dodji Seketeli <dodji@redhat.com> wrote:
> Manuel López-Ibáñez <lopezibanez@gmail.com> writes:
>
>> libcpp uses diagnostic->override_column to give a custom column number
>> to diagnostics. This is taken into account when building the prefix,
>> but it was missing when placing the caret.
>>
>> Before:
>>
>> /home/manuel/override_column.c:1:4: warning: "/*" within comment [-Wcomment]
>>  /* /* */
>>  ^
>>
>> After:
>>
>> /home/manuel/override_column.c:1:4: warning: "/*" within comment [-Wcomment]
>>  /* /* */
>>     ^
>>
>>
>> Committed as obvious in r218295.
>
> Thank you for this.


Thinking about it a bit more, there is no reason to not encapsulate
this logic into a function. I could have used a macro, but I guess we
now prefer inline functions where possible (a member function would
have been even better but since I wanted this to be obvious, I'll
leave it for a future C++fication of diagnostics.c).

Patch below. Bootstrapped and regression tested on x86_64-linux-gnu.
Committed to trunk as obvious cleanup.

> Thinking about it again, it would be nice to have a regression test for
> this.  At some point, I guess we should do something for regression'
> tests about the placement of the caret.  Oh well.

That would be useful. Yet, if someone has the knowledge/time for that,
I would strongly suggest to spend it on adding support for testing
diagnostics from LTO:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62234
All the new shiny -Wodr warnings might be broken the day before
release and nobody will notice.

Cheers,

Manuel.

Patch
diff mbox

Index: gcc/diagnostic.c
===================================================================
--- gcc/diagnostic.c    (revision 218408)
+++ gcc/diagnostic.c    (revision 218409)
@@ -260,6 +260,8 @@ 
 #undef DEFINE_DIAGNOSTIC_KIND
     NULL
   };
+  gcc_assert (diagnostic->kind < DK_LAST_DIAGNOSTIC_KIND);
+
   const char *text = _(diagnostic_kind_text[diagnostic->kind]);
   const char *text_cs = "", *text_ce = "";
   const char *locus_cs, *locus_ce;
@@ -274,11 +276,7 @@ 
   locus_cs = colorize_start (pp_show_color (pp), "locus");
   locus_ce = colorize_stop (pp_show_color (pp));

-  expanded_location s = expand_location_to_spelling_point
(diagnostic->location);
-  if (diagnostic->override_column)
-    s.column = diagnostic->override_column;
-  gcc_assert (diagnostic->kind < DK_LAST_DIAGNOSTIC_KIND);
-
+  expanded_location s = diagnostic_expand_location (diagnostic);
   return
     (s.file == NULL
      ? build_message_string ("%s%s:%s %s%s%s", locus_cs, progname, locus_ce,
@@ -289,8 +287,8 @@ 
      : context->show_column
      ? build_message_string ("%s%s:%d:%d:%s %s%s%s", locus_cs, s.file, s.line,
                             s.column, locus_ce, text_cs, text, text_ce)
-     : build_message_string ("%s%s:%d:%s %s%s%s", locus_cs, s.file,
s.line, locus_ce,
-                            text_cs, text, text_ce));
+     : build_message_string ("%s%s:%d:%s %s%s%s", locus_cs, s.file, s.line,
+                            locus_ce, text_cs, text, text_ce));
 }

 /* If LINE is longer than MAX_WIDTH, and COLUMN is not smaller than
@@ -337,9 +335,7 @@ 
     return;

   context->last_location = diagnostic->location;
-  s = expand_location_to_spelling_point (diagnostic->location);
-  if (diagnostic->override_column)
-    s.column = diagnostic->override_column;
+  s = diagnostic_expand_location (diagnostic);
   line = location_get_source_line (s, &line_width);
   if (line == NULL || s.column > line_width)
     return;
Index: gcc/diagnostic.h
===================================================================
--- gcc/diagnostic.h    (revision 218408)
+++ gcc/diagnostic.h    (revision 218409)
@@ -297,6 +297,18 @@ 

 void diagnostic_file_cache_fini (void);

+/* Expand the location of this diagnostic. Use this function for
consistency. */
+
+static inline expanded_location
+diagnostic_expand_location (const diagnostic_info * diagnostic)
+{
+  expanded_location s
+    = expand_location_to_spelling_point (diagnostic->location);
+  if (diagnostic->override_column)
+    s.column = diagnostic->override_column;
+  return s;
+}
+
 /* Pure text formatting support functions.  */
 extern char *file_name_as_prefix (diagnostic_context *, const char *);

Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog       (revision 218408)
+++ gcc/ChangeLog       (revision 218409)
@@ -1,3 +1,9 @@ 
+2014-12-05  Manuel López-Ibáñez  <manu@gcc.gnu.org>
+
+       * diagnostic.h (diagnostic_expand_location): New inline function.
+       * diagnostic.c (diagnostic_build_prefix): Use it.
+       (diagnostic_show_locus): Likewise.
+
 2014-12-04  H.J. Lu  <hongjiu.lu@intel.com>

        PR bootstrap/64189
Index: gcc/fortran/error.c
===================================================================
--- gcc/fortran/error.c (revision 218408)
+++ gcc/fortran/error.c (revision 218409)
@@ -1143,10 +1143,7 @@ 
   pretty_printer *pp = context->printer;
   const char *locus_cs = colorize_start (pp_show_color (pp), "locus");
   const char *locus_ce = colorize_stop (pp_show_color (pp));
-  expanded_location s = expand_location_to_spelling_point
(diagnostic->location);
-  if (diagnostic->override_column)
-    s.column = diagnostic->override_column;
-
+  expanded_location s = diagnostic_expand_location (diagnostic);
   return (s.file == NULL
          ? build_message_string ("%s%s:%s", locus_cs, progname, locus_ce )
          : !strcmp (s.file, N_("<built-in>"))
Index: gcc/fortran/ChangeLog
===================================================================
--- gcc/fortran/ChangeLog       (revision 218408)
+++ gcc/fortran/ChangeLog       (revision 218409)
@@ -1,5 +1,10 @@ 
 2014-12-05  Manuel López-Ibáñez  <manu@gcc.gnu.org>

+       * error.c (gfc_diagnostic_build_locus_prefix): Use
+       diagnostic_expand_location.
+
+2014-12-05  Manuel López-Ibáñez  <manu@gcc.gnu.org>
+
        * scanner.c (gfc_next_char_literal): Use gfc_warning_now.
        (load_file): Use the line length as the column hint for
        linemap_line_start. Reserve a location for the highest column of