This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH obvious/diagnostics] Honor override_column when printing the caret
- From: Manuel López-Ibáñez <lopezibanez at gmail dot com>
- To: Dodji Seketeli <dodji at redhat dot com>
- Cc: Gcc Patch List <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 5 Dec 2014 01:06:31 +0100
- Subject: Re: [PATCH obvious/diagnostics] Honor override_column when printing the caret
- Authentication-results: sourceware.org; auth=none
- References: <CAESRpQBmeYksSTbBXxi5jUWzdvSFbeLPuaf+LoepwjC2iVsD1g at mail dot gmail dot com> <87ppc1p18n dot fsf at redhat dot com>
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.
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