On 3 December 2014 at 09:48, Dodji Seketeli <do...@redhat.com> wrote: > Manuel López-Ibáñez <lopeziba...@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 <m...@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...@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 <m...@gcc.gnu.org> + * error.c (gfc_diagnostic_build_locus_prefix): Use + diagnostic_expand_location. + +2014-12-05 Manuel López-Ibáñez <m...@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