Previously diagnostic_context::report_diagnostic had, after the call to pp_format (phases 1 and 2 of formatting the message):
m_output_format->on_begin_diagnostic (*diagnostic); pp_output_formatted_text (this->printer, m_urlifier); if (m_show_cwe) print_any_cwe (*diagnostic); if (m_show_rules) print_any_rules (*diagnostic); if (m_show_option_requested) print_option_information (*diagnostic, orig_diag_kind); m_output_format->on_end_diagnostic (*diagnostic, orig_diag_kind); This patch replaces all of the above with a single call to m_output_format->on_report_diagnostic (*diagnostic, orig_diag_kind); moving responsibility for phase 3 of formatting and printing the result from diagnostic_context to the output format. This simplifies diagnostic_context::report_diagnostic and allows us to move the code that prints CWEs, rules, and option information in textual form from diagnostic_context to diagnostic_text_output_format, where it belongs. No functional change intended. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r15-3200-gac707d30ce449f. gcc/ChangeLog: * diagnostic-format-json.cc (json_output_format::on_begin_diagnostic): Delete. (json_output_format::on_end_diagnostic): Rename to... (json_output_format::on_report_diagnostic): ...this and add call to pp_output_formatted_text. (diagnostic_output_format_init_json): Drop unnecessary calls to disable textual printing of CWEs, rules, and options. * diagnostic-format-sarif.cc (sarif_builder::end_diagnostic): Rename to... (sarif_builder::on_report_diagnostic): ...this and add call to pp_output_formatted_text. (sarif_output_format::on_begin_diagnostic): Delete. (sarif_output_format::on_end_diagnostic): Rename to... (sarif_output_format::on_report_diagnostic): ...this and update call to m_builder accordingly. (diagnostic_output_format_init_sarif): Drop unnecessary calls to disable textual printing of CWEs, rules, and options. * diagnostic.cc (diagnostic_context::print_any_cwe): Convert to... (diagnostic_text_output_format::print_any_cwe): ...this. (diagnostic_context::print_any_rules): Convert to... (diagnostic_text_output_format::print_any_rules): ...this. (diagnostic_context::print_option_information): Convert to... (diagnostic_text_output_format::print_option_information): ...this. (diagnostic_context::report_diagnostic): Replace calls to the output format's on_begin_diagnostic, to pp_output_formatted_text, printing CWE, rules, option info, and the call to the format's on_end_diagnostic with a call to the format's on_report_diagnostic. (diagnostic_text_output_format::on_begin_diagnostic): Delete. (diagnostic_text_output_format::on_end_diagnostic): Delete. (diagnostic_text_output_format::on_report_diagnostic): New vfunc, which effectively does the on_begin_diagnostic, the call to pp_output_formatted_text, the calls for printing CWE, rules, option info, and the call to the diagnostic_finalizer. * diagnostic.h (diagnostic_output_format::on_begin_diagnostic): Delete. (diagnostic_output_format::on_end_diagnostic): Delete. (diagnostic_output_format::on_report_diagnostic): New. (diagnostic_text_output_format::on_begin_diagnostic): Delete. (diagnostic_text_output_format::on_end_diagnostic): Delete. (diagnostic_text_output_format::on_report_diagnostic): New. (class diagnostic_context): Add friend class diagnostic_text_output_format. (diagnostic_context::get_urlifier): New accessor. (diagnostic_context::print_any_cwe): Move decl... (diagnostic_text_output_format::print_any_cwe): ...to here. (diagnostic_context::print_any_rules): Move decl... (diagnostic_text_output_format::print_any_rules): ...to here. (diagnostic_context::print_option_information): Move decl... (diagnostic_text_output_format::print_option_information): ...to here. Signed-off-by: David Malcolm <dmalc...@redhat.com> --- gcc/diagnostic-format-json.cc | 24 +-- gcc/diagnostic-format-sarif.cc | 34 ++--- gcc/diagnostic.cc | 261 +++++++++++++++++---------------- gcc/diagnostic.h | 28 ++-- 4 files changed, 171 insertions(+), 176 deletions(-) diff --git a/gcc/diagnostic-format-json.cc b/gcc/diagnostic-format-json.cc index b78cb92cfd7a..f2e9d0d79e51 100644 --- a/gcc/diagnostic-format-json.cc +++ b/gcc/diagnostic-format-json.cc @@ -47,13 +47,8 @@ public: m_cur_children_array = nullptr; } void - on_begin_diagnostic (const diagnostic_info &) final override - { - /* No-op. */ - } - void - on_end_diagnostic (const diagnostic_info &diagnostic, - diagnostic_t orig_diag_kind) final override; + on_report_diagnostic (const diagnostic_info &diagnostic, + diagnostic_t orig_diag_kind) final override; void on_diagram (const diagnostic_diagram &) final override { /* No-op. */ @@ -225,14 +220,16 @@ make_json_for_path (diagnostic_context &context, } -/* Implementation of "on_end_diagnostic" vfunc for JSON output. +/* Implementation of "on_report_diagnostic" vfunc for JSON output. Generate a JSON object for DIAGNOSTIC, and store for output within current diagnostic group. */ void -json_output_format::on_end_diagnostic (const diagnostic_info &diagnostic, - diagnostic_t orig_diag_kind) +json_output_format::on_report_diagnostic (const diagnostic_info &diagnostic, + diagnostic_t orig_diag_kind) { + pp_output_formatted_text (m_context.printer, m_context.get_urlifier ()); + json::object *diag_obj = new json::object (); /* Get "kind" of diagnostic. */ @@ -395,13 +392,6 @@ diagnostic_output_format_init_json (diagnostic_context &context) /* Suppress normal textual path output. */ context.set_path_format (DPF_NONE); - /* The metadata is handled in JSON format, rather than as text. */ - context.set_show_cwe (false); - context.set_show_rules (false); - - /* The option is handled in JSON format, rather than as text. */ - context.set_show_option_requested (false); - /* Don't colorize the text. */ pp_show_color (context.printer) = false; context.set_show_highlight_colors (false); diff --git a/gcc/diagnostic-format-sarif.cc b/gcc/diagnostic-format-sarif.cc index 1d99c904ff0c..554bf3cb2d5c 100644 --- a/gcc/diagnostic-format-sarif.cc +++ b/gcc/diagnostic-format-sarif.cc @@ -592,9 +592,9 @@ public: const char *main_input_filename_, bool formatted); - void end_diagnostic (diagnostic_context &context, - const diagnostic_info &diagnostic, - diagnostic_t orig_diag_kind); + void on_report_diagnostic (diagnostic_context &context, + const diagnostic_info &diagnostic, + diagnostic_t orig_diag_kind); void emit_diagram (diagnostic_context &context, const diagnostic_diagram &diagram); void end_group (); @@ -1350,13 +1350,15 @@ sarif_builder::sarif_builder (diagnostic_context &context, false); } -/* Implementation of "end_diagnostic" for SARIF output. */ +/* Implementation of "on_report_diagnostic" for SARIF output. */ void -sarif_builder::end_diagnostic (diagnostic_context &context, - const diagnostic_info &diagnostic, - diagnostic_t orig_diag_kind) +sarif_builder::on_report_diagnostic (diagnostic_context &context, + const diagnostic_info &diagnostic, + diagnostic_t orig_diag_kind) { + pp_output_formatted_text (context.printer, context.get_urlifier ()); + if (diagnostic.kind == DK_ICE || diagnostic.kind == DK_ICE_NOBT) { m_invocation_obj->add_notification_for_ice (context, diagnostic, *this); @@ -2920,15 +2922,10 @@ public: m_builder.end_group (); } void - on_begin_diagnostic (const diagnostic_info &) final override + on_report_diagnostic (const diagnostic_info &diagnostic, + diagnostic_t orig_diag_kind) final override { - /* No-op, */ - } - void - on_end_diagnostic (const diagnostic_info &diagnostic, - diagnostic_t orig_diag_kind) final override - { - m_builder.end_diagnostic (m_context, diagnostic, orig_diag_kind); + m_builder.on_report_diagnostic (m_context, diagnostic, orig_diag_kind); } void on_diagram (const diagnostic_diagram &diagram) final override { @@ -3022,13 +3019,6 @@ diagnostic_output_format_init_sarif (diagnostic_context &context) /* Override callbacks. */ context.set_ice_handler_callback (sarif_ice_handler); - /* The metadata is handled in SARIF format, rather than as text. */ - context.set_show_cwe (false); - context.set_show_rules (false); - - /* The option is handled in SARIF format, rather than as text. */ - context.set_show_option_requested (false); - /* Don't colorize the text. */ pp_show_color (context.printer) = false; context.set_show_highlight_colors (false); diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc index 92bd4f808453..497bbe79705c 100644 --- a/gcc/diagnostic.cc +++ b/gcc/diagnostic.cc @@ -1232,117 +1232,6 @@ get_cwe_url (int cwe) return xasprintf ("https://cwe.mitre.org/data/definitions/%i.html", cwe); } -/* If DIAGNOSTIC has a CWE identifier, print it. - - For example, if the diagnostic metadata associates it with CWE-119, - " [CWE-119]" will be printed, suitably colorized, and with a URL of a - description of the security issue. */ - -void -diagnostic_context::print_any_cwe (const diagnostic_info &diagnostic) -{ - if (diagnostic.metadata == NULL) - return; - - int cwe = diagnostic.metadata->get_cwe (); - if (cwe) - { - pretty_printer * const pp = this->printer; - char *saved_prefix = pp_take_prefix (pp); - pp_string (pp, " ["); - pp_string (pp, colorize_start (pp_show_color (pp), - diagnostic_kind_color[diagnostic.kind])); - if (pp->supports_urls_p ()) - { - char *cwe_url = get_cwe_url (cwe); - pp_begin_url (pp, cwe_url); - free (cwe_url); - } - pp_printf (pp, "CWE-%i", cwe); - pp_set_prefix (pp, saved_prefix); - if (pp->supports_urls_p ()) - pp_end_url (pp); - pp_string (pp, colorize_stop (pp_show_color (pp))); - pp_character (pp, ']'); - } -} - -/* If DIAGNOSTIC has any rules associated with it, print them. - - For example, if the diagnostic metadata associates it with a rule - named "STR34-C", then " [STR34-C]" will be printed, suitably colorized, - with any URL provided by the rule. */ - -void -diagnostic_context::print_any_rules (const diagnostic_info &diagnostic) -{ - if (diagnostic.metadata == NULL) - return; - - for (unsigned idx = 0; idx < diagnostic.metadata->get_num_rules (); idx++) - { - const diagnostic_metadata::rule &rule - = diagnostic.metadata->get_rule (idx); - if (char *desc = rule.make_description ()) - { - pretty_printer * const pp = this->printer; - char *saved_prefix = pp_take_prefix (pp); - pp_string (pp, " ["); - pp_string (pp, - colorize_start (pp_show_color (pp), - diagnostic_kind_color[diagnostic.kind])); - char *url = NULL; - if (pp->supports_urls_p ()) - { - url = rule.make_url (); - if (url) - pp_begin_url (pp, url); - } - pp_string (pp, desc); - pp_set_prefix (pp, saved_prefix); - if (pp->supports_urls_p ()) - if (url) - pp_end_url (pp); - free (url); - pp_string (pp, colorize_stop (pp_show_color (pp))); - pp_character (pp, ']'); - free (desc); - } - } -} - -/* Print any metadata about the option used to control DIAGNOSTIC to CONTEXT's - printer, e.g. " [-Werror=uninitialized]". - Subroutine of diagnostic_context::report_diagnostic. */ - -void -diagnostic_context::print_option_information (const diagnostic_info &diagnostic, - diagnostic_t orig_diag_kind) -{ - if (char *option_text = make_option_name (diagnostic.option_index, - orig_diag_kind, diagnostic.kind)) - { - char *option_url = nullptr; - if (this->printer->supports_urls_p ()) - option_url = make_option_url (diagnostic.option_index); - pretty_printer * const pp = this->printer; - pp_string (pp, " ["); - pp_string (pp, colorize_start (pp_show_color (pp), - diagnostic_kind_color[diagnostic.kind])); - if (option_url) - pp_begin_url (pp, option_url); - pp_string (pp, option_text); - if (option_url) - { - pp_end_url (pp); - free (option_url); - } - pp_string (pp, colorize_stop (pp_show_color (pp))); - pp_character (pp, ']'); - free (option_text); - } -} - /* Returns whether a DIAGNOSTIC should be printed, and adjusts diagnostic->kind as appropriate for #pragma GCC diagnostic and -Werror=foo. */ @@ -1517,15 +1406,10 @@ diagnostic_context::report_diagnostic (diagnostic_info *diagnostic) m_diagnostic_groups.m_emission_count++; pp_format (this->printer, &diagnostic->message, m_urlifier); - m_output_format->on_begin_diagnostic (*diagnostic); - pp_output_formatted_text (this->printer, m_urlifier); - if (m_show_cwe) - print_any_cwe (*diagnostic); - if (m_show_rules) - print_any_rules (*diagnostic); - if (m_show_option_requested) - print_option_information (*diagnostic, orig_diag_kind); - m_output_format->on_end_diagnostic (*diagnostic, orig_diag_kind); + /* Call vfunc in the output format. This is responsible for + phase 3 of formatting, and for printing the result. */ + m_output_format->on_report_diagnostic (*diagnostic, orig_diag_kind); + switch (m_extra_output_kind) { default: @@ -1815,16 +1699,27 @@ diagnostic_text_output_format::~diagnostic_text_output_format () } } +/* Implementation of diagnostic_output_format::on_report_diagnostic vfunc + for GCC's standard textual output. */ + void -diagnostic_text_output_format::on_begin_diagnostic (const diagnostic_info &diagnostic) +diagnostic_text_output_format:: +on_report_diagnostic (const diagnostic_info &diagnostic, + diagnostic_t orig_diag_kind) { (*diagnostic_starter (&m_context)) (&m_context, &diagnostic); -} -void -diagnostic_text_output_format::on_end_diagnostic (const diagnostic_info &diagnostic, - diagnostic_t orig_diag_kind) -{ + pp_output_formatted_text (m_context.printer, m_context.get_urlifier ()); + + if (m_context.m_show_cwe) + print_any_cwe (diagnostic); + + if (m_context.m_show_rules) + print_any_rules (diagnostic); + + if (m_context.m_show_option_requested) + print_option_information (diagnostic, orig_diag_kind); + (*diagnostic_finalizer (&m_context)) (&m_context, &diagnostic, orig_diag_kind); } @@ -1843,6 +1738,120 @@ diagnostic_text_output_format::on_diagram (const diagnostic_diagram &diagram) pp_flush (m_context.printer); } +/* If DIAGNOSTIC has a CWE identifier, print it. + + For example, if the diagnostic metadata associates it with CWE-119, + " [CWE-119]" will be printed, suitably colorized, and with a URL of a + description of the security issue. */ + +void +diagnostic_text_output_format::print_any_cwe (const diagnostic_info &diagnostic) +{ + if (diagnostic.metadata == NULL) + return; + + int cwe = diagnostic.metadata->get_cwe (); + if (cwe) + { + pretty_printer * const pp = m_context.printer; + char *saved_prefix = pp_take_prefix (pp); + pp_string (pp, " ["); + pp_string (pp, colorize_start (pp_show_color (pp), + diagnostic_kind_color[diagnostic.kind])); + if (pp->supports_urls_p ()) + { + char *cwe_url = get_cwe_url (cwe); + pp_begin_url (pp, cwe_url); + free (cwe_url); + } + pp_printf (pp, "CWE-%i", cwe); + pp_set_prefix (pp, saved_prefix); + if (pp->supports_urls_p ()) + pp_end_url (pp); + pp_string (pp, colorize_stop (pp_show_color (pp))); + pp_character (pp, ']'); + } +} + +/* If DIAGNOSTIC has any rules associated with it, print them. + + For example, if the diagnostic metadata associates it with a rule + named "STR34-C", then " [STR34-C]" will be printed, suitably colorized, + with any URL provided by the rule. */ + +void +diagnostic_text_output_format:: +print_any_rules (const diagnostic_info &diagnostic) +{ + if (diagnostic.metadata == NULL) + return; + + for (unsigned idx = 0; idx < diagnostic.metadata->get_num_rules (); idx++) + { + const diagnostic_metadata::rule &rule + = diagnostic.metadata->get_rule (idx); + if (char *desc = rule.make_description ()) + { + pretty_printer * const pp = m_context.printer; + char *saved_prefix = pp_take_prefix (pp); + pp_string (pp, " ["); + pp_string (pp, + colorize_start (pp_show_color (pp), + diagnostic_kind_color[diagnostic.kind])); + char *url = NULL; + if (pp->supports_urls_p ()) + { + url = rule.make_url (); + if (url) + pp_begin_url (pp, url); + } + pp_string (pp, desc); + pp_set_prefix (pp, saved_prefix); + if (pp->supports_urls_p ()) + if (url) + pp_end_url (pp); + free (url); + pp_string (pp, colorize_stop (pp_show_color (pp))); + pp_character (pp, ']'); + free (desc); + } + } +} + +/* Print any metadata about the option used to control DIAGNOSTIC to CONTEXT's + printer, e.g. " [-Werror=uninitialized]". + Subroutine of diagnostic_context::report_diagnostic. */ + +void +diagnostic_text_output_format:: +print_option_information (const diagnostic_info &diagnostic, + diagnostic_t orig_diag_kind) +{ + if (char *option_text + = m_context.make_option_name (diagnostic.option_index, + orig_diag_kind, diagnostic.kind)) + { + char *option_url = nullptr; + pretty_printer * const pp = m_context.printer; + if (pp->supports_urls_p ()) + option_url = m_context.make_option_url (diagnostic.option_index); + pp_string (pp, " ["); + pp_string (pp, colorize_start (pp_show_color (pp), + diagnostic_kind_color[diagnostic.kind])); + if (option_url) + pp_begin_url (pp, option_url); + pp_string (pp, option_text); + if (option_url) + { + pp_end_url (pp); + free (option_url); + } + pp_string (pp, colorize_stop (pp_show_color (pp))); + pp_character (pp, ']'); + free (option_text); + } +} + /* Set the output format for CONTEXT to FORMAT, using BASE_FILE_NAME for file-based output formats. */ diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h index 2a9f2751dca2..0a496e4bfab9 100644 --- a/gcc/diagnostic.h +++ b/gcc/diagnostic.h @@ -208,9 +208,12 @@ public: virtual void on_begin_group () = 0; virtual void on_end_group () = 0; - virtual void on_begin_diagnostic (const diagnostic_info &) = 0; - virtual void on_end_diagnostic (const diagnostic_info &, - diagnostic_t orig_diag_kind) = 0; + + /* Vfunc with responsibility for phase 3 of formatting the message + and "printing" the result. */ + virtual void on_report_diagnostic (const diagnostic_info &, + diagnostic_t orig_diag_kind) = 0; + virtual void on_diagram (const diagnostic_diagram &diagram) = 0; virtual bool machine_readable_stderr_p () const = 0; @@ -237,14 +240,19 @@ public: ~diagnostic_text_output_format (); void on_begin_group () override {} void on_end_group () override {} - void on_begin_diagnostic (const diagnostic_info &) override; - void on_end_diagnostic (const diagnostic_info &, - diagnostic_t orig_diag_kind) override; + void on_report_diagnostic (const diagnostic_info &, + diagnostic_t orig_diag_kind) override; void on_diagram (const diagnostic_diagram &diagram) override; bool machine_readable_stderr_p () const final override { return false; } + +private: + void print_any_cwe (const diagnostic_info &diagnostic); + void print_any_rules (const diagnostic_info &diagnostic); + void print_option_information (const diagnostic_info &diagnostic, + diagnostic_t orig_diag_kind); }; /* A stack of sets of classifications: each entry in the stack is @@ -382,6 +390,8 @@ public: friend diagnostic_finalizer_fn & diagnostic_finalizer (diagnostic_context *context); + friend class diagnostic_text_output_format; + typedef void (*ice_handler_callback_t) (diagnostic_context *); typedef void (*set_locations_callback_t) (diagnostic_context *, diagnostic_info *); @@ -522,6 +532,7 @@ public: { return m_client_data_hooks; } + urlifier *get_urlifier () const { return m_urlifier; } text_art::theme *get_diagram_theme () const { return m_diagrams.m_theme; } int converted_column (expanded_location s) const; @@ -586,11 +597,6 @@ public: private: bool includes_seen_p (const line_map_ordinary *map); - void print_any_cwe (const diagnostic_info &diagnostic); - void print_any_rules (const diagnostic_info &diagnostic); - void print_option_information (const diagnostic_info &diagnostic, - diagnostic_t orig_diag_kind); - void show_any_path (const diagnostic_info &diagnostic); void error_recursion () ATTRIBUTE_NORETURN; -- 2.26.3