This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Support %N$/*N$ style format arguments in GCC diagnostics (take 2)
On Fri, May 20, 2005 at 01:22:24PM -0700, Zack Weinberg wrote:
> > I tried to make the requirements match POSIX *printf requirements.
> > pp_base_format_text works in 3 phases, first it scans the format string
> > and records the arguments, then, with optional help of a hook stores
> > the arguments into a union and finally prints them as it used to print them
> > before, only instead of va_arg directly it uses that union.
>
> I'd like to draw your attention to the algorithm outlined in
> <http://gcc.gnu.org/ml/gcc-patches/2003-07/msg02551.html>. I think
> it's pretty close to the algorithm you implemented, but it doesn't
> need the union (at the cost of slight, IMO actively desirable,
> deviation from POSIX - you can't refer to the same argument twice with
> different format specifiers) and should be structurally simpler.
It doesn't seem to be much simpler, because it needs to play ugly games
with changing the pretty printer in place (passing a scratch printer
to the format decoder routine does not work, because the original
printer is usually embedded into some larger structure and relies on
the other fields). Anyway, I have implemented that below.
2005-05-25 Jakub Jelinek <jakub@redhat.com>
* pretty-print.h (PP_NL_ARGMAX): Define.
(text_info): Add locus and args fields.
(pp_prepare_to_format, pp_base_prepare_to_format): Remove.
(pp_format_args): Define.
(pp_base_format_args): New prototype.
* pretty-print.c (pp_base_prepare_to_format): Remove.
(arg_printer): New static variable.
(pp_base_format_args): New function.
(pp_base_format_text): Support for %N$ and %M$.*N$s style arguments.
Just print the arguments previously recorded in text->args array.
Free objects from arg_printer.buffer->obstack at the end.
(pp_base_format_verbatim): Call pp_format_args right before
pp_format_text.
(pp_printf): Likewise. Clear text.locus.
(pp_verbatim): Clear text.locus.
* diagnostic.c (diagnostic_report_diagnostic): Don't call
pp_prepare_to_format. Instead, set diagnostic->message.locus
and call pp_format_args.
(verbatim): Clear text.locus.
cp/
* error.c (cp_printer): If '+' is seen and text->locus not NULL,
for codes with trees set *text->locus to location_of (tree_arg).
--- gcc/pretty-print.h.jj 2004-11-11 10:37:33.000000000 +0100
+++ gcc/pretty-print.h 2005-05-25 13:28:40.000000000 +0200
@@ -25,6 +25,9 @@ Software Foundation, 59 Temple Place - S
#include "obstack.h"
#include "input.h"
+/* Maximum number of format string arguments. */
+#define PP_NL_ARGMAX 30
+
/* The type of a text to be formatted according a format specification
along with a list of things. */
typedef struct
@@ -32,6 +35,8 @@ typedef struct
const char *format_spec;
va_list *args_ptr;
int err_no; /* for %m */
+ location_t *locus;
+ const char *args[PP_NL_ARGMAX];
} text_info;
/* How often diagnostics are prefixed by their locations:
@@ -158,8 +163,7 @@ struct pretty_print_info
#define pp_append_text(PP, B, E) \
pp_base_append_text (pp_base (PP), B, E)
#define pp_flush(PP) pp_base_flush (pp_base (PP))
-#define pp_prepare_to_format(PP, TI, LOC) \
- pp_base_prepare_to_format (pp_base (PP), TI, LOC)
+#define pp_format_args(PP, TI) pp_base_format_args (pp_base (PP), TI)
#define pp_format_text(PP, TI) pp_base_format_text (pp_base (PP), TI)
#define pp_format_verbatim(PP, TI) \
pp_base_format_verbatim (pp_base (PP), TI)
@@ -263,8 +267,7 @@ extern void pp_printf (pretty_printer *,
extern void pp_verbatim (pretty_printer *, const char *, ...);
extern void pp_base_flush (pretty_printer *);
-extern void pp_base_prepare_to_format (pretty_printer *, text_info *,
- location_t *);
+extern void pp_base_format_args (pretty_printer *, text_info *);
extern void pp_base_format_text (pretty_printer *, text_info *);
extern void pp_base_format_verbatim (pretty_printer *, text_info *);
--- gcc/pretty-print.c.jj 2005-04-29 09:18:04.000000000 +0200
+++ gcc/pretty-print.c 2005-05-25 16:43:56.000000000 +0200
@@ -166,42 +166,10 @@ pp_base_indent (pretty_printer *pp)
pp_space (pp);
}
-/* Prepare PP to format a message pointed to by TEXT, with tentative
- location LOCUS. It is expected that a call to pp_format_text with
- exactly the same PP and TEXT arguments will follow. This routine
- may modify the data in memory at TEXT and LOCP, and if it does,
- caller is expected to notice.
-
- Currently, all this does is notice a %H or %J escape at the beginning
- of the string, and update LOCUS to match. */
-void
-pp_base_prepare_to_format (pretty_printer *pp ATTRIBUTE_UNUSED,
- text_info *text,
- location_t *locus)
-{
- const char *p = text->format_spec;
- tree t;
-
- /* Extract the location information if any. */
- if (p[0] == '%')
- switch (p[1])
- {
- case 'H':
- *locus = *va_arg (*text->args_ptr, location_t *);
- text->format_spec = p + 2;
- break;
+static pretty_printer arg_printer;
- case 'J':
- t = va_arg (*text->args_ptr, tree);
- *locus = DECL_SOURCE_LOCATION (t);
- text->format_spec = p + 2;
- break;
- }
-}
-
-
-/* Format a message pointed to by TEXT. The following format specifiers are
- recognized as being client independent:
+/* Format arguments of a message pointed to by TEXT.
+ The following format specifiers are recognized as being client independent:
%d, %i: (signed) integer in base ten.
%u: unsigned integer in base ten.
%o: unsigned integer in base eight.
@@ -218,58 +186,176 @@ pp_base_prepare_to_format (pretty_printe
%>: closing quote.
%': apostrophe (should only be used in untranslated messages;
translations should use appropriate punctuation directly).
- %.*s: a substring the length of which is specified by an integer.
+ %.*s: a substring the length of which is specified by an argument
+ integer.
+ %Ns: likewise, but length specified as constant in the format string.
%H: location_t.
- Flag 'q': quote formatted text (must come immediately after '%'). */
-void
-pp_base_format_text (pretty_printer *pp, text_info *text)
-{
- for (; *text->format_spec; ++text->format_spec)
+ %J: a decl tree, from which DECL_SOURCE_LOCATION will be recorded.
+ Flag 'q': quote formatted text (must come immediately after '%').
+ Arguments can be used sequentially, or through %N$ resp. *N$ notation
+ Nth argument after the format string. If %N$ / *N$ notation is used, it
+ must be used for all arguments, except %m, %%, %<, %> and %' where either
+ form can be used. When %M$.*N$ is used, M must be N + 1.
+ The format string must have conversion specifiers with argument numbers 1
+ up to highest argument. A format string can have at most 30 arguments. */
+void
+pp_base_format_args (pretty_printer *pp, text_info *text)
+{
+ const char *format_start = text->format_spec;
+ const char *p;
+ unsigned int curarg = 0;
+ bool any_numbered = false, any_unnumbered = false;
+ unsigned int argno;
+ pretty_printer save_pp;
+
+ /* First phase. Scan the format string to see if % or %N$ style
+ specifiers are used, record them in the TEXT->ARGS array. */
+ memset (text->args, 0, sizeof (text->args));
+ for (p = format_start; *p; )
+ if (*p++ == '%')
+ {
+ unsigned int n;
+ const char *start;
+ bool numbered = false;
+
+ if (*p >= '0' && *p <= '9')
+ {
+ char *end;
+ n = strtoul (p, &end, 10) - 1;
+ gcc_assert (*end == '$');
+ p = (const char *) (end + 1);
+ numbered = true;
+ }
+ else
+ n = curarg;
+
+ if (*p == 'q')
+ ++p;
+ start = p;
+ if (*p == 'w')
+ ++p;
+ else
+ while (*p == 'l')
+ ++p;
+
+ /* %%, %m, %<, %>, %' don't use arguments, so can be used with
+ both These specifiers don't need any arguments. */
+ if (strchr ("%m<>'", *p) == NULL)
+ {
+ if (numbered)
+ {
+ any_numbered = true;
+ gcc_assert (! any_unnumbered);
+ }
+ else
+ {
+ any_unnumbered = true;
+ gcc_assert (! any_numbered);
+ ++curarg;
+ }
+ }
+ else if (!numbered)
+ {
+ ++p;
+ continue;
+ }
+
+ gcc_assert (n < PP_NL_ARGMAX);
+ if (*p == '+')
+ ++p;
+ if (*p == '#')
+ ++p;
+ if (*p == '.')
+ {
+ /* We handle '%.Ns' and '%.*s' or '%M$.*N$s'
+ (where M == N + 1). */
+ if (p[1] >= '0' && p[1] <= '9')
+ for (p += 2; *p >= '0' && *p <= '9'; ++p);
+ else
+ {
+ unsigned int pn;
+
+ gcc_assert (p[1] == '*');
+ p += 2;
+ if (numbered)
+ {
+ char *end;
+
+ pn = strtoul (p, &end, 10) - 1;
+ gcc_assert (p != end && *end == '$');
+ gcc_assert (n > 0 && pn == n - 1);
+ p = (const char *) (end + 1);
+ }
+ else
+ {
+ pn = n;
+ n = curarg++;
+ gcc_assert (n < PP_NL_ARGMAX);
+ }
+
+ gcc_assert (text->args[pn] == NULL
+ || *text->args[pn] == '\0');
+ text->args[pn] = "";
+ }
+ gcc_assert (*p == 's');
+ }
+
+ ++p;
+ gcc_assert (text->args[n] == NULL
+ || memcmp (text->args[n], start, p - start) == 0);
+ text->args[n] = start;
+ }
+
+ /* Initialize arg_printer if it has not been initialized yet. */
+ if (arg_printer.buffer == NULL)
+ {
+ pp_construct (&arg_printer, NULL, 0);
+ arg_printer.buffer->stream = NULL;
+ }
+
+ /* This is ugly. pp is often embedded into larger structures,
+ so we can't just call pp_format_decoder with &arg_printer.
+ Instead, we modify *pp and restore it back afterwards. */
+ memcpy (&save_pp, pp, sizeof (pretty_printer));
+ memcpy (pp, &arg_printer, sizeof (pretty_printer));
+ pp_format_decoder (pp) = pp_format_decoder (&save_pp);
+
+ /* Second phase. Print the arguments into a arg_printer strings,
+ store them into the text->args array. */
+ for (argno = 0; argno < PP_NL_ARGMAX; ++argno)
{
int precision = 0;
bool wide = false;
- bool quoted = false;
- /* Ignore text. */
- {
- const char *p = text->format_spec;
- while (*p && *p != '%')
- ++p;
- pp_wrap_text (pp, text->format_spec, p);
- text->format_spec = p;
- }
+ if (text->args[argno] == NULL)
+ continue;
+ gcc_assert (argno == 0 || text->args[argno - 1] != NULL);
+ /* "" is the int length for %.*s or %M$.*N$s, will be handled
+ by the following argument. */
+ if (*text->args[argno] == '\0')
+ continue;
- if (*text->format_spec == '\0')
- break;
+ text->format_spec = text->args[argno];
- /* We got a '%'. Check for 'q', then parse precision modifiers,
- if any. */
- if (*++text->format_spec == 'q')
+ switch (*text->format_spec)
{
- quoted = true;
+ case 'w':
+ wide = true;
++text->format_spec;
+ break;
+
+ case 'l':
+ do
+ ++precision;
+ while (*++text->format_spec == 'l');
+ break;
+
+ default:
+ break;
}
- switch (*text->format_spec)
- {
- case 'w':
- wide = true;
- ++text->format_spec;
- break;
-
- case 'l':
- do
- ++precision;
- while (*++text->format_spec == 'l');
- break;
-
- default:
- break;
- }
/* We don't support precision beyond that of "long long". */
gcc_assert (precision <= 2);
- if (quoted)
- pp_string (pp, open_quote);
switch (*text->format_spec)
{
case 'c':
@@ -278,87 +364,97 @@ pp_base_format_text (pretty_printer *pp,
case 'd':
case 'i':
- if (wide)
- pp_wide_integer (pp, va_arg (*text->args_ptr, HOST_WIDE_INT));
- else
- pp_integer_with_precision
- (pp, *text->args_ptr, precision, int, "d");
+ if (wide)
+ pp_wide_integer (pp, va_arg (*text->args_ptr, HOST_WIDE_INT));
+ else
+ pp_integer_with_precision
+ (pp, *text->args_ptr, precision, int, "d");
break;
case 'o':
- if (wide)
- pp_scalar (pp, "%" HOST_WIDE_INT_PRINT "o",
- va_arg (*text->args_ptr, unsigned HOST_WIDE_INT));
- else
- pp_integer_with_precision
- (pp, *text->args_ptr, precision, unsigned, "u");
+ if (wide)
+ pp_scalar (pp, "%" HOST_WIDE_INT_PRINT "o",
+ va_arg (*text->args_ptr, unsigned HOST_WIDE_INT));
+ else
+ pp_integer_with_precision
+ (pp, *text->args_ptr, precision, unsigned, "o");
break;
case 's':
pp_string (pp, va_arg (*text->args_ptr, const char *));
break;
- case 'p':
- pp_pointer (pp, va_arg (*text->args_ptr, void *));
- break;
+ case 'p':
+ pp_pointer (pp, va_arg (*text->args_ptr, void *));
+ break;
case 'u':
- if (wide)
- pp_scalar (pp, HOST_WIDE_INT_PRINT_UNSIGNED,
- va_arg (*text->args_ptr, unsigned HOST_WIDE_INT));
- else
- pp_integer_with_precision
- (pp, *text->args_ptr, precision, unsigned, "u");
+ if (wide)
+ pp_scalar (pp, HOST_WIDE_INT_PRINT_UNSIGNED,
+ va_arg (*text->args_ptr, unsigned HOST_WIDE_INT));
+ else
+ pp_integer_with_precision
+ (pp, *text->args_ptr, precision, unsigned, "u");
break;
case 'x':
- if (wide)
- pp_scalar (pp, HOST_WIDE_INT_PRINT_HEX,
- va_arg (*text->args_ptr, unsigned HOST_WIDE_INT));
- else
- pp_integer_with_precision
- (pp, *text->args_ptr, precision, unsigned, "x");
+ if (wide)
+ pp_scalar (pp, HOST_WIDE_INT_PRINT_HEX,
+ va_arg (*text->args_ptr, unsigned HOST_WIDE_INT));
+ else
+ pp_integer_with_precision
+ (pp, *text->args_ptr, precision, unsigned, "x");
break;
case 'm':
- pp_string (pp, xstrerror (text->err_no));
- break;
-
case '%':
- pp_character (pp, '%');
- break;
-
case '<':
- pp_string (pp, open_quote);
- break;
-
case '>':
case '\'':
- pp_string (pp, close_quote);
+ /* These don't need an argument, so will be handled in the
+ next phase. */
+ text->args[argno] = "%";
+ continue;
+
+ case 'H':
+ {
+ location_t *locus = va_arg (*text->args_ptr, location_t *);
+ gcc_assert (text->locus != NULL);
+ *text->locus = *locus;
+ }
break;
- case 'H':
- {
- location_t *locus = va_arg (*text->args_ptr, location_t *);
- expanded_location s = expand_location (*locus);
- pp_string (pp, "file '");
- pp_string (pp, s.file);
- pp_string (pp, "', line ");
- pp_decimal_int (pp, s.line);
- }
- break;
+ case 'J':
+ {
+ tree t = va_arg (*text->args_ptr, tree);
+ gcc_assert (text->locus != NULL);
+ *text->locus = DECL_SOURCE_LOCATION (t);
+ }
+ break;
case '.':
{
int n;
const char *s;
- /* We handle no precision specifier but '%.*s'. */
- ++text->format_spec;
- gcc_assert (*text->format_spec == '*');
+ char *end;
+ /* We handle '%.Ns' and '%.*s' or '%M$.*N$s'
+ (where M == N + 1). The format string should be verified
+ already from the first phase. */
++text->format_spec;
- gcc_assert (*text->format_spec == 's');
+ if (*text->format_spec >= '0' && *text->format_spec <= '9')
+ {
+ n = strtoul (text->format_spec, &end, 10);
+ text->format_spec = (const char *) end;
+ gcc_assert (*text->format_spec == 's');
+ }
+ else
+ {
+ gcc_assert (*text->format_spec == '*');
+ while (*text->format_spec != 's')
+ ++text->format_spec;
+ n = va_arg (*text->args_ptr, int);
+ }
- n = va_arg (*text->args_ptr, int);
s = va_arg (*text->args_ptr, const char *);
pp_append_text (pp, s, s + n);
}
@@ -367,15 +463,129 @@ pp_base_format_text (pretty_printer *pp,
default:
{
bool ok;
-
+
gcc_assert (pp_format_decoder (pp));
ok = pp_format_decoder (pp) (pp, text);
gcc_assert (ok);
}
}
+
+ obstack_1grow (&arg_printer.buffer->obstack, '\0');
+ text->args[argno] = obstack_finish (&arg_printer.buffer->obstack);
+ }
+
+ text->format_spec = format_start;
+ memcpy (pp, &save_pp, sizeof (pretty_printer));
+}
+
+/* Format of a message pointed to by TEXT. */
+void
+pp_base_format_text (pretty_printer *pp, text_info *text)
+{
+ unsigned int curarg;
+
+ /* This is a third phase, first 2 phases done in pp_base_format_args.
+ Now we actually print it. */
+ for (curarg = 0; *text->format_spec; ++text->format_spec)
+ {
+ bool quoted = false;
+ bool numbered = false;
+ const char *p;
+
+ /* Ignore text. */
+ p = text->format_spec;
+ while (*p && *p != '%')
+ ++p;
+ pp_wrap_text (pp, text->format_spec, p);
+ text->format_spec = p;
+
+ if (*text->format_spec == '\0')
+ break;
+
+ /* We got a '%'. First check for 'N$'. */
+ ++text->format_spec;
+ if (*text->format_spec >= '0' && *text->format_spec <= '9')
+ {
+ char *end;
+ curarg = strtoul (text->format_spec, &end, 10) - 1;
+ gcc_assert (*end == '$');
+ text->format_spec = (const char *) (end + 1);
+ numbered = true;
+ }
+
+ /* Check for 'q', then parse precision modifiers, if any. */
+ if (*text->format_spec == 'q')
+ {
+ quoted = true;
+ ++text->format_spec;
+ }
+ if (*text->format_spec == 'w')
+ ++text->format_spec;
+ while (*text->format_spec == 'l')
+ ++text->format_spec;
+
+ if (quoted)
+ pp_string (pp, open_quote);
+
+ if (*text->format_spec == '+')
+ ++text->format_spec;
+ if (*text->format_spec == '#')
+ ++text->format_spec;
+ switch (*text->format_spec)
+ {
+ case 'm':
+ pp_string (pp, xstrerror (text->err_no));
+ break;
+
+ case '%':
+ pp_character (pp, '%');
+ break;
+
+ case '<':
+ pp_string (pp, open_quote);
+ break;
+
+ case '>':
+ case '\'':
+ pp_string (pp, close_quote);
+ break;
+
+ case 'H':
+ case 'J':
+ ++curarg;
+ break;
+
+ case '.':
+ if (numbered)
+ --curarg;
+ gcc_assert (text->args[curarg] != NULL && *text->args[curarg] == '\0');
+ gcc_assert (text->args[curarg + 1] != NULL);
+ while (*text->format_spec != 's')
+ ++text->format_spec;
+ pp_string (pp, text->args[curarg + 1]);
+ curarg += 2;
+ break;
+
+ default:
+ gcc_assert (text->args[curarg] != NULL);
+ pp_string (pp, text->args[curarg]);
+ ++curarg;
+ break;
+ }
+
if (quoted)
pp_string (pp, close_quote);
}
+
+ /* Now free everything in the argument obstack. */
+ for (curarg = 0; curarg < PP_NL_ARGMAX; ++curarg)
+ if (text->args[curarg] == NULL)
+ break;
+ else if (*text->args[curarg] != '\0' && *text->args[curarg] != '%')
+ {
+ obstack_free (&arg_printer.buffer->obstack, text->args[curarg]);
+ break;
+ }
}
/* Helper subroutine of output_verbatim and verbatim. Do the appropriate
@@ -390,6 +600,7 @@ pp_base_format_verbatim (pretty_printer
pp->prefixing_rule = DIAGNOSTICS_SHOW_PREFIX_NEVER;
pp_line_cutoff (pp) = 0;
/* Do the actual formatting. */
+ pp_format_args (pp, text);
pp_format_text (pp, text);
/* Restore previous settings. */
pp_prefixing_rule (pp) = rule;
@@ -553,6 +764,8 @@ pp_printf (pretty_printer *pp, const cha
text.err_no = errno;
text.args_ptr = ≈
text.format_spec = msg;
+ text.locus = NULL;
+ pp_format_args (pp, &text);
pp_format_text (pp, &text);
va_end (ap);
}
@@ -569,6 +782,7 @@ pp_verbatim (pretty_printer *pp, const c
text.err_no = errno;
text.args_ptr = ≈
text.format_spec = msg;
+ text.locus = NULL;
pp_format_verbatim (pp, &text);
va_end (ap);
}
--- gcc/diagnostic.c.jj 2005-05-06 09:58:26.000000000 +0200
+++ gcc/diagnostic.c 2005-05-25 11:42:18.000000000 +0200
@@ -347,8 +347,8 @@ diagnostic_report_diagnostic (diagnostic
= ACONCAT ((diagnostic->message.format_spec,
" [", cl_options[diagnostic->option_index].opt_text, "]", NULL));
- pp_prepare_to_format (context->printer, &diagnostic->message,
- &diagnostic->location);
+ diagnostic->message.locus = &diagnostic->location;
+ pp_format_args (context->printer, &diagnostic->message);
(*diagnostic_starter (context)) (context, diagnostic);
pp_format_text (context->printer, &diagnostic->message);
(*diagnostic_finalizer (context)) (context, diagnostic);
@@ -405,6 +405,7 @@ verbatim (const char *msgid, ...)
text.err_no = errno;
text.args_ptr = ≈
text.format_spec = _(msgid);
+ text.locus = NULL;
pp_format_verbatim (global_dc->printer, &text);
pp_flush (global_dc->printer);
va_end (ap);
--- gcc/cp/error.c.jj 2005-03-25 11:37:25.000000000 +0100
+++ gcc/cp/error.c 2005-05-25 11:52:16.000000000 +0200
@@ -2276,13 +2276,18 @@ cp_printer (pretty_printer *pp, text_inf
{
int verbose = 0;
const char *result;
-#define next_tree va_arg (*text->args_ptr, tree)
+ bool set_locus = false;
+ tree t = NULL;
+#define next_tree (t = va_arg (*text->args_ptr, tree))
#define next_tcode va_arg (*text->args_ptr, enum tree_code)
#define next_lang va_arg (*text->args_ptr, enum languages)
#define next_int va_arg (*text->args_ptr, int)
if (*text->format_spec == '+')
- ++text->format_spec;
+ {
+ ++text->format_spec;
+ set_locus = (text->locus != NULL);
+ }
if (*text->format_spec == '#')
{
verbose = 1;
@@ -2308,6 +2313,8 @@ cp_printer (pretty_printer *pp, text_inf
}
pp_base_string (pp, result);
+ if (set_locus && t != NULL)
+ *text->locus = location_of (t);
return true;
#undef next_tree
#undef next_tcode
Jakub