In some situations, we would like to point to a location which was not
encoded when tokenizing. This happens, for example, in two prominent
cases:
1) To get precise locations within strings
(https://gcc.gnu.org/PR52952) for example, for Wformat warnings.
2) In the Fortran FE, which gives quite precise location information
by tracking the characters that it wants to warn about instead of
relying on the line-map machinery.
The most straightforward way to implement this is by adding variants
of diagnostic functions that take an explicit "offset" argument and
pass this offset through the whole diagnostics machinery. This is what
I implemented in the patch format_offset.diff attached. The downside
is that we would need to add even more variants (with/without offset)
of various diagnostic functions and track the offset/no-offset cases
explicitly.
The nicer/cleaner alternative is to somehow (re)compute a single
location value from a given location plus the new offset. This is what
I implemented in patch fortran-diagnostics-part3.diff in
linemap_redo_position_for_column(). As far as I understand, this
method only works reliably if the location+offset does not jump to a
different line map, that is, if to_column < (1u <<
map->d.ordinary.column_bits). Otherwise, we may need to recompute all
successive line-maps to accommodate the new location. The best way to
do the latter (or to work-around that issue) is not clear to me at the
moment.
Thus, I am putting forward these two alternative implementations and
seeking comments/advice/help in deciding what would be the best way to
fix this key missing piece of GCC diagnostics.
Related to this, perhaps I should make a more general call for help.
Despite the heroic, constant torrent of diagnostic fixes by Paolo,
Marek and others, I have not seen much progress on the key
infrastructure issues in the roadmap
(https://gcc.gnu.org/wiki/Better_Diagnostics). We have had at least
one major item per release since GCC 4.5, but I don't see any
particular item being tackled for GCC 5.0. Are you planning to tackle
any of them?
I have a simple patch to implement Fix-it hints but it needs more
work. Unfortunately, I have very little free time to dedicate to GCC
nowadays, so I'm afraid I might not even be able to finish this in
time. Any item in that list would be a nice major feature for GCC 5.0.
Perhaps we need to ask for help in gcc/gcc-help or some other forum.
Cheers,
Manuel.
Index: gcc/c-family/c-format.c
===================================================================
--- gcc/c-family/c-format.c (revision 197155)
+++ gcc/c-family/c-format.c (working copy)
@@ -377,10 +377,11 @@ typedef struct format_wanted_type
int format_length;
/* The actual parameter to check against the wanted type. */
tree param;
/* The argument number of that parameter. */
int arg_num;
+ int offset_loc;
/* The next type to check for this format conversion, or NULL if none. */
struct format_wanted_type *next;
} format_wanted_type;
/* Convenience macro for format_length_info meaning unused. */
@@ -903,10 +904,11 @@ typedef struct
/* Number of leaves of the format argument that were unterminated
strings. */
int number_unterminated;
/* Number of leaves of the format argument that were not counted above. */
int number_other;
+ location_t loc;
} format_check_results;
typedef struct
{
format_check_results *res;
@@ -996,11 +998,11 @@ check_function_format (tree attrs, int n
{
if (is_attribute_p ("format", TREE_PURPOSE (a)))
{
/* Yup; check it. */
function_format_info info;
- decode_format_attr (TREE_VALUE (a), &info, 1);
+ decode_format_attr (TREE_VALUE (a), &info, /*validated=*/true);
if (warn_format)
{
/* FIXME: Rewrite all the internal functions in this file
to use the ARGARRAY directly instead of constructing this
temporary list. */
@@ -1465,10 +1467,11 @@ check_format_arg (void *ctx, tree format
if (TREE_CODE (format_tree) != ADDR_EXPR)
{
res->number_non_literal++;
return;
}
+ res->loc = EXPR_LOCATION(format_tree);
format_tree = TREE_OPERAND (format_tree, 0);
if (format_types[info->format_type].flags
& (int) FMT_FLAG_PARSE_ARG_CONVERT_EXTERNAL)
{
bool objc_str = (info->format_type == gcc_objc_string_format_type);
@@ -1637,11 +1640,13 @@ check_format_info_main (format_check_res
if (*format_chars++ != '%')
continue;
if (*format_chars == 0)
{
- warning (OPT_Wformat_, "spurious trailing %<%%%> in format");
+ warning_at (res->loc ? res->loc : input_location,
+ format_chars - orig_format_chars,
+ OPT_Wformat_, "spurious trailing %<%%%> in format");
continue;
}
if (*format_chars == '%')
{
++format_chars;
@@ -2449,10 +2454,11 @@ format_type_warning (format_wanted_type
const char *wanted_type_name = type->wanted_type_name;
const char *format_start = type->format_start;
int format_length = type->format_length;
int pointer_count = type->pointer_count;
int arg_num = type->arg_num;
+ int offset_loc = type->offset_loc;
char *p;
/* If ARG_TYPE is a typedef with a misleading name (for example,
size_t but not the standard size_t expected by printf %zu), avoid
printing the typedef name. */
Index: gcc/diagnostic.c
===================================================================
--- gcc/diagnostic.c (revision 197155)
+++ gcc/diagnostic.c (working copy)
@@ -189,10 +189,11 @@ diagnostic_set_info_translated (diagnost
diagnostic->message.format_spec = msg;
diagnostic->location = location;
diagnostic->override_column = 0;
diagnostic->kind = kind;
diagnostic->option_index = 0;
+ diagnostic->offset = 0;
}
/* Initialize DIAGNOSTIC, where the message GMSGID has not yet been
translated. */
void
@@ -215,10 +216,11 @@ diagnostic_build_prefix (diagnostic_cont
#undef DEFINE_DIAGNOSTIC_KIND
"must-not-happen"
};
const char *text = _(diagnostic_kind_text[diagnostic->kind]);
expanded_location s = expand_location_to_spelling_point
(diagnostic->location);
+ s.column += diagnostic->offset;
if (diagnostic->override_column)
s.column = diagnostic->override_column;
gcc_assert (diagnostic->kind < DK_LAST_DIAGNOSTIC_KIND);
return
@@ -269,10 +271,11 @@ diagnostic_show_locus (diagnostic_contex
|| diagnostic->location == context->last_location)
return;
context->last_location = diagnostic->location;
s = expand_location_to_spelling_point (diagnostic->location);
+ s.column += diagnostic->offset;
line = location_get_source_line (s);
if (line == NULL)
return;
max_width = context->caret_max_width;
@@ -945,10 +948,27 @@ warning_at (location_t location, int opt
ret = report_diagnostic (&diagnostic);
va_end (ap);
return ret;
}
+
+bool
+warning_at (location_t location, unsigned int offset, int opt, const char
*gmsgid, ...)
+{
+ diagnostic_info diagnostic;
+ va_list ap;
+ bool ret;
+
+ va_start (ap, gmsgid);
+ diagnostic_set_info (&diagnostic, gmsgid, &ap, location, DK_WARNING);
+ diagnostic.option_index = opt;
+ diagnostic.offset = offset;
+ ret = report_diagnostic (&diagnostic);
+ va_end (ap);
+ return ret;
+}
+
/* A "pedantic" warning at LOCATION: issues a warning unless
-pedantic-errors was given on the command line, in which case it
issues an error. Use this for diagnostics required by the relevant
language standard, if you have chosen not to make them errors.
Index: gcc/diagnostic.h
===================================================================
--- gcc/diagnostic.h (revision 197155)
+++ gcc/diagnostic.h (working copy)
@@ -36,10 +36,11 @@ typedef struct diagnostic_info
void *x_data;
/* The kind of diagnostic it is about. */
diagnostic_t kind;
/* Which OPT_* directly controls this diagnostic. */
int option_index;
+ int offset;
} diagnostic_info;
/* Each time a diagnostic's classification is changed with a pragma,
we record the change and the location of the change in an array of
these structs. */
Index: gcc/diagnostic-core.h
===================================================================
--- gcc/diagnostic-core.h (revision 197155)
+++ gcc/diagnostic-core.h (working copy)
@@ -58,10 +58,13 @@ extern void internal_error (const char *
ATTRIBUTE_NORETURN;
/* Pass one of the OPT_W* from options.h as the first parameter. */
extern bool warning (int, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
extern bool warning_at (location_t, int, const char *, ...)
ATTRIBUTE_GCC_DIAG(3,4);
+extern bool warning_at (location_t, unsigned int, int, const char *, ...)
+ ATTRIBUTE_GCC_DIAG(4,5);
+
extern void error (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
extern void error_n (location_t, int, const char *, const char *, ...)
ATTRIBUTE_GCC_DIAG(3,5) ATTRIBUTE_GCC_DIAG(4,5);
extern void error_at (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
extern void fatal_error (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2)
Index: gcc/tree-diagnostic.c
===================================================================
--- gcc/tree-diagnostic.c (revision 214251)
+++ gcc/tree-diagnostic.c (working copy)
@@ -244,11 +244,11 @@ virt_loc_aware_diagnostic_finalizer (dia
maybe_unwind_expanded_macro_loc (context, diagnostic,
diagnostic->location);
}
/* Default tree printer. Handles declarations only. */
-static bool
+bool
default_tree_printer (pretty_printer *pp, text_info *text, const char *spec,
int precision, bool wide, bool set_locus, bool hash)
{
tree t;
Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h (revision 214251)
+++ gcc/fortran/gfortran.h (working copy)
@@ -2696,11 +2696,13 @@ void gfc_warning_cmdline (const char *gm
void gfc_clear_warning (void);
void gfc_warning_check (void);
void gfc_error (const char *, ...) ATTRIBUTE_GCC_GFC(1,2);
+void gfc_error_cmdline (const char *gmsgid, ...) ATTRIBUTE_GCC_GFC(1,2);
void gfc_error_now (const char *, ...) ATTRIBUTE_GCC_GFC(1,2);
+void gfc_error_now_2 (const char *, ...) ATTRIBUTE_GCC_GFC(1,2);
void gfc_fatal_error (const char *, ...) ATTRIBUTE_NORETURN
ATTRIBUTE_GCC_GFC(1,2);
void gfc_internal_error (const char *, ...) ATTRIBUTE_NORETURN
ATTRIBUTE_GCC_GFC(1,2);
void gfc_clear_error (void);
int gfc_error_check (void);
int gfc_error_flag_test (void);
Index: gcc/fortran/error.c
===================================================================
--- gcc/fortran/error.c (revision 214251)
+++ gcc/fortran/error.c (working copy)
@@ -956,10 +956,67 @@ gfc_warning_now (const char *gmsgid, ...
gfc_increment_error_count();
buffer_flag = i;
}
+/* Encode and return a source_location from a column number. The
+ source line considered is the last source line used to call
+ linemap_line_start, i.e, the last source line which a location was
+ encoded from. */
+
+source_location
+linemap_redo_position_for_column (struct line_maps *set, source_location loc,
+ unsigned int to_column)
+{
+ const struct line_map * map = linemap_lookup (set, loc);
+ gcc_assert(to_column < (1u << map->d.ordinary.column_bits));
+ source_location r = loc + to_column;
+ gcc_assert (map == linemap_lookup (set, r));
+ return r;
+}
+
+bool
+default_tree_printer (pretty_printer *pp, text_info *text, const char *spec,
+ int precision, bool wide, bool set_locus, bool hash);
+
+/* Called from output_format -- during diagnostic message processing --
+ to handle C++ specific format specifier with the following meanings:
+
+ %C Current locus (no argument)
+*/
+static bool
+gfc_format_decoder (pretty_printer *pp,
+ text_info *text, const char *spec,
+ int precision, bool wide, bool set_locus, bool verbose)
+{
+ const char *result;
+ switch (*spec)
+ {
+ case 'C':
+ {
+ result = "(1)";
+ gcc_assert (text->locus);
+ *text->locus = gfc_current_locus.lb->location;
+ gcc_assert (gfc_current_locus.nextc - gfc_current_locus.lb->line);
+ unsigned int c1 = gfc_current_locus.nextc - gfc_current_locus.lb->line;
+ *text->locus
+ = linemap_redo_position_for_column (line_table, *text->locus,
+ c1 + 1);
+ global_dc->caret_char = '1';
+ break;
+ }
+ default:
+ /* We can get called from the gimplifier. */
+ /* ??? Is there a way to set the formart_decoder back to this default? */
+ return default_tree_printer (pp, text, spec, precision, wide, set_locus,
+ verbose);
+ }
+
+ pp_string (pp, result);
+ return true;
+}
+
/* Return a malloc'd string describing a location. The caller is
responsible for freeing the memory. */
static char *
gfc_diagnostic_build_prefix (diagnostic_context *context,
const diagnostic_info *diagnostic)
@@ -1065,10 +1122,24 @@ gfc_warning_cmdline (const char *gmsgid,
DK_WARNING);
report_diagnostic (&diagnostic);
va_end (argp);
}
+/* Give an error about the command-line. */
+
+void
+gfc_error_cmdline (const char *gmsgid, ...)
+{
+ va_list argp;
+ diagnostic_info diagnostic;
+
+ va_start (argp, gmsgid);
+ diagnostic_set_info (&diagnostic, gmsgid, &argp, UNKNOWN_LOCATION, DK_ERROR);
+ report_diagnostic (&diagnostic);
+ va_end (argp);
+}
+
/* Clear the warning flag. */
void
gfc_clear_warning (void)
{
@@ -1145,10 +1216,29 @@ warning:
/* Immediate error. */
void
+gfc_error_now_2 (const char *gmsgid, ...)
+{
+ va_list argp;
+ diagnostic_info diagnostic;
+
+ va_start (argp, gmsgid);
+ diagnostic_set_info (&diagnostic, gmsgid, &argp, UNKNOWN_LOCATION, DK_ERROR);
+ report_diagnostic (&diagnostic);
+ va_end (argp);
+
+ gfc_increment_error_count();
+
+ if (flag_fatal_errors)
+ exit (FATAL_EXIT_CODE);
+}
+
+/* Immediate error. */
+
+void
gfc_error_now (const char *gmsgid, ...)
{
va_list argp;
int i;
@@ -1319,7 +1409,8 @@ gfc_errors_to_warnings (int f)
void
gfc_diagnostics_init (void)
{
diagnostic_starter (global_dc) = gfc_diagnostic_starter;
diagnostic_finalizer (global_dc) = gfc_diagnostic_finalizer;
+ diagnostic_format_decoder (global_dc) = gfc_format_decoder;
global_dc->caret_char = '^';
}
Index: gcc/fortran/parse.c
===================================================================
--- gcc/fortran/parse.c (revision 214251)
+++ gcc/fortran/parse.c (working copy)
@@ -844,11 +844,11 @@ next_free (void)
do
c = gfc_next_ascii_char ();
while (ISDIGIT(c));
if (!gfc_is_whitespace (c))
- gfc_error_now ("Non-numeric character in statement label at %C");
+ gfc_error_now_2 ("Non-numeric character in statement label at %C");
return ST_NONE;
}
else
{
