[PATCH 12/49] Add diagnostic paths

[David Malcolm]

On Sat, 2019-12-07 at 07:45 -0700, Jeff Law wrote:
> On Fri, 2019-11-15 at 20:22 -0500, David Malcolm wrote:
> > This patch adds support for associating a "diagnostic_path" with a
> > diagnostic: a sequence of events predicted by the compiler that
> > leads
> > to
> > the problem occurring, with their locations in the user's source,
> > text descriptions, and stack information (for handling
> > interprocedural
> > paths).
> > 
> > For example, the following (hypothetical) error has a 3-event
> > intraprocedural path:
> > 
> > test.c: In function 'demo':
> > test.c:29:5: error: passing NULL as argument 1 to 'PyList_Append'
> > which
> >   requires a non-NULL parameter
> >    29 |     PyList_Append(list, item);
> >       |     ^~~~~~~~~~~~~~~~~~~~~~~~~
> >   'demo': events 1-3
> >      |
> >      |   25 |   list = PyList_New(0);
> >      |      |          ^~~~~~~~~~~~~
> >      |      |          |
> >      |      |          (1) when 'PyList_New' fails, returning NULL
> >      |   26 |
> >      |   27 |   for (i = 0; i < count; i++) {
> >      |      |   ~~~
> >      |      |   |
> >      |      |   (2) when 'i < count'
> >      |   28 |     item = PyLong_FromLong(random());
> >      |   29 |     PyList_Append(list, item);
> >      |      |     ~~~~~~~~~~~~~~~~~~~~~~~~~
> >      |      |     |
> >      |      |     (3) when calling 'PyList_Append', passing NULL
> > from
> > (1) as argument 1
> >      |
> > 
> > The patch adds a new "%@" format code for printing event IDs, so
> > that
> > in the above, the description of event (3) mentions event (1),
> > showing
> > the user where the bogus NULL value comes from (the event IDs are
> > colorized to draw the user's attention to them).
> > 
> > There is a separation between data vs presentation: the above shows
> > how
> > the diagnostic-printing code has consolidated the path into a
> > single
> > run
> > of events, since all the events are near each other and within the
> > same
> > function; more complicated examples (such as interprocedural paths)
> > might be printed as multiple runs of events.
> > 
> > Examples of how interprocedural paths are printed can be seen in
> > the
> > test suite (which uses a plugin to exercise the code without
> > relying
> > on specific warnings using this functionality).
> > 
> > Other output formats include
> > - JSON,
> > - printing each event as a separate "note", and
> > - to not emit paths.
> > 
> > (I have a separate script that can generate HTML from the JSON, but
> > HTML
> > is not my speciality; help from a web front-end expert to make it
> > look
> > good would be appreciated).
> > 
> > gcc/ChangeLog:
> > 	* Makefile.in (OBJS): Add tree-diagnostic-path.o.
> > 	* common.opt (fdiagnostics-path-format=): New option.
> > 	(diagnostic_path_format): New enum.
> > 	(fdiagnostics-show-path-depths): New option.
> > 	* coretypes.h (diagnostic_event_id_t): New forward decl.
> > 	* diagnostic-color.c (color_dict): Add "path".
> > 	* diagnostic-event-id.h: New file.
> > 	* diagnostic-format-json.cc (json_from_expanded_location): Make
> > 	non-static.
> > 	(json_end_diagnostic): Call context->make_json_for_path if it
> > 	exists and the diagnostic has a path.
> > 	(diagnostic_output_format_init): Clear context->print_path.
> > 	* diagnostic-path.h: New file.
> > 	* diagnostic-show-locus.c (colorizer::set_range): Special-case
> > 	when printing a run of events in a diagnostic_path so that they
> > 	all get the same color.
> > 	(layout::m_diagnostic_path_p): New field.
> > 	(layout::layout): Initialize it.
> > 	(layout::print_any_labels): Don't colorize the label text for
> > an
> > 	event in a diagnostic_path.
> > 	(gcc_rich_location::add_location_if_nearby): Add
> > 	"restrict_to_current_line_spans" and "label" params.  Pass the
> > 	former to layout.maybe_add_location_range; pass the latter
> > 	when calling add_range.
> > 	* diagnostic.c: Include "diagnostic-path.h".
> > 	(diagnostic_initialize): Initialize context->path_format and
> > 	context->show_path_depths.
> > 	(diagnostic_show_any_path): New function.
> > 	(diagnostic_path::interprocedural_p): New function.
> > 	(diagnostic_report_diagnostic): Call diagnostic_show_any_path.
> > 	(simple_diagnostic_path::num_events): New function.
> > 	(simple_diagnostic_path::get_event): New function.
> > 	(simple_diagnostic_path::add_event): New function.
> > 	(simple_diagnostic_event::simple_diagnostic_event): New ctor.
> > 	(simple_diagnostic_event::~simple_diagnostic_event): New dtor.
> > 	(debug): New overload taking a diagnostic_path *.
> > 	* diagnostic.def (DK_DIAGNOSTIC_PATH): New.
> > 	* diagnostic.h (enum diagnostic_path_format): New enum.
> > 	(json::value): New forward decl.
> > 	(diagnostic_context::path_format): New field.
> > 	(diagnostic_context::show_path_depths): New field.
> > 	(diagnostic_context::print_path): New callback field.
> > 	(diagnostic_context::make_json_for_path): New callback field.
> > 	(diagnostic_show_any_path): New decl.
> > 	(json_from_expanded_location): New decl.
> > 	* doc/invoke.texi (-fdiagnostics-path-format=): New option.
> > 	(-fdiagnostics-show-path-depths): New option.
> > 	(-fdiagnostics-color): Add "path" to description of default
> > 	GCC_COLORS; describe it.
> > 	(-fdiagnostics-format=json): Document how diagnostic paths are
> > 	represented in the JSON output format.
> > 	* gcc-rich-location.h
> > (gcc_rich_location::add_location_if_nearby):
> > 	Add optional params "restrict_to_current_line_spans" and
> > "label".
> > 	* opts.c (common_handle_option): Handle
> > 	OPT_fdiagnostics_path_format_ and
> > 	OPT_fdiagnostics_show_path_depths.
> > 	* pretty-print.c: Include "diagnostic-event-id.h".
> > 	(pp_format): Implement "%@" format code for printing
> > 	diagnostic_event_id_t *.
> > 	(selftest::test_pp_format): Add tests for "%@".
> > 	* selftest-run-tests.c (selftest::run_tests): Call
> > 	selftest::tree_diagnostic_path_cc_tests.
> > 	* selftest.h (selftest::tree_diagnostic_path_cc_tests): New
> > decl.
> > 	* toplev.c (general_init): Initialize global_dc->path_format
> > and
> > 	global_dc->show_path_depths.
> > 	* tree-diagnostic-path.cc: New file.
> > 	* tree-diagnostic.c (maybe_unwind_expanded_macro_loc): Make
> > 	non-static.  Drop "diagnostic" param in favor of storing the
> > 	original value of "where" and re-using it.
> > 	(virt_loc_aware_diagnostic_finalizer): Update for dropped param
> > of
> > 	maybe_unwind_expanded_macro_loc.
> > 	(tree_diagnostics_defaults): Initialize context->print_path and
> > 	context->make_json_for_path.
> > 	* tree-diagnostic.h (default_tree_diagnostic_path_printer): New
> > 	decl.
> > 	(default_tree_make_json_for_path): New decl.
> > 	(maybe_unwind_expanded_macro_loc): New decl.
> > 
> > gcc/c-family/ChangeLog:
> > 	* c-format.c (local_event_ptr_node): New.
> > 	(PP_FORMAT_CHAR_TABLE): Add entry for "%@".
> > 	(init_dynamic_diag_info): Initialize local_event_ptr_node.
> > 	* c-format.h (T_EVENT_PTR): New define.
> > 
> > gcc/testsuite/ChangeLog:
> > 	* gcc.dg/format/gcc_diag-10.c (diagnostic_event_id_t): New
> > 	typedef.
> > 	(test_diag): Add coverage of "%@".
> > 	* gcc.dg/plugin/diagnostic-path-format-default.c: New test.
> > 	* gcc.dg/plugin/diagnostic-path-format-inline-events-1.c: New
> > test.
> > 	* gcc.dg/plugin/diagnostic-path-format-inline-events-2.c: New
> > test.
> > 	* gcc.dg/plugin/diagnostic-path-format-inline-events-3.c: New
> > test.
> > 	* gcc.dg/plugin/diagnostic-path-format-none.c: New test.
> > 	* gcc.dg/plugin/diagnostic-test-paths-1.c: New test.
> > 	* gcc.dg/plugin/diagnostic-test-paths-2.c: New test.
> > 	* gcc.dg/plugin/diagnostic-test-paths-3.c: New test.
> > 	* gcc.dg/plugin/diagnostic_plugin_test_paths.c: New.
> > 	* gcc.dg/plugin/plugin.exp: Add the new plugin and test cases.
> > 
> > libcpp/ChangeLog:
> > 	* include/line-map.h (class diagnostic_path): New forward decl.
> > 	(rich_location::get_path): New accessor.
> > 	(rich_location::set_path): New function.
> > 	(rich_location::m_path): New field.
> > 	* line-map.c (rich_location::rich_location): Initialize m_path.
> Seems like we could make an argument to include this in an expanded
> "diagnostic framework" maintainership which would allow self-
> approval.

I think I can self-approve this (and there's an argument that it could
be useful to plugin authors even without the analyzer itself).

That said, it has a (slight) dependency on the auto_delete_vec patch,
which I can't self-approve; see:
  https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01035.html

Dave