On Fri, Jul 27, 2018 at 11:48 PM David Malcolm <dmalc...@redhat.com> wrote:
>
> The format_char_info tables in c-format.c for our own formats contain
> a lot of repetition.
>
> This patch adds a macro to express the conversion specifiers implemented
> within pp_format, making it clearer which are custom ones added by the
> various diagnostic_format_decoder callbacks.
>
> Doing so uncovered a few mistakes in the data (based on comparison with
> the source of the diagnostic_format_decoder callbacks, and the notes
> below), which the patch fixes:
>
> - gcc_diag_char_table didn't have 'Z', but it *is* implemented by pp_format.
>
> - removed erroneous 'G' and 'K' entries from gcc_diag_char_table: they're
> implemented by default_tree_printer (and thus in "tdiag") and by the
> C/C++ FEs, but not in pp_format.
>
> - removed "v" (lower case) from gcc_tdiag_char_table and
> gcc_cxxdiag_char_table
>
> Notes:
>
> pretty-print.h uses this for ATTRIBUTE_GCC_PPDIAG, used by pp_printf
> and pp_verbatim:
>
> whereas diagnostic-core.h uses this for ATTRIBUTE_GCC_DIAG, used by
> the various diagnostic functions:
>
> /* If we haven't already defined a front-end-specific diagnostics
> style, use the generic one. */
>
> Hence I'm assuming that __gcc_diag__ is for use for when we don't
> know what kind of diagnostic_format_decoder we have, and we can
> only rely on pp_format's core functionality, where __gcc_tdiag__
> is allowed to assume default_tree_printer.
OK if nobody objects.
Thanks,
Richard.
> gcc/c-family/ChangeLog:
> * c-format.c (PP_FORMAT_CHAR_TABLE): New macro, based on existing
> table entries for gcc_diag_char_table, and the 'Z' entry from
> gcc_tdiag_char_table, changing the "chain" entry for 'Z' from
> &gcc_tdiag_char_table[0] to &gcc_diag_char_table[0].
> (gcc_diag_char_table): Use PP_FORMAT_CHAR_TABLE, implicitly
> adding missing "Z" for this table. Remove erroneous "G" and "K"
> entries.
> (gcc_tdiag_char_table): Use PP_FORMAT_CHAR_TABLE. Remove "v".
> (gcc_cdiag_char_table): Use PP_FORMAT_CHAR_TABLE.
> (gcc_cxxdiag_char_table): Use PP_FORMAT_CHAR_TABLE. Remove "v".
>
> gcc/testsuite/ChangeLog:
> * gcc.dg/format/gcc_diag-1.c (foo): Update the %v tests for
> tdiag and cxxdiag.
> * gcc.dg/format/gcc_diag-10.c (test_diag): Update tests of %G
> and %K.
> ---
> gcc/c-family/c-format.c | 99
> ++++++++++---------------------
> gcc/testsuite/gcc.dg/format/gcc_diag-1.c | 4 +-
> gcc/testsuite/gcc.dg/format/gcc_diag-10.c | 7 +--
> 3 files changed, 35 insertions(+), 75 deletions(-)
>
> diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
> index a0192dd..82841e4 100644
> --- a/gcc/c-family/c-format.c
> +++ b/gcc/c-family/c-format.c
> @@ -679,43 +679,40 @@ static const format_char_info asm_fprintf_char_table[] =
> { NULL, 0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
> };
>
> +/* GCC-specific format_char_info arrays. */
> +
> +/* The conversion specifiers implemented within pp_format, and thus supported
> + by all pretty_printer instances within GCC. */
> +
> +#define PP_FORMAT_CHAR_TABLE \
> + { "di", 0, STD_C89, { T89_I, BADLEN, BADLEN, T89_L, T9L_LL,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "",
> NULL }, \
> + { "ox", 0, STD_C89, { T89_UI, BADLEN, BADLEN, T89_UL, T9L_ULL,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "",
> NULL }, \
> + { "u", 0, STD_C89, { T89_UI, BADLEN, BADLEN, T89_UL, T9L_ULL,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "",
> NULL }, \
> + { "c", 0, STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "",
> NULL }, \
> + { "s", 1, STD_C89, { T89_C, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "pq", "cR",
> NULL }, \
> + { "p", 1, STD_C89, { T89_V, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "c",
> NULL }, \
> + { "r", 1, STD_C89, { T89_C, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "",
> "//cR", NULL }, \
> + { "<", 0, STD_C89, NOARGUMENTS, "", "<", NULL }, \
> + { ">", 0, STD_C89, NOARGUMENTS, "", ">", NULL }, \
> + { "'" , 0, STD_C89, NOARGUMENTS, "", "", NULL }, \
> + { "R", 0, STD_C89, NOARGUMENTS, "", "\\", NULL }, \
> + { "m", 0, STD_C89, NOARGUMENTS, "q", "", NULL }, \
> + { "Z", 1, STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "",
> &gcc_diag_char_table[0] }
> +
> static const format_char_info gcc_diag_char_table[] =
> {
> - /* C89 conversion specifiers. */
> - { "di", 0, STD_C89, { T89_I, BADLEN, BADLEN, T89_L, T9L_LL,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "",
> NULL },
> - { "ox", 0, STD_C89, { T89_UI, BADLEN, BADLEN, T89_UL, T9L_ULL,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "",
> NULL },
> - { "u", 0, STD_C89, { T89_UI, BADLEN, BADLEN, T89_UL, T9L_ULL,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "",
> NULL },
> - { "c", 0, STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "",
> NULL },
> - { "s", 1, STD_C89, { T89_C, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "pq", "cR",
> NULL },
> - { "p", 1, STD_C89, { T89_V, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "c",
> NULL },
> -
> - /* Custom conversion specifiers. */
> + /* The conversion specifiers implemented within pp_format. */
> + PP_FORMAT_CHAR_TABLE,
>
> - /* G requires a "gcall*" argument at runtime. */
> - { "G", 1, STD_C89, { T89_G, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "\"",
> NULL },
> - /* K requires a "tree" argument at runtime. */
> - { "K", 1, STD_C89, { T89_T, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "\"",
> NULL },
> -
> - { "r", 1, STD_C89, { T89_C, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "",
> "//cR", NULL },
> - { "<", 0, STD_C89, NOARGUMENTS, "", "<", NULL },
> - { ">", 0, STD_C89, NOARGUMENTS, "", ">", NULL },
> - { "'" , 0, STD_C89, NOARGUMENTS, "", "", NULL },
> - { "R", 0, STD_C89, NOARGUMENTS, "", "\\", NULL },
> - { "m", 0, STD_C89, NOARGUMENTS, "q", "", NULL },
> { NULL, 0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
> };
>
> static const format_char_info gcc_tdiag_char_table[] =
> {
> - /* C89 conversion specifiers. */
> - { "di", 0, STD_C89, { T89_I, BADLEN, BADLEN, T89_L, T9L_LL,
> BADLEN, BADLEN, BADLEN, BADLEN }, "q", "", NULL },
> - { "ox", 0, STD_C89, { T89_UI, BADLEN, BADLEN, T89_UL, T9L_ULL,
> BADLEN, BADLEN, BADLEN, BADLEN }, "q", "", NULL },
> - { "u", 0, STD_C89, { T89_UI, BADLEN, BADLEN, T89_UL, T9L_ULL,
> BADLEN, BADLEN, BADLEN, BADLEN }, "q", "", NULL },
> - { "c", 0, STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN }, "q", "", NULL },
> - { "s", 1, STD_C89, { T89_C, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN }, "pq", "cR", NULL },
> - { "p", 1, STD_C89, { T89_V, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN }, "q", "c", NULL },
> + /* The conversion specifiers implemented within pp_format. */
> + PP_FORMAT_CHAR_TABLE,
>
> - /* Custom conversion specifiers. */
> + /* Custom conversion specifiers implemented by default_tree_printer. */
>
> /* These will require a "tree" at runtime. */
> { "DFTV", 1, STD_C89, { T89_T, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN }, "q+", "'", NULL },
> @@ -725,29 +722,15 @@ static const format_char_info gcc_tdiag_char_table[] =
> /* G requires a "gcall*" argument at runtime. */
> { "G", 1, STD_C89, { T89_G, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN }, "", "\"", NULL },
>
> - { "v", 0, STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q#", "",
> NULL },
> -
> - { "r", 1, STD_C89, { T89_C, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "",
> "/cR", NULL },
> - { "<", 0, STD_C89, NOARGUMENTS, "", "<", NULL },
> - { ">", 0, STD_C89, NOARGUMENTS, "", ">", NULL },
> - { "'", 0, STD_C89, NOARGUMENTS, "", "", NULL },
> - { "R", 0, STD_C89, NOARGUMENTS, "", "\\", NULL },
> - { "m", 0, STD_C89, NOARGUMENTS, "q", "", NULL },
> - { "Z", 1, STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "",
> &gcc_tdiag_char_table[0] },
> { NULL, 0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
> };
>
> static const format_char_info gcc_cdiag_char_table[] =
> {
> - /* C89 conversion specifiers. */
> - { "di", 0, STD_C89, { T89_I, BADLEN, BADLEN, T89_L, T9L_LL,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "",
> NULL },
> - { "ox", 0, STD_C89, { T89_UI, BADLEN, BADLEN, T89_UL, T9L_ULL,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "",
> NULL },
> - { "u", 0, STD_C89, { T89_UI, BADLEN, BADLEN, T89_UL, T9L_ULL,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "",
> NULL },
> - { "c", 0, STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "",
> NULL },
> - { "s", 1, STD_C89, { T89_C, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "pq", "cR",
> NULL },
> - { "p", 1, STD_C89, { T89_V, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "c",
> NULL },
> + /* The conversion specifiers implemented within pp_format. */
> + PP_FORMAT_CHAR_TABLE,
>
> - /* Custom conversion specifiers. */
> + /* Custom conversion specifiers implemented by c_tree_printer. */
>
> /* These will require a "tree" at runtime. */
> { "DFTV", 1, STD_C89, { T89_T, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q+", "'",
> NULL },
> @@ -759,33 +742,20 @@ static const format_char_info gcc_cdiag_char_table[] =
>
> { "v", 0, STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q#", "",
> NULL },
>
> - { "r", 1, STD_C89, { T89_C, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "",
> "/cR", NULL },
> - { "<", 0, STD_C89, NOARGUMENTS, "", "<", NULL },
> - { ">", 0, STD_C89, NOARGUMENTS, "", ">", NULL },
> - { "'", 0, STD_C89, NOARGUMENTS, "", "", NULL },
> - { "R", 0, STD_C89, NOARGUMENTS, "", "\\", NULL },
> - { "m", 0, STD_C89, NOARGUMENTS, "q", "", NULL },
> - { "Z", 1, STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "",
> &gcc_tdiag_char_table[0] },
> { NULL, 0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
> };
>
> static const format_char_info gcc_cxxdiag_char_table[] =
> {
> - /* C89 conversion specifiers. */
> - { "di", 0, STD_C89, { T89_I, BADLEN, BADLEN, T89_L, T9L_LL,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "",
> NULL },
> - { "ox", 0, STD_C89, { T89_UI, BADLEN, BADLEN, T89_UL, T9L_ULL,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "",
> NULL },
> - { "u", 0, STD_C89, { T89_UI, BADLEN, BADLEN, T89_UL, T9L_ULL,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "",
> NULL },
> - { "c", 0, STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "",
> NULL },
> - { "s", 1, STD_C89, { T89_C, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "pq", "cR",
> NULL },
> - { "p", 1, STD_C89, { T89_V, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "c",
> NULL },
> + /* The conversion specifiers implemented within pp_format. */
> + PP_FORMAT_CHAR_TABLE,
>
> - /* Custom conversion specifiers. */
> + /* Custom conversion specifiers implemented by cp_printer. */
>
> /* These will require a "tree" at runtime. */
> { "ADFHISTVX",1,STD_C89,{ T89_T, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q+#",
> "'", NULL },
> { "E", 1,STD_C89,{ T89_T, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q+#", "", NULL },
> { "K", 1, STD_C89,{ T89_T, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "\"", NULL },
> - { "v", 0,STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q#", "", NULL },
>
> /* G requires a "gcall*" argument at runtime. */
> { "G", 1, STD_C89,{ T89_G, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "\"", NULL },
> @@ -793,13 +763,6 @@ static const format_char_info gcc_cxxdiag_char_table[] =
> /* These accept either an 'int' or an 'enum tree_code' (which is handled
> as an 'int'.) */
> { "CLOPQ",0,STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q", "",
> NULL },
>
> - { "r", 1, STD_C89, { T89_C, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "",
> "/cR", NULL },
> - { "<", 0, STD_C89, NOARGUMENTS, "", "<", NULL },
> - { ">", 0, STD_C89, NOARGUMENTS, "", ">", NULL },
> - { "'", 0, STD_C89, NOARGUMENTS, "", "", NULL },
> - { "R", 0, STD_C89, NOARGUMENTS, "", "\\", NULL },
> - { "m", 0, STD_C89, NOARGUMENTS, "q", "", NULL },
> - { "Z", 1, STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "",
> &gcc_tdiag_char_table[0] },
> { NULL, 0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
> };
>
> diff --git a/gcc/testsuite/gcc.dg/format/gcc_diag-1.c
> b/gcc/testsuite/gcc.dg/format/gcc_diag-1.c
> index 4dcdb05..034e097 100644
> --- a/gcc/testsuite/gcc.dg/format/gcc_diag-1.c
> +++ b/gcc/testsuite/gcc.dg/format/gcc_diag-1.c
> @@ -87,9 +87,9 @@ foo (int i, int i1, int i2, unsigned int u, double d, char
> *s, void *p,
> cxxdiag ("%<%+#A%+#D%+#E%+#F%+#T%+#V%>", t1, t1, t1, t1, t1, t1);
> cxxdiag ("%C%L%O%P%Q", i, i, i, i, i);
>
> - tdiag ("%v%qv%#v", i, i, i);
> + tdiag ("%v", i); /* { dg-warning "format" } */
> cdiag ("%v%qv%#v", i, i, i);
> - cxxdiag ("%v%qv%#v", i, i, i);
> + cxxdiag ("%v", i); /* { dg-warning "format" } */
>
> tdiag ("%Z", v, v_len);
> cdiag ("%Z", v, v_len);
> diff --git a/gcc/testsuite/gcc.dg/format/gcc_diag-10.c
> b/gcc/testsuite/gcc.dg/format/gcc_diag-10.c
> index 9bda73b..ab9bc2f 100644
> --- a/gcc/testsuite/gcc.dg/format/gcc_diag-10.c
> +++ b/gcc/testsuite/gcc.dg/format/gcc_diag-10.c
> @@ -32,8 +32,8 @@ void test_diag (tree t, gcall *gc)
> diag ("%>"); /* { dg-warning "unmatched quoting directive " } */
> diag ("%<foo%<bar%>%>"); /* { dg-warning "nested quoting directive" } */
>
> - diag ("%G", gc);
> - diag ("%K", t);
> + diag ("%G", gc); /* { dg-warning "format" } */
> + diag ("%K", t); /* { dg-warning "format" } */
>
> diag ("%R"); /* { dg-warning "unmatched color reset directive" } */
> diag ("%r", ""); /* { dg-warning "unterminated color directive" } */
> @@ -42,9 +42,6 @@ void test_diag (tree t, gcall *gc)
> diag ("%r%r%R", "", "");
> diag ("%r%R%r%R", "", "");
>
> - diag ("%<%G%>", gc); /* { dg-warning ".G. conversion used within a quoted
> sequence" } */
> - diag ("%<%K%>", t); /* { dg-warning ".K. conversion used within a quoted
> sequence" } */
> -
> diag ("%<%R%>"); /* { dg-warning "unmatched color reset directive" }
> */
> diag ("%<%r%>", ""); /* { dg-warning "unterminated color directive" } */
> diag ("%<%r%R%>", "");
> --
> 1.8.5.3
>
