From: Richard Biener <richard.guenther@gmail.com>
To: David Malcolm <dmalcolm@redhat.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH 5/5] Formatted printing for dump_* in the middle-end
Date: Tue, 31 Jul 2018 13:03:00 -0000 [thread overview]
Message-ID: <CAFiYyc1f3hRRq11t3yNR508DiN205ut+SOu_xT5_PoR6=BfQ7w@mail.gmail.com> (raw)
In-Reply-To: <1532730752-10525-6-git-send-email-dmalcolm@redhat.com>
On Fri, Jul 27, 2018 at 11:49 PM David Malcolm <dmalcolm@redhat.com> wrote:
>
> This patch converts dump_print and dump_printf_loc from using
> printf (and thus ATTRIBUTE_PRINTF) to using a new pretty-printer
> based on pp_format, which supports formatting middle-end types.
>
> In particular, the following codes are implemented (in addition
> to the standard pretty_printer ones):
>
> %E: gimple *:
> Equivalent to: dump_gimple_expr (MSG_*, TDF_SLIM, stmt, 0)
> %G: gimple *:
> Equivalent to: dump_gimple_stmt (MSG_*, TDF_SLIM, stmt, 0)
> %T: tree:
> Equivalent to: dump_generic_expr (MSG_*, arg, TDF_SLIM).
>
> Hence it becomes possible to convert e.g.:
>
> if (dump_enabled_p ())
> {
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> "not vectorized: different sized vector "
> "types in statement, ");
> dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM, vectype);
> dump_printf (MSG_MISSED_OPTIMIZATION, " and ");
> dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM, nunits_vectype);
> dump_printf (MSG_MISSED_OPTIMIZATION, "\n");
> }
>
> into a one-liner:
>
> if (dump_enabled_p ())
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> "not vectorized: different sized vector "
> "types in statement, %T and %T\n",
> vectype, nunits_vectype);
>
> Unlike regular pretty-printers, this one captures optinfo_item
> instances for the formatted chunks as appropriate, so that when
> written out to a JSON optimization record, the relevant parts of
> the message are labelled by type, and by source location (so that
> e.g. %G is entirely equivalent to using dump_gimple_stmt).
>
> dump_printf and dump_printf_loc become marked with
> ATTRIBUTE_GCC_DUMP_PRINTF, which the patch also implements.
>
> gcc/c-family/ChangeLog:
> * c-format.c (enum format_type): Add gcc_dump_printf_format_type.
> (local_gimple_ptr_node): New decl.
> (gcc_dump_printf_length_specs): New.
> (gcc_dump_printf_flag_pairs): New.
> (gcc_dump_printf_flag_specs): New.
> (gcc_dump_printf_char_table): New.
> (format_types_orig): Add entry for "gcc_dump_printf".
> (init_dynamic_diag_info): Create local_gimple_ptr_node.
> Set up length_char_specs and conversion_specs for
> gcc_dump_printf_format_type.
> (handle_format_attribute): Handle gcc_dump_printf_format_type.
> * c-format.h (T89_GIMPLE): New macro.
Iff the c-family changes are neccessary (are they?) then how does this
work for non-c-family languages which do not link c-family/c-format.o?
> gcc/ChangeLog:
> * dump-context.h: Include "dumpfile.h".
> (dump_context::dump_printf_va): Convert final param from va_list
> to va_list *. Convert from ATTRIBUTE_PRINTF to
> ATTRIBUTE_GCC_DUMP_PRINTF.
> (dump_context::dump_printf_loc_va): Likewise.
> * dumpfile.c: Include "stringpool.h".
> (make_item_for_dump_printf_va): Delete.
> (make_item_for_dump_printf): Delete.
> (class dump_pretty_printer): New class.
> (dump_pretty_printer::dump_pretty_printer): New ctor.
> (dump_pretty_printer::emit_items): New member function.
> (dump_pretty_printer::emit_any_pending_textual_chunks): New member
> function.
> (dump_pretty_printer::emit_item): New member function.
> (dump_pretty_printer::stash_item): New member function.
> (dump_pretty_printer::format_decoder_cb): New member function.
> (dump_pretty_printer::decode_format): New member function.
> (dump_context::dump_printf_va): Reimplement in terms of
> dump_pretty_printer.
> (dump_context::dump_printf_loc_va): Convert final param from va_list
> to va_list *.
> (dump_context::begin_scope): Reimplement call to
> make_item_for_dump_printf.
> (dump_printf): Update for change to dump_printf_va.
> (dump_printf_loc): Likewise.
> (selftest::test_capture_of_dump_calls): Convert "stmt" from
> greturn * to gimple *. Add a test_decl. Add tests of dump_printf
> with %T, %E, and %G.
> * dumpfile.h (ATTRIBUTE_GCC_DUMP_PRINTF): New macro.
> (dump_printf): Replace ATTRIBUTE_PRINTF_2 with
> ATTRIBUTE_GCC_DUMP_PRINTF (2, 3).
> (dump_printf_loc): Replace ATTRIBUTE_PRINTF_3 with
> ATTRIBUTE_GCC_DUMP_PRINTF (3, 0).
>
> gcc/testsuite/ChangeLog:
> * gcc.dg/format/gcc_diag-1.c: Fix typo. Add test coverage for
> gcc_dump_printf.
> * gcc.dg/format/gcc_diag-10.c: Add gimple typedef. Add test
> coverage for gcc_dump_printf.
> ---
> gcc/c-family/c-format.c | 60 ++++-
> gcc/c-family/c-format.h | 1 +
> gcc/dump-context.h | 7 +-
> gcc/dumpfile.c | 358 +++++++++++++++++++++++++++---
> gcc/dumpfile.h | 20 +-
> gcc/testsuite/gcc.dg/format/gcc_diag-1.c | 15 +-
> gcc/testsuite/gcc.dg/format/gcc_diag-10.c | 26 +++
> 7 files changed, 448 insertions(+), 39 deletions(-)
>
> diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
> index 82841e4..b524468 100644
> --- a/gcc/c-family/c-format.c
> +++ b/gcc/c-family/c-format.c
> @@ -44,6 +44,7 @@ enum format_type { printf_format_type, asm_fprintf_format_type,
> gcc_diag_format_type, gcc_tdiag_format_type,
> gcc_cdiag_format_type,
> gcc_cxxdiag_format_type, gcc_gfc_format_type,
> + gcc_dump_printf_format_type,
> gcc_objc_string_format_type,
> format_type_error = -1};
>
> @@ -56,6 +57,7 @@ struct function_format_info
>
> /* Initialized in init_dynamic_diag_info. */
> static GTY(()) tree local_tree_type_node;
> +static GTY(()) tree local_gimple_ptr_node;
> static GTY(()) tree local_gcall_ptr_node;
> static GTY(()) tree locus;
>
> @@ -461,6 +463,7 @@ static const format_length_info gcc_diag_length_specs[] =
> #define gcc_tdiag_length_specs gcc_diag_length_specs
> #define gcc_cdiag_length_specs gcc_diag_length_specs
> #define gcc_cxxdiag_length_specs gcc_diag_length_specs
> +#define gcc_dump_printf_length_specs gcc_diag_length_specs
>
> /* This differs from printf_length_specs only in that "Z" is not accepted. */
> static const format_length_info scanf_length_specs[] =
> @@ -550,6 +553,7 @@ static const format_flag_pair gcc_diag_flag_pairs[] =
> #define gcc_cdiag_flag_pairs gcc_diag_flag_pairs
> #define gcc_cxxdiag_flag_pairs gcc_diag_flag_pairs
> #define gcc_gfc_flag_pairs gcc_diag_flag_pairs
> +#define gcc_dump_printf_flag_pairs gcc_diag_flag_pairs
>
> static const format_flag_spec gcc_diag_flag_specs[] =
> {
> @@ -565,6 +569,7 @@ static const format_flag_spec gcc_diag_flag_specs[] =
> #define gcc_cdiag_flag_specs gcc_diag_flag_specs
> #define gcc_cxxdiag_flag_specs gcc_diag_flag_specs
> #define gcc_gfc_flag_specs gcc_diag_flag_specs
> +#define gcc_dump_printf_flag_specs gcc_diag_flag_specs
>
> static const format_flag_spec scanf_flag_specs[] =
> {
> @@ -786,6 +791,22 @@ static const format_char_info gcc_gfc_char_table[] =
> { NULL, 0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
> };
>
> +static const format_char_info gcc_dump_printf_char_table[] =
> +{
> + /* The conversion specifiers implemented within pp_format. */
> + PP_FORMAT_CHAR_TABLE,
> +
> + /* Custom conversion specifiers implemented by dump_pretty_printer. */
> +
> + /* E and G require a "gimple *" argument at runtime. */
> + { "EG", 1, STD_C89, { T89_GIMPLE, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "\"", NULL },
> +
> + /* T requires a "tree" at runtime. */
> + { "T", 1, STD_C89, { T89_T, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "\"", NULL },
> +
> + { NULL, 0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
> +};
> +
> static const format_char_info scan_char_table[] =
> {
> /* C89 conversion specifiers. */
> @@ -885,6 +906,13 @@ static const format_kind_info format_types_orig[] =
> 0, 0, 0, 0, 0, 0,
> NULL, NULL
> },
> + { "gcc_dump_printf", gcc_dump_printf_length_specs,
> + gcc_dump_printf_char_table, "q+#", NULL,
> + gcc_dump_printf_flag_specs, gcc_dump_printf_flag_pairs,
> + FMT_FLAG_ARG_CONVERT,
> + 0, 0, 'p', 0, 'L', 0,
> + NULL, &integer_type_node
> + },
> { "NSString", NULL, NULL, NULL, NULL,
> NULL, NULL,
> FMT_FLAG_ARG_CONVERT|FMT_FLAG_PARSE_ARG_CONVERT_EXTERNAL, 0, 0, 0, 0, 0, 0,
> @@ -3838,6 +3866,29 @@ init_dynamic_diag_info (void)
> local_tree_type_node = void_type_node;
> }
>
> + /* Similar to the above but for gimple *. */
> + if (!local_gimple_ptr_node
> + || local_gimple_ptr_node == void_type_node)
> + {
> + if ((local_gimple_ptr_node = maybe_get_identifier ("gimple")))
> + {
> + local_gimple_ptr_node
> + = identifier_global_value (local_gimple_ptr_node);
> + if (local_gimple_ptr_node)
> + {
> + if (TREE_CODE (local_gimple_ptr_node) != TYPE_DECL)
> + {
> + error ("%<gimple%> is not defined as a type");
> + local_gimple_ptr_node = 0;
> + }
> + else
> + local_gimple_ptr_node = TREE_TYPE (local_gimple_ptr_node);
> + }
> + }
> + else
> + local_gimple_ptr_node = void_type_node;
> + }
> +
> /* Similar to the above but for gcall*. */
> if (!local_gcall_ptr_node
> || local_gcall_ptr_node == void_type_node)
> @@ -3905,6 +3956,7 @@ init_dynamic_diag_info (void)
> dynamic_format_types[gcc_tdiag_format_type].length_char_specs =
> dynamic_format_types[gcc_cdiag_format_type].length_char_specs =
> dynamic_format_types[gcc_cxxdiag_format_type].length_char_specs =
> + dynamic_format_types[gcc_dump_printf_format_type].length_char_specs =
> diag_ls = (format_length_info *)
> xmemdup (gcc_diag_length_specs,
> sizeof (gcc_diag_length_specs),
> @@ -3931,6 +3983,8 @@ init_dynamic_diag_info (void)
> gcc_cdiag_char_table;
> dynamic_format_types[gcc_cxxdiag_format_type].conversion_specs =
> gcc_cxxdiag_char_table;
> + dynamic_format_types[gcc_dump_printf_format_type].conversion_specs =
> + gcc_dump_printf_char_table;
> }
>
> #ifdef TARGET_FORMAT_TYPES
> @@ -4085,7 +4139,8 @@ handle_format_attribute (tree *node, tree ARG_UNUSED (name), tree args,
> || info.format_type == gcc_diag_format_type
> || info.format_type == gcc_tdiag_format_type
> || info.format_type == gcc_cdiag_format_type
> - || info.format_type == gcc_cxxdiag_format_type)
> + || info.format_type == gcc_cxxdiag_format_type
> + || info.format_type == gcc_dump_printf_format_type)
> {
> /* Our first time through, we have to make sure that our
> format_type data is allocated dynamically and is modifiable. */
> @@ -4107,7 +4162,8 @@ handle_format_attribute (tree *node, tree ARG_UNUSED (name), tree args,
> else if (info.format_type == gcc_diag_format_type
> || info.format_type == gcc_tdiag_format_type
> || info.format_type == gcc_cdiag_format_type
> - || info.format_type == gcc_cxxdiag_format_type)
> + || info.format_type == gcc_cxxdiag_format_type
> + || info.format_type == gcc_dump_printf_format_type)
> init_dynamic_diag_info ();
> else
> gcc_unreachable ();
> diff --git a/gcc/c-family/c-format.h b/gcc/c-family/c-format.h
> index f828e77..4dfdbdb 100644
> --- a/gcc/c-family/c-format.h
> +++ b/gcc/c-family/c-format.h
> @@ -298,6 +298,7 @@ struct format_kind_info
> #define T_UC &unsigned_char_type_node
> #define T99_UC { STD_C99, NULL, T_UC }
> #define T_V &void_type_node
> +#define T89_GIMPLE { STD_C89, NULL, &local_gimple_ptr_node }
> #define T89_G { STD_C89, NULL, &local_gcall_ptr_node }
> #define T89_T { STD_C89, NULL, &local_tree_type_node }
> #define T89_V { STD_C89, NULL, T_V }
> diff --git a/gcc/dump-context.h b/gcc/dump-context.h
> index f40ea14..54095d3 100644
> --- a/gcc/dump-context.h
> +++ b/gcc/dump-context.h
> @@ -22,6 +22,7 @@ along with GCC; see the file COPYING3. If not see
> #ifndef GCC_DUMP_CONTEXT_H
> #define GCC_DUMP_CONTEXT_H 1
>
> +#include "dumpfile.h" // for ATTRIBUTE_GCC_DUMP_PRINTF
> #include "pretty-print.h"
>
> /* A class for handling the various dump_* calls.
> @@ -73,11 +74,11 @@ class dump_context
> tree t);
>
> void dump_printf_va (dump_flags_t dump_kind, const char *format,
> - va_list ap) ATTRIBUTE_PRINTF (3, 0);
> + va_list *ap) ATTRIBUTE_GCC_DUMP_PRINTF (3, 0);
>
> void dump_printf_loc_va (dump_flags_t dump_kind, const dump_location_t &loc,
> - const char *format, va_list ap)
> - ATTRIBUTE_PRINTF (4, 0);
> + const char *format, va_list *ap)
> + ATTRIBUTE_GCC_DUMP_PRINTF (4, 0);
>
> template<unsigned int N, typename C>
> void dump_dec (dump_flags_t dump_kind, const poly_int<N, C> &value);
> diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
> index 10e9cab..7131f56 100644
> --- a/gcc/dumpfile.c
> +++ b/gcc/dumpfile.c
> @@ -38,6 +38,7 @@ along with GCC; see the file COPYING3. If not see
> #include "cgraph.h"
> #include "tree-pass.h" /* for "current_pass". */
> #include "optinfo-emit-json.h"
> +#include "stringpool.h" /* for get_identifier. */
>
> /* If non-NULL, return one past-the-end of the matching SUBPART of
> the WHOLE string. */
> @@ -681,56 +682,262 @@ dump_context::dump_generic_expr_loc (dump_flags_t dump_kind,
> dump_generic_expr (dump_kind, extra_dump_flags, t);
> }
>
> -/* Make an item for the given dump call. */
> +/* A subclass of pretty_printer for implementing dump_context::dump_printf_va.
> + In particular, the formatted chunks are captured as optinfo_item instances,
> + thus retaining metadata about the entities being dumped (e.g. source
> + locations), rather than just as plain text. */
>
> -static optinfo_item *
> -make_item_for_dump_printf_va (const char *format, va_list ap)
> - ATTRIBUTE_PRINTF (1, 0);
> +class dump_pretty_printer : public pretty_printer
> +{
> +public:
> + dump_pretty_printer (dump_context *context, dump_flags_t dump_kind);
>
> -static optinfo_item *
> -make_item_for_dump_printf_va (const char *format, va_list ap)
> + void emit_items (optinfo *dest);
> +
> +private:
> + /* Information on an optinfo_item that was generated during phase 2 of
> + formatting. */
> + struct stashed_item
> + {
> + stashed_item (const char **buffer_ptr_, optinfo_item *item_)
> + : buffer_ptr (buffer_ptr_), item (item_) {}
> + const char **buffer_ptr;
> + optinfo_item *item;
> + };
> +
> + static bool format_decoder_cb (pretty_printer *pp, text_info *text,
> + const char *spec, int /*precision*/,
> + bool /*wide*/, bool /*set_locus*/,
> + bool /*verbose*/, bool */*quoted*/,
> + const char **buffer_ptr);
> +
> + bool decode_format (text_info *text, const char *spec,
> + const char **buffer_ptr);
> +
> + void stash_item (const char **buffer_ptr, optinfo_item *item);
> +
> + void emit_any_pending_textual_chunks (optinfo *dest);
> +
> + void emit_item (optinfo_item *item, optinfo *dest);
> +
> + dump_context *m_context;
> + dump_flags_t m_dump_kind;
> + auto_vec<stashed_item> m_stashed_items;
> +};
> +
> +/* dump_pretty_printer's ctor. */
> +
> +dump_pretty_printer::dump_pretty_printer (dump_context *context,
> + dump_flags_t dump_kind)
> +: pretty_printer (), m_context (context), m_dump_kind (dump_kind),
> + m_stashed_items ()
> +{
> + pp_format_decoder (this) = format_decoder_cb;
> +}
> +
> +/* Phase 3 of formatting; compare with pp_output_formatted_text.
> +
> + Emit optinfo_item instances for the various formatted chunks from phases
> + 1 and 2 (i.e. pp_format).
> +
> + Some chunks may already have had their items built (during decode_format).
> + These chunks have been stashed into m_stashed_items; we emit them here.
> +
> + For all other purely textual chunks, they are printed into
> + buffer->formatted_obstack, and then emitted as a textual optinfo_item.
> + This consolidates multiple adjacent text chunks into a single text
> + optinfo_item. */
> +
> +void
> +dump_pretty_printer::emit_items (optinfo *dest)
> +{
> + output_buffer *buffer = pp_buffer (this);
> + struct chunk_info *chunk_array = buffer->cur_chunk_array;
> + const char **args = chunk_array->args;
> +
> + gcc_assert (buffer->obstack == &buffer->formatted_obstack);
> + gcc_assert (buffer->line_length == 0);
> +
> + unsigned stashed_item_idx = 0;
> + for (unsigned chunk = 0; args[chunk]; chunk++)
> + {
> + if (stashed_item_idx < m_stashed_items.length ()
> + && args[chunk] == *m_stashed_items[stashed_item_idx].buffer_ptr)
> + {
> + emit_any_pending_textual_chunks (dest);
> + /* This chunk has a stashed item: use it. */
> + emit_item (m_stashed_items[stashed_item_idx++].item, dest);
> + }
> + else
> + /* This chunk is purely textual. Print it (to
> + buffer->formatted_obstack), so that we can consolidate adjacent
> + chunks into one textual optinfo_item. */
> + pp_string (this, args[chunk]);
> + }
> +
> + emit_any_pending_textual_chunks (dest);
> +
> + /* Ensure that we consumed all of stashed_items. */
> + gcc_assert (stashed_item_idx == m_stashed_items.length ());
> +
> + /* Deallocate the chunk structure and everything after it (i.e. the
> + associated series of formatted strings). */
> + buffer->cur_chunk_array = chunk_array->prev;
> + obstack_free (&buffer->chunk_obstack, chunk_array);
> +}
> +
> +/* Subroutine of dump_pretty_printer::emit_items
> + for consolidating multiple adjacent pure-text chunks into single
> + optinfo_items (in phase 3). */
> +
> +void
> +dump_pretty_printer::emit_any_pending_textual_chunks (optinfo *dest)
> {
> - char *formatted_text = xvasprintf (format, ap);
> + gcc_assert (buffer->obstack == &buffer->formatted_obstack);
> +
> + /* Don't emit an item if the pending text is empty. */
> + if (output_buffer_last_position_in_text (buffer) == NULL)
> + return;
> +
> + char *formatted_text = xstrdup (pp_formatted_text (this));
> optinfo_item *item
> = new optinfo_item (OPTINFO_ITEM_KIND_TEXT, UNKNOWN_LOCATION,
> formatted_text);
> - return item;
> + emit_item (item, dest);
> +
> + /* Clear the pending text by unwinding formatted_text back to the start
> + of the buffer (without deallocating). */
> + obstack_free (&buffer->formatted_obstack,
> + buffer->formatted_obstack.object_base);
> }
>
> -/* Make an item for the given dump call. */
> +/* Emit ITEM and take ownership of it. If DEST is non-NULL, add ITEM
> + to DEST; otherwise delete ITEM. */
>
> -static optinfo_item *
> -make_item_for_dump_printf (const char *format, ...)
> - ATTRIBUTE_PRINTF (1, 2);
> +void
> +dump_pretty_printer::emit_item (optinfo_item *item, optinfo *dest)
> +{
> + m_context->emit_item (item, m_dump_kind);
> + if (dest)
> + dest->add_item (item);
> + else
> + delete item;
> +}
>
> -static optinfo_item *
> -make_item_for_dump_printf (const char *format, ...)
> +/* Record that ITEM (generated in phase 2 of formatting) is to be used for
> + the chunk at BUFFER_PTR in phase 3 (by emit_items). */
> +
> +void
> +dump_pretty_printer::stash_item (const char **buffer_ptr, optinfo_item *item)
> {
> - va_list ap;
> - va_start (ap, format);
> - optinfo_item *item
> - = make_item_for_dump_printf_va (format, ap);
> - va_end (ap);
> - return item;
> + gcc_assert (buffer_ptr);
> + gcc_assert (item);
> +
> + m_stashed_items.safe_push (stashed_item (buffer_ptr, item));
> +}
> +
> +/* pp_format_decoder callback for dump_pretty_printer, and thus for
> + dump_printf and dump_printf_loc.
> +
> + A wrapper around decode_format, for type-safety. */
> +
> +bool
> +dump_pretty_printer::format_decoder_cb (pretty_printer *pp, text_info *text,
> + const char *spec, int /*precision*/,
> + bool /*wide*/, bool /*set_locus*/,
> + bool /*verbose*/, bool */*quoted*/,
> + const char **buffer_ptr)
> +{
> + dump_pretty_printer *opp = static_cast <dump_pretty_printer *> (pp);
> + return opp->decode_format (text, spec, buffer_ptr);
> +}
> +
> +/* Format decoder for dump_pretty_printer, and thus for dump_printf and
> + dump_printf_loc.
> +
> + Supported format codes (in addition to the standard pretty_printer ones)
> + are:
> +
> + %E: gimple *:
> + Equivalent to: dump_gimple_expr (MSG_*, TDF_SLIM, stmt, 0)
> + %G: gimple *:
> + Equivalent to: dump_gimple_stmt (MSG_*, TDF_SLIM, stmt, 0)
> + %T: tree:
> + Equivalent to: dump_generic_expr (MSG_*, arg, TDF_SLIM).
> +
> + FIXME: add symtab_node?
> +
> + These format codes build optinfo_item instances, thus capturing metadata
> + about the arguments being dumped, as well as the textual output. */
> +
> +bool
> +dump_pretty_printer::decode_format (text_info *text, const char *spec,
> + const char **buffer_ptr)
> +{
> + /* Various format codes that imply making an optinfo_item and stashed it
> + for later use (to capture metadata, rather than plain text). */
> + switch (*spec)
> + {
> + case 'E':
> + {
> + gimple *stmt = va_arg (*text->args_ptr, gimple *);
> +
> + /* Make an item for the stmt, and stash it. */
> + optinfo_item *item = make_item_for_dump_gimple_expr (stmt, 0, TDF_SLIM);
> + stash_item (buffer_ptr, item);
> + return true;
> + }
> +
> + case 'G':
> + {
> + gimple *stmt = va_arg (*text->args_ptr, gimple *);
> +
> + /* Make an item for the stmt, and stash it. */
> + optinfo_item *item = make_item_for_dump_gimple_stmt (stmt, 0, TDF_SLIM);
> + stash_item (buffer_ptr, item);
> + return true;
> + }
> +
> + case 'T':
> + {
> + tree t = va_arg (*text->args_ptr, tree);
> +
> + /* Make an item for the tree, and stash it. */
> + optinfo_item *item = make_item_for_dump_generic_expr (t, TDF_SLIM);
> + stash_item (buffer_ptr, item);
> + return true;
> + }
> +
> + default:
> + return false;
> + }
> }
>
> /* Output a formatted message using FORMAT on appropriate dump streams. */
>
> void
> dump_context::dump_printf_va (dump_flags_t dump_kind, const char *format,
> - va_list ap)
> + va_list *ap)
> {
> - optinfo_item *item = make_item_for_dump_printf_va (format, ap);
> - emit_item (item, dump_kind);
> + dump_pretty_printer pp (this, dump_kind);
>
> + text_info text;
> + text.err_no = errno;
> + text.args_ptr = ap;
> + text.format_spec = format;
> +
> + /* Phases 1 and 2, using pp_format. */
> + pp_format (&pp, &text);
> +
> + /* Phase 3. */
> if (optinfo_enabled_p ())
> {
> optinfo &info = ensure_pending_optinfo ();
> info.handle_dump_file_kind (dump_kind);
> - info.add_item (item);
> + pp.emit_items (&info);
> }
> else
> - delete item;
> + pp.emit_items (NULL);
> }
>
> /* Similar to dump_printf, except source location is also printed, and
> @@ -739,7 +946,7 @@ dump_context::dump_printf_va (dump_flags_t dump_kind, const char *format,
> void
> dump_context::dump_printf_loc_va (dump_flags_t dump_kind,
> const dump_location_t &loc,
> - const char *format, va_list ap)
> + const char *format, va_list *ap)
> {
> dump_loc (dump_kind, loc);
> dump_printf_va (dump_kind, format, ap);
> @@ -851,7 +1058,11 @@ dump_context::begin_scope (const char *name, const dump_location_t &loc)
> if (m_test_pp)
> ::dump_loc (MSG_NOTE, m_test_pp, loc.get_location_t ());
>
> - optinfo_item *item = make_item_for_dump_printf ("=== %s ===\n", name);
> + pretty_printer pp;
> + pp_printf (&pp, "=== %s ===\n", name);
> + optinfo_item *item
> + = new optinfo_item (OPTINFO_ITEM_KIND_TEXT, UNKNOWN_LOCATION,
> + xstrdup (pp_formatted_text (&pp)));
> emit_item (item, MSG_NOTE);
>
> if (optinfo_enabled_p ())
> @@ -859,7 +1070,6 @@ dump_context::begin_scope (const char *name, const dump_location_t &loc)
> optinfo &info = begin_next_optinfo (loc);
> info.m_kind = OPTINFO_KIND_SCOPE;
> info.add_item (item);
> - end_any_optinfo ();
> }
> else
> delete item;
> @@ -1006,7 +1216,7 @@ dump_printf (dump_flags_t dump_kind, const char *format, ...)
> {
> va_list ap;
> va_start (ap, format);
> - dump_context::get ().dump_printf_va (dump_kind, format, ap);
> + dump_context::get ().dump_printf_va (dump_kind, format, &ap);
> va_end (ap);
> }
>
> @@ -1019,7 +1229,7 @@ dump_printf_loc (dump_flags_t dump_kind, const dump_location_t &loc,
> {
> va_list ap;
> va_start (ap, format);
> - dump_context::get ().dump_printf_loc_va (dump_kind, loc, format, ap);
> + dump_context::get ().dump_printf_loc_va (dump_kind, loc, format, &ap);
> va_end (ap);
> }
>
> @@ -1808,9 +2018,12 @@ test_capture_of_dump_calls (const line_table_case &case_)
>
> dump_location_t loc = dump_location_t::from_location_t (where);
>
> - greturn *stmt = gimple_build_return (NULL);
> + gimple *stmt = gimple_build_return (NULL);
> gimple_set_location (stmt, where);
>
> + tree test_decl = build_decl (UNKNOWN_LOCATION, VAR_DECL,
> + get_identifier ("test_decl"),
> + integer_type_node);
> /* Run all tests twice, with and then without optinfo enabled, to ensure
> that immediate destinations vs optinfo-based destinations both
> work, independently of each other, with no leaks. */
> @@ -1834,6 +2047,89 @@ test_capture_of_dump_calls (const line_table_case &case_)
> }
> }
>
> + /* Test of dump_printf with %T. */
> + {
> + temp_dump_context tmp (with_optinfo, MSG_ALL);
> + dump_printf (MSG_NOTE, "tree: %T", integer_zero_node);
> +
> + ASSERT_DUMPED_TEXT_EQ (tmp, "tree: 0");
> + if (with_optinfo)
> + {
> + optinfo *info = tmp.get_pending_optinfo ();
> + ASSERT_TRUE (info != NULL);
> + ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> + ASSERT_EQ (info->num_items (), 2);
> + ASSERT_IS_TEXT (info->get_item (0), "tree: ");
> + ASSERT_IS_TREE (info->get_item (1), UNKNOWN_LOCATION, "0");
> + }
> + }
> +
> + /* Test of dump_printf with %E. */
> + {
> + temp_dump_context tmp (with_optinfo, MSG_ALL);
> + dump_printf (MSG_NOTE, "gimple: %E", stmt);
> +
> + ASSERT_DUMPED_TEXT_EQ (tmp, "gimple: return;");
> + if (with_optinfo)
> + {
> + optinfo *info = tmp.get_pending_optinfo ();
> + ASSERT_TRUE (info != NULL);
> + ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> + ASSERT_EQ (info->num_items (), 2);
> + ASSERT_IS_TEXT (info->get_item (0), "gimple: ");
> + ASSERT_IS_GIMPLE (info->get_item (1), where, "return;");
> + }
> + }
> +
> + /* Test of dump_printf with %G. */
> + {
> + temp_dump_context tmp (with_optinfo, MSG_ALL);
> + dump_printf (MSG_NOTE, "gimple: %G", stmt);
> +
> + ASSERT_DUMPED_TEXT_EQ (tmp, "gimple: return;\n");
> + if (with_optinfo)
> + {
> + optinfo *info = tmp.get_pending_optinfo ();
> + ASSERT_TRUE (info != NULL);
> + ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> + ASSERT_EQ (info->num_items (), 2);
> + ASSERT_IS_TEXT (info->get_item (0), "gimple: ");
> + ASSERT_IS_GIMPLE (info->get_item (1), where, "return;\n");
> + }
> + }
> +
> + /* dump_print_loc with multiple format codes. This tests various
> + things:
> + - intermingling of text, format codes handled by the base
> + pretty_printer, and dump-specific format codes
> + - multiple dump-specific format codes: some consecutive, others
> + separated by text, trailing text after the final one. */
> + {
> + temp_dump_context tmp (with_optinfo, MSG_ALL);
> + dump_printf_loc (MSG_NOTE, loc, "before %T and %T"
> + " %i consecutive %E%E after\n",
> + integer_zero_node, test_decl, 42, stmt, stmt);
> +
> + ASSERT_DUMPED_TEXT_EQ (tmp,
> + "test.txt:5:10: note: before 0 and test_decl"
> + " 42 consecutive return;return; after\n");
> + if (with_optinfo)
> + {
> + optinfo *info = tmp.get_pending_optinfo ();
> + ASSERT_TRUE (info != NULL);
> + ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> + ASSERT_EQ (info->num_items (), 8);
> + ASSERT_IS_TEXT (info->get_item (0), "before ");
> + ASSERT_IS_TREE (info->get_item (1), UNKNOWN_LOCATION, "0");
> + ASSERT_IS_TEXT (info->get_item (2), " and ");
> + ASSERT_IS_TREE (info->get_item (3), UNKNOWN_LOCATION, "test_decl");
> + ASSERT_IS_TEXT (info->get_item (4), " 42 consecutive ");
> + ASSERT_IS_GIMPLE (info->get_item (5), where, "return;");
> + ASSERT_IS_GIMPLE (info->get_item (6), where, "return;");
> + ASSERT_IS_TEXT (info->get_item (7), " after\n");
> + }
> + }
> +
> /* Tree, via dump_generic_expr. */
> {
> temp_dump_context tmp (with_optinfo, MSG_ALL);
> diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h
> index 2b174e5..1fe64cb 100644
> --- a/gcc/dumpfile.h
> +++ b/gcc/dumpfile.h
> @@ -23,6 +23,19 @@ along with GCC; see the file COPYING3. If not see
>
> #include "profile-count.h"
>
> +/* An attribute for annotating formatting printing functions that use
> + the dumpfile/optinfo formatting codes. These are the pretty_printer
> + format codes (see pretty-print.c), with additional codes for middle-end
> + specific entities (see dumpfile.c). */
> +
> +#if GCC_VERSION >= 3005
> +#define ATTRIBUTE_GCC_DUMP_PRINTF(m, n) \
> + __attribute__ ((__format__ (__gcc_dump_printf__, m ,n))) \
> + ATTRIBUTE_NONNULL(m)
> +#else
> +#define ATTRIBUTE_GCC_DUMP_PRINTF(m, n) ATTRIBUTE_NONNULL(m)
> +#endif
> +
> /* Different tree dump places. When you add new tree dump places,
> extend the DUMP_FILES array in dumpfile.c. */
> enum tree_dump_index
> @@ -476,9 +489,12 @@ dump_enabled_p (void)
> to minimize the work done for the common case where dumps
> are disabled. */
>
> -extern void dump_printf (dump_flags_t, const char *, ...) ATTRIBUTE_PRINTF_2;
> +extern void dump_printf (dump_flags_t, const char *, ...)
> + ATTRIBUTE_GCC_DUMP_PRINTF (2, 3);
> +
> extern void dump_printf_loc (dump_flags_t, const dump_location_t &,
> - const char *, ...) ATTRIBUTE_PRINTF_3;
> + const char *, ...)
> + ATTRIBUTE_GCC_DUMP_PRINTF (3, 0);
> extern void dump_function (int phase, tree fn);
> extern void dump_basic_block (dump_flags_t, basic_block, int);
> extern void dump_generic_expr_loc (dump_flags_t, const dump_location_t &,
> diff --git a/gcc/testsuite/gcc.dg/format/gcc_diag-1.c b/gcc/testsuite/gcc.dg/format/gcc_diag-1.c
> index 034e097..8761456 100644
> --- a/gcc/testsuite/gcc.dg/format/gcc_diag-1.c
> +++ b/gcc/testsuite/gcc.dg/format/gcc_diag-1.c
> @@ -1,4 +1,4 @@
> -/* Test for GCC diagnositc formats. */
> +/* Test for GCC diagnostic formats. */
> /* Origin: Kaveh Ghazi <ghazi@caip.rutgers.edu> */
> /* { dg-do compile } */
> /* { dg-options "-Wformat" } */
> @@ -24,6 +24,7 @@ extern int diag (const char *, ...) ATTRIBUTE_DIAG(__gcc_diag__);
> extern int tdiag (const char *, ...) ATTRIBUTE_DIAG(__gcc_tdiag__);
> extern int cdiag (const char *, ...) ATTRIBUTE_DIAG(__gcc_cdiag__);
> extern int cxxdiag (const char *, ...) ATTRIBUTE_DIAG(__gcc_cxxdiag__);
> +extern int dump (const char *, ...) ATTRIBUTE_DIAG(__gcc_dump_printf__);
>
> void
> foo (int i, int i1, int i2, unsigned int u, double d, char *s, void *p,
> @@ -39,36 +40,44 @@ foo (int i, int i1, int i2, unsigned int u, double d, char *s, void *p,
> tdiag ("%%");
> cdiag ("%%");
> cxxdiag ("%%");
> + dump ("%%");
> diag ("%d%i%o%u%x%c%s%p%%", i, i, u, u, u, i, s, p);
> tdiag ("%d%i%o%u%x%c%s%p%%", i, i, u, u, u, i, s, p);
> cdiag ("%d%i%o%u%x%c%s%p%%", i, i, u, u, u, i, s, p);
> cxxdiag ("%d%i%o%u%x%c%s%p%%", i, i, u, u, u, i, s, p);
> + dump ("%d%i%o%u%x%c%s%p%%", i, i, u, u, u, i, s, p);
> diag ("%qd%qi%qo%qu%qx%qc%qs%qp%<%%%'%>", i, i, u, u, u, i, s, p);
> tdiag ("%qd%qi%qo%qu%qx%qc%qs%qp%<%%%'%>", i, i, u, u, u, i, s, p);
> cdiag ("%qd%qi%qo%qu%qx%qc%qs%qp%<%%%'%>", i, i, u, u, u, i, s, p);
> cxxdiag ("%qd%qi%qo%qu%qx%qc%qs%qp%<%%%'%>", i, i, u, u, u, i, s, p);
> + dump ("%qd%qi%qo%qu%qx%qc%qs%qp%<%%%'%>", i, i, u, u, u, i, s, p);
> diag ("%ld%li%lo%lu%lx", l, l, ul, ul, ul);
> tdiag ("%ld%li%lo%lu%lx", l, l, ul, ul, ul);
> cdiag ("%ld%li%lo%lu%lx", l, l, ul, ul, ul);
> cxxdiag ("%ld%li%lo%lu%lx", l, l, ul, ul, ul);
> + dump ("%ld%li%lo%lu%lx", l, l, ul, ul, ul);
> diag ("%lld%lli%llo%llu%llx", ll, ll, ull, ull, ull);
> tdiag ("%lld%lli%llo%llu%llx", ll, ll, ull, ull, ull);
> cdiag ("%lld%lli%llo%llu%llx", ll, ll, ull, ull, ull);
> cxxdiag ("%lld%lli%llo%llu%llx", ll, ll, ull, ull, ull);
> + dump ("%lld%lli%llo%llu%llx", ll, ll, ull, ull, ull);
> diag ("%wd%wi%wo%wu%wx", ll, ll, ull, ull, ull);
> tdiag ("%wd%wi%wo%wu%wx", ll, ll, ull, ull, ull);
> cdiag ("%wd%wi%wo%wu%wx", ll, ll, ull, ull, ull);
> cxxdiag ("%wd%wi%wo%wu%wx", ll, ll, ull, ull, ull);
> + dump ("%wd%wi%wo%wu%wx", ll, ll, ull, ull, ull);
> diag ("%.*s", i, s);
> tdiag ("%.*s", i, s);
> cdiag ("%.*s", i, s);
> cxxdiag ("%.*s", i, s);
> + dump ("%.*s", i, s);
>
> /* Extensions provided in the diagnostic framework. */
> diag ("%m");
> tdiag ("%m");
> cdiag ("%m");
> cxxdiag ("%m");
> + dump ("%m");
>
> /* Quote directives to avoid "warning: conversion used unquoted." */
> tdiag ("%<%D%F%T%V%>", t1, t1, t1, t1);
> @@ -94,20 +103,24 @@ foo (int i, int i1, int i2, unsigned int u, double d, char *s, void *p,
> tdiag ("%Z", v, v_len);
> cdiag ("%Z", v, v_len);
> cxxdiag ("%Z", v, v_len);
> + dump ("%Z", v, v_len);
>
> /* Bad stuff with extensions. */
> diag ("%m", i); /* { dg-warning "format" "extra arg" } */
> tdiag ("%m", i); /* { dg-warning "format" "extra arg" } */
> cdiag ("%m", i); /* { dg-warning "format" "extra arg" } */
> cxxdiag ("%m", i); /* { dg-warning "format" "extra arg" } */
> + dump ("%m", i); /* { dg-warning "format" "extra arg" } */
> diag ("%#m"); /* { dg-warning "format" "bogus modifier" } */
> tdiag ("%#m"); /* { dg-warning "format" "bogus modifier" } */
> cdiag ("%#m"); /* { dg-warning "format" "bogus modifier" } */
> cxxdiag ("%#m"); /* { dg-warning "format" "bogus modifier" } */
> + dump ("%#m"); /* { dg-warning "format" "bogus modifier" } */
> diag ("%+m"); /* { dg-warning "format" "bogus modifier" } */
> tdiag ("%+m"); /* { dg-warning "format" "bogus modifier" } */
> cdiag ("%+m"); /* { dg-warning "format" "bogus modifier" } */
> cxxdiag ("%+m"); /* { dg-warning "format" "bogus modifier" } */
> + dump ("%+m"); /* { dg-warning "format" "bogus modifier" } */
> diag ("%D", t1); /* { dg-warning "format" "bogus tree" } */
> tdiag ("%A", t1); /* { dg-warning "format" "bogus tree" } */
> tdiag ("%E", t1);
> diff --git a/gcc/testsuite/gcc.dg/format/gcc_diag-10.c b/gcc/testsuite/gcc.dg/format/gcc_diag-10.c
> index ab9bc2f..8b5aadf1 100644
> --- a/gcc/testsuite/gcc.dg/format/gcc_diag-10.c
> +++ b/gcc/testsuite/gcc.dg/format/gcc_diag-10.c
> @@ -19,12 +19,16 @@ typedef union tree_node *tree;
> the C test to find the symbol. */
> typedef struct gcall gcall;
>
> +/* Likewise for gimple. */
> +typedef struct gimple gimple;
> +
> #define FORMAT(kind) __attribute__ ((format (__gcc_## kind ##__, 1, 2)))
>
> void diag (const char*, ...) FORMAT (diag);
> void cdiag (const char*, ...) FORMAT (cdiag);
> void tdiag (const char*, ...) FORMAT (tdiag);
> void cxxdiag (const char*, ...) FORMAT (cxxdiag);
> +void dump (const char*, ...) FORMAT (dump_printf);
>
> void test_diag (tree t, gcall *gc)
> {
> @@ -157,3 +161,25 @@ void test_cxxdiag (tree t, gcall *gc)
> cxxdiag ("%<%V%>", t);
> cxxdiag ("%<%X%>", t);
> }
> +
> +void test_dump (tree t, gimple *stmt)
> +{
> + dump ("%<"); /* { dg-warning "unterminated quoting directive" } */
> + dump ("%>"); /* { dg-warning "unmatched quoting directive " } */
> + dump ("%<foo%<bar%>%>"); /* { dg-warning "nested quoting directive" } */
> +
> + dump ("%R"); /* { dg-warning "unmatched color reset directive" } */
> + dump ("%r", ""); /* { dg-warning "unterminated color directive" } */
> + dump ("%r%r", "", ""); /* { dg-warning "unterminated color directive" } */
> + dump ("%r%R", "");
> + dump ("%r%r%R", "", "");
> + dump ("%r%R%r%R", "", "");
> +
> + dump ("%<%R%>"); /* { dg-warning "unmatched color reset directive" } */
> + dump ("%<%r%>", ""); /* { dg-warning "unterminated color directive" } */
> + dump ("%<%r%R%>", "");
> +
> + dump ("%E", stmt);
> + dump ("%T", t);
> + dump ("%G", stmt);
> +}
> --
> 1.8.5.3
>
next prev parent reply other threads:[~2018-07-31 13:03 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-27 21:47 [PATCH 0/5] dump_printf support for middle-end types David Malcolm
2018-07-27 21:47 ` [PATCH 3/5] C++: clean up cp_printer David Malcolm
2018-07-28 14:06 ` Jason Merrill
2018-07-27 21:47 ` [PATCH 4/5] c-family: clean up the data tables in c-format.c David Malcolm
2018-07-31 12:56 ` Richard Biener
2018-07-31 13:08 ` Marek Polacek
2018-07-27 21:47 ` [PATCH 2/5] dumpfile.c: eliminate special-casing of dump_file/alt_dump_file David Malcolm
2018-07-31 12:54 ` Richard Biener
2018-07-31 15:34 ` David Malcolm
2018-07-31 15:37 ` Richard Biener
2018-07-27 21:47 ` [PATCH 5/5] Formatted printing for dump_* in the middle-end David Malcolm
2018-07-31 13:03 ` Richard Biener [this message]
2018-07-31 14:19 ` David Malcolm
2018-07-31 14:21 ` Richard Biener
2018-07-31 14:33 ` Richard Biener
2018-07-31 19:56 ` Joseph Myers
2018-08-02 17:09 ` [PATCH] v2: " David Malcolm
2018-08-09 22:11 ` Joseph Myers
2018-08-17 4:08 ` Jeff Law
2018-08-17 18:24 ` David Malcolm
2018-08-27 6:58 ` Jakub Jelinek
2018-08-27 23:46 ` [PATCH] Fix version check for ATTRIBUTE_GCC_DUMP_PRINTF David Malcolm
2018-08-28 6:44 ` Jakub Jelinek
2018-08-28 12:26 ` Jakub Jelinek
2018-08-28 14:19 ` David Malcolm
2018-07-27 21:47 ` [PATCH 1/5] Simplify dump_context by adding a dump_loc member function David Malcolm
2018-07-31 12:51 ` Richard Biener
2018-07-31 12:50 ` [PATCH 0/5] dump_printf support for middle-end types Richard Biener
2018-07-31 14:01 ` David Malcolm
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFiYyc1f3hRRq11t3yNR508DiN205ut+SOu_xT5_PoR6=BfQ7w@mail.gmail.com' \
--to=richard.guenther@gmail.com \
--cc=dmalcolm@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).