https://gcc.gnu.org/g:ac707d30ce449f30c6018829d443956fdd653f4c
commit r15-3200-gac707d30ce449f30c6018829d443956fdd653f4c
Author: David Malcolm <dmalc...@redhat.com>
Date: Mon Aug 26 12:24:22 2024 -0400
diagnostics: consolidate on_{begin,end}_diagnostic into on_report_diagnostic
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.
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>
Diff:
---
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;
