diff mbox series

[pushed:,r15-3975] diagnostics: avoid using diagnostic_context's m_printer [PR116613]

Message ID 20240930160317.2857155-1-dmalcolm@redhat.com
State New
Headers show
Series [pushed:,r15-3975] diagnostics: avoid using diagnostic_context's m_printer [PR116613] | expand

Commit Message

David Malcolm Sept. 30, 2024, 4:03 p.m. UTC 
As work towards supporting multiple diagnostic outputs (where each
output has its own pretty_printer), avoid using diagnostic_context's
m_printer field.  Instead, use the output format's printer.  Currently
this *is* the dc's printer, but eventually it might not be.

No functional change intended.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r15-3975-gcce52867d1892c.

gcc/ChangeLog:
	PR other/116613
	* diagnostic-format-json.cc (diagnostic_output_format_init_json):
	Pass in the format.  Use the format's printer when disabling
	colorization.  Move the call to set_output_format into here.
	(diagnostic_output_format_init_json_stderr): Update for above
	change.
	(diagnostic_output_format_init_json_file): Likewise.
	* diagnostic-format-sarif.cc
	(diagnostic_output_format_init_sarif): Use the format's printer
	when disabling colorization.
	* diagnostic-path.cc (selftest::test_empty_path): Use the
	text_output's printer.
	(selftest::test_intraprocedural_path): Likewise.
	(selftest::test_interprocedural_path_1): Likewise.
	(selftest::test_interprocedural_path_2): Likewise.
	(selftest::test_recursion): Likewise.
	(selftest::test_control_flow_1): Likewise.
	(selftest::test_control_flow_2): Likewise.
	(selftest::test_control_flow_3): Likewise.
	(selftest::assert_cfg_edge_path_streq): Likewise.
	(selftest::test_control_flow_5): Likewise.
	(selftest::test_control_flow_6): Likewise.

gcc/testsuite/ChangeLog:
	PR other/116613
	* gcc.dg/plugin/diagnostic_group_plugin.c
	(test_output_format::on_begin_group): Use get_printer () rather
	than accessing m_context.m_printer.
	(test_output_format::on_end_group): Likewise.
	* gcc.dg/plugin/diagnostic_plugin_xhtml_format.c
	(xhtml_builder::m_printer): New field.
	(xhtml_builder::xhtml_builder): Add "pp" param and use it to
	initialize m_printer.
	(xhtml_builder::on_report_diagnostic): Drop "context" param.
	(xhtml_builder::make_element_for_diagnostic): Likewise.  Use
	this->m_printer rather than the context's m_printer.  Pass
	m_printer to call to diagnostic_show_locus.
	(xhtml_builder::emit_diagram): Drop "context" param.
	(xhtml_output_format::on_report_diagnostic): Drop context param
	from call to m_builder.
	(xhtml_output_format::on_diagram): Likewise.
	(xhtml_output_format::xhtml_output_format): Pass result of
	get_printer as printer for builder.
	(diagnostic_output_format_init_xhtml): Use the fmt's printer
	rather than the context's.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
---
 gcc/diagnostic-format-json.cc                 | 23 +++++----
 gcc/diagnostic-format-sarif.cc                |  2 +-
 gcc/diagnostic-path.cc                        | 38 +++++++--------
 .../gcc.dg/plugin/diagnostic_group_plugin.c   | 12 +++--
 .../plugin/diagnostic_plugin_xhtml_format.c   | 47 +++++++++----------
 5 files changed, 64 insertions(+), 58 deletions(-)
diff mbox series

Patch

diff --git a/gcc/diagnostic-format-json.cc b/gcc/diagnostic-format-json.cc
index 448b6cb54eee..cf900a9ecba2 100644
--- a/gcc/diagnostic-format-json.cc
+++ b/gcc/diagnostic-format-json.cc
@@ -393,14 +393,17 @@  private:
    to a file).  */
 
 static void
-diagnostic_output_format_init_json (diagnostic_context &context)
+diagnostic_output_format_init_json (diagnostic_context &context,
+				    std::unique_ptr<json_output_format> fmt)
 {
   /* Suppress normal textual path output.  */
   context.set_path_format (DPF_NONE);
 
   /* Don't colorize the text.  */
-  pp_show_color (context.m_printer) = false;
+  pp_show_color (fmt->get_printer ()) = false;
   context.set_show_highlight_colors (false);
+
+  context.set_output_format (fmt.release ());
 }
 
 /* Populate CONTEXT in preparation for JSON output to stderr.  */
@@ -409,9 +412,10 @@  void
 diagnostic_output_format_init_json_stderr (diagnostic_context &context,
 					   bool formatted)
 {
-  diagnostic_output_format_init_json (context);
-  context.set_output_format (new json_stderr_output_format (context,
-							    formatted));
+  diagnostic_output_format_init_json
+    (context,
+     ::make_unique<json_stderr_output_format> (context,
+					       formatted));
 }
 
 /* Populate CONTEXT in preparation for JSON output to a file named
@@ -422,10 +426,11 @@  diagnostic_output_format_init_json_file (diagnostic_context &context,
 					 bool formatted,
 					 const char *base_file_name)
 {
-  diagnostic_output_format_init_json (context);
-  context.set_output_format (new json_file_output_format (context,
-							  formatted,
-							  base_file_name));
+  diagnostic_output_format_init_json
+    (context,
+     ::make_unique<json_file_output_format> (context,
+					     formatted,
+					     base_file_name));
 }
 
 #if CHECKING_P
diff --git a/gcc/diagnostic-format-sarif.cc b/gcc/diagnostic-format-sarif.cc
index 7b11dfd89a31..11b224c4c90d 100644
--- a/gcc/diagnostic-format-sarif.cc
+++ b/gcc/diagnostic-format-sarif.cc
@@ -3349,7 +3349,7 @@  diagnostic_output_format_init_sarif (diagnostic_context &context,
   context.set_ice_handler_callback (sarif_ice_handler);
 
   /* Don't colorize the text.  */
-  pp_show_color (context.m_printer) = false;
+  pp_show_color (fmt->get_printer ()) = false;
   context.set_show_highlight_colors (false);
 
   context.m_printer->set_token_printer
diff --git a/gcc/diagnostic-path.cc b/gcc/diagnostic-path.cc
index feee054ea4df..bef56b1d9d12 100644
--- a/gcc/diagnostic-path.cc
+++ b/gcc/diagnostic-path.cc
@@ -1180,7 +1180,7 @@  test_empty_path (pretty_printer *event_pp)
 
   print_path_summary_as_text (summary, text_output, true);
   ASSERT_STREQ ("",
-		pp_formatted_text (dc.m_printer));
+		pp_formatted_text (text_output.get_printer ()));
 }
 
 /* Verify that print_path_summary works on a purely intraprocedural path.  */
@@ -1204,7 +1204,7 @@  test_intraprocedural_path (pretty_printer *event_pp)
   ASSERT_STREQ ("  `foo': events 1-2 (depth 0)\n"
 		" (1): first `free'\n"
 		" (2): double `free'\n",
-		pp_formatted_text (dc.m_printer));
+		pp_formatted_text (text_output.get_printer ()));
 }
 
 /* Verify that print_path_summary works on an interprocedural path.  */
@@ -1288,7 +1288,7 @@  test_interprocedural_path_1 (pretty_printer *event_pp)
        "                  | (17): entering `wrapped_free'\n"
        "                  | (18): calling free\n"
        "                  |\n",
-       pp_formatted_text (dc.m_printer));
+       pp_formatted_text (text_output.get_printer ()));
   }
   {
     test_diagnostic_context dc;
@@ -1346,7 +1346,7 @@  test_interprocedural_path_1 (pretty_printer *event_pp)
        "                  │ (17): entering `wrapped_free'\n"
        "                  │ (18): calling free\n"
        "                  │\n",
-       pp_formatted_text (dc.m_printer));
+       pp_formatted_text (text_output.get_printer ()));
   }
 
 }
@@ -1400,7 +1400,7 @@  test_interprocedural_path_2 (pretty_printer *event_pp)
        "                  |\n"
        "                  | (8): entering `baz'\n"
        "                  |\n",
-       pp_formatted_text (dc.m_printer));
+       pp_formatted_text (text_output.get_printer ()));
   }
   {
     test_diagnostic_context dc;
@@ -1434,7 +1434,7 @@  test_interprocedural_path_2 (pretty_printer *event_pp)
        "                  │\n"
        "                  │ (8): entering `baz'\n"
        "                  │\n",
-       pp_formatted_text (dc.m_printer));
+       pp_formatted_text (text_output.get_printer ()));
   }
 }
 
@@ -1481,7 +1481,7 @@  test_recursion (pretty_printer *event_pp)
        "                         |\n"
        "                         | (7): entering `factorial'\n"
        "                         |\n",
-       pp_formatted_text (dc.m_printer));
+       pp_formatted_text (text_output.get_printer ()));
   }
   {
     test_diagnostic_context dc;
@@ -1510,7 +1510,7 @@  test_recursion (pretty_printer *event_pp)
        "                         │\n"
        "                         │ (7): entering `factorial'\n"
        "                         │\n",
-       pp_formatted_text (dc.m_printer));
+       pp_formatted_text (text_output.get_printer ()));
   }
 }
 
@@ -1627,7 +1627,7 @@  test_control_flow_1 (const line_table_case &case_,
        "|         |\n"
        "+-------->(2) ...to here\n"
        "          (3) dereference of NULL `p'\n",
-       pp_formatted_text (dc.m_printer));
+       pp_formatted_text (text_output.get_printer ()));
   }
   {
     test_diagnostic_context dc;
@@ -1649,7 +1649,7 @@  test_control_flow_1 (const line_table_case &case_,
        "          |\n"
        "          (2) ...to here\n"
        "          (3) dereference of NULL `p'\n",
-       pp_formatted_text (dc.m_printer));
+       pp_formatted_text (text_output.get_printer ()));
   }
   {
     test_diagnostic_context dc;
@@ -1675,7 +1675,7 @@  test_control_flow_1 (const line_table_case &case_,
        "      ||         |\n"
        "      |+-------->(2) ...to here\n"
        "      |          (3) dereference of NULL `p'\n",
-       pp_formatted_text (dc.m_printer));
+       pp_formatted_text (text_output.get_printer ()));
   }
   {
     test_diagnostic_context dc;
@@ -1698,7 +1698,7 @@  test_control_flow_1 (const line_table_case &case_,
        "      |          |\n"
        "      |          (2) ...to here\n"
        "      |          (3) dereference of NULL `p'\n",
-       pp_formatted_text (dc.m_printer));
+       pp_formatted_text (text_output.get_printer ()));
   }
   {
     test_diagnostic_context dc;
@@ -1723,7 +1723,7 @@  test_control_flow_1 (const line_table_case &case_,
        "│         |\n"
        "└────────>(2) ...to here\n"
        "          (3) dereference of NULL `p'\n",
-       pp_formatted_text (dc.m_printer));
+       pp_formatted_text (text_output.get_printer ()));
   }
   {
     test_diagnostic_context dc;
@@ -1749,7 +1749,7 @@  test_control_flow_1 (const line_table_case &case_,
        "      |│         |\n"
        "      |└────────>(2) ...to here\n"
        "      |          (3) dereference of NULL `p'\n",
-       pp_formatted_text (dc.m_printer));
+       pp_formatted_text (text_output.get_printer ()));
   }
 }
 
@@ -1836,7 +1836,7 @@  test_control_flow_2 (const line_table_case &case_,
        "      ||                              ^~~~\n"
        "      ||                              |\n"
        "      |+----------------------------->(5) ...to here\n",
-       pp_formatted_text (dc.m_printer));
+       pp_formatted_text (text_output.get_printer ()));
   }
 }
 
@@ -1917,7 +1917,7 @@  test_control_flow_3 (const line_table_case &case_,
        "      ||                  ^\n"
        "      ||                  |\n"
        "      |+----------------->(5) ...to here\n",
-       pp_formatted_text (dc.m_printer));
+       pp_formatted_text (text_output.get_printer ()));
   }
 }
 
@@ -1947,7 +1947,7 @@  assert_cfg_edge_path_streq (const location &loc,
   path_summary summary (text_output, path, true);
   print_path_summary_as_text (summary, text_output, false);
   ASSERT_STREQ_AT (loc, expected_str,
-		   pp_formatted_text (dc.m_printer));
+		   pp_formatted_text (text_output.get_printer ()));
 }
 
 /* Assert that if we make a path with an event with "from here..." at SRC_LOC
@@ -2292,7 +2292,7 @@  test_control_flow_5 (const line_table_case &case_,
        "      ||            ~                   ~~~~~~~~~~~~~~~~~~~~~~~~~~\n"
        "      ||            |                   |\n"
        "      |+----------->(4) ...to here      (5) allocated here\n",
-       pp_formatted_text (dc.m_printer));
+       pp_formatted_text (text_output.get_printer ()));
   }
 }
 
@@ -2388,7 +2388,7 @@  test_control_flow_6 (const line_table_case &case_,
        "      ||    ~~~~~~~~~~~~~\n"
        "      ||              |\n"
        "      |+------------->(5) ...to here\n",
-       pp_formatted_text (dc.m_printer));
+       pp_formatted_text (text_output.get_printer ()));
   }
 }
 
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic_group_plugin.c b/gcc/testsuite/gcc.dg/plugin/diagnostic_group_plugin.c
index b4e876bc3a0c..b6def2317dfc 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic_group_plugin.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic_group_plugin.c
@@ -194,17 +194,19 @@  class test_output_format : public diagnostic_text_output_format
   void on_begin_group () final override
   {
     /* Loudly announce a new diagnostic group.  */
-    pp_string (m_context.m_printer,
+    pretty_printer *const pp = get_printer ();
+    pp_string (pp,
 	       "================================= BEGIN GROUP ==============================");
-    pp_newline (m_context.m_printer);
+    pp_newline (pp);
   }
   void on_end_group () final override
   {
     /* Loudly announce the end of a diagnostic group.  */
-    pp_set_prefix (m_context.m_printer, NULL);
-    pp_string (m_context.m_printer,
+    pretty_printer *const pp = get_printer ();
+    pp_set_prefix (pp, NULL);
+    pp_string (pp,
 	       "---------------------------------- END GROUP -------------------------------");
-    pp_newline_and_flush (m_context.m_printer);
+    pp_newline_and_flush (pp);
   }
 };
 
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_xhtml_format.c b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_xhtml_format.c
index 0f13e8d6d01a..2ce8cc29660c 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_xhtml_format.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_xhtml_format.c
@@ -265,13 +265,12 @@  class xhtml_builder
 {
 public:
   xhtml_builder (diagnostic_context &context,
+		 pretty_printer &pp,
 		 const line_maps *line_maps);
 
-  void on_report_diagnostic (diagnostic_context &context,
-			     const diagnostic_info &diagnostic,
+  void on_report_diagnostic (const diagnostic_info &diagnostic,
 			     diagnostic_t orig_diag_kind);
-  void emit_diagram (diagnostic_context &context,
-		     const diagnostic_diagram &diagram);
+  void emit_diagram (const diagnostic_diagram &diagram);
   void end_group ();
 
   std::unique_ptr<xml::element> take_current_diagnostic ()
@@ -285,11 +284,11 @@  public:
 
 private:
   std::unique_ptr<xml::element>
-  make_element_for_diagnostic (diagnostic_context &context,
-			       const diagnostic_info &diagnostic,
+  make_element_for_diagnostic (const diagnostic_info &diagnostic,
 			       diagnostic_t orig_diag_kind);
 
   diagnostic_context &m_context;
+  pretty_printer &m_printer;
   const line_maps *m_line_maps;
 
   std::unique_ptr<xml::document> m_document;
@@ -318,8 +317,10 @@  make_span (label_text class_)
 /* xhtml_builder's ctor.  */
 
 xhtml_builder::xhtml_builder (diagnostic_context &context,
+			      pretty_printer &pp,
 			      const line_maps *line_maps)
 : m_context (context),
+  m_printer (pp),
   m_line_maps (line_maps)
 {
   gcc_assert (m_line_maps);
@@ -358,14 +359,13 @@  xhtml_builder::xhtml_builder (diagnostic_context &context,
 /* Implementation of "on_report_diagnostic" for XHTML output.  */
 
 void
-xhtml_builder::on_report_diagnostic (diagnostic_context &context,
-				     const diagnostic_info &diagnostic,
+xhtml_builder::on_report_diagnostic (const diagnostic_info &diagnostic,
 				     diagnostic_t orig_diag_kind)
 {
   // TODO: handle (diagnostic.kind == DK_ICE || diagnostic.kind == DK_ICE_NOBT)
 
   auto diag_element
-    = make_element_for_diagnostic (context, diagnostic, orig_diag_kind);
+    = make_element_for_diagnostic (diagnostic, orig_diag_kind);
   if (m_cur_diagnostic_element)
     /* Nested diagnostic.  */
     m_cur_diagnostic_element->add_child (std::move (diag_element));
@@ -375,8 +375,7 @@  xhtml_builder::on_report_diagnostic (diagnostic_context &context,
 }
 
 std::unique_ptr<xml::element>
-xhtml_builder::make_element_for_diagnostic (diagnostic_context &context,
-					    const diagnostic_info &diagnostic,
+xhtml_builder::make_element_for_diagnostic (const diagnostic_info &diagnostic,
 					    diagnostic_t orig_diag_kind)
 {
   class xhtml_token_printer : public token_printer
@@ -469,10 +468,10 @@  xhtml_builder::make_element_for_diagnostic (diagnostic_context &context,
 
   auto message_span = make_span (label_text::borrow ("gcc-message"));
   xhtml_token_printer tok_printer (*this, *message_span.get ());
-  context.m_printer->set_token_printer (&tok_printer);
-  pp_output_formatted_text (context.m_printer, context.get_urlifier ());
-  context.m_printer->set_token_printer (nullptr);
-  pp_clear_output_area (context.m_printer);
+  m_printer.set_token_printer (&tok_printer);
+  pp_output_formatted_text (&m_printer, m_context.get_urlifier ());
+  m_printer.set_token_printer (nullptr);
+  pp_clear_output_area (&m_printer);
   diag_element->add_child (std::move (message_span));
 
   if (diagnostic.metadata)
@@ -529,10 +528,11 @@  xhtml_builder::make_element_for_diagnostic (diagnostic_context &context,
     auto pre = ::make_unique<xml::element> ("pre", true);
     pre->set_attr ("class", label_text::borrow ("gcc-annotated-source"));
     // TODO: ideally we'd like to capture elements within the following:
-    diagnostic_show_locus (&context, diagnostic.richloc, diagnostic.kind);
+    diagnostic_show_locus (&m_context, diagnostic.richloc, diagnostic.kind,
+			   &m_printer);
     pre->add_text
-      (label_text::take (xstrdup (pp_formatted_text (context.m_printer))));
-    pp_clear_output_area (context.m_printer);
+      (label_text::take (xstrdup (pp_formatted_text (&m_printer))));
+    pp_clear_output_area (&m_printer);
     diag_element->add_child (std::move (pre));
   }
 
@@ -543,8 +543,7 @@  xhtml_builder::make_element_for_diagnostic (diagnostic_context &context,
    for XHTML output.  */
 
 void
-xhtml_builder::emit_diagram (diagnostic_context &/*context*/,
-			     const diagnostic_diagram &/*diagram*/)
+xhtml_builder::emit_diagram (const diagnostic_diagram &/*diagram*/)
 {
   /* We must be within the emission of a top-level diagnostic.  */
   gcc_assert (m_cur_diagnostic_element);
@@ -616,11 +615,11 @@  public:
   on_report_diagnostic (const diagnostic_info &diagnostic,
 			    diagnostic_t orig_diag_kind) final override
   {
-    m_builder.on_report_diagnostic (m_context, diagnostic, orig_diag_kind);
+    m_builder.on_report_diagnostic (diagnostic, orig_diag_kind);
   }
   void on_diagram (const diagnostic_diagram &diagram) final override
   {
-    m_builder.emit_diagram (m_context, diagram);
+    m_builder.emit_diagram (diagram);
   }
   void after_diagnostic (const diagnostic_info &)
   {
@@ -636,7 +635,7 @@  protected:
   xhtml_output_format (diagnostic_context &context,
 		       const line_maps *line_maps)
   : diagnostic_output_format (context),
-    m_builder (context, line_maps)
+    m_builder (context, *get_printer (), line_maps)
   {}
 
   xhtml_builder m_builder;
@@ -712,7 +711,7 @@  diagnostic_output_format_init_xhtml (diagnostic_context &context,
   context.set_ice_handler_callback (xhtml_ice_handler);
 
   /* Don't colorize the text.  */
-  pp_show_color (context.m_printer) = false;
+  pp_show_color (fmt->get_printer ()) = false;
   context.set_show_highlight_colors (false);
 
   context.set_output_format (fmt.release ());