On Fri, 2020-01-10 at 08:38 -0700, Jeff Law wrote: > On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote: > > I believe I can self-approve this with my "diagnostic messages" > > maintainer hat on. It has dependencies on the auto_delete_vec > > and the -fdiagnostics-nn-line-numbers patches. > > > > Changed in v5: > > - updated copyright years to include 2020 > > > > Changed in v4: > > - Add support for paths for signal-handlers: > > https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00215.html > > - Fix comment > > > > Changed in v3: > > - Fixup for rebase (r278634): c-format.c: get_pointer_to_named_type > > -> get_named_type > > https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00530.html > > > > Changed in v2: > > - Fixup for rebase (r277284) for json::number -> > > json::integer_number > > https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02035.html > > > > 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-test-paths-4.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. > Given this is primarily in the diagnostic machinery, I'm leaving it > to > your discretion when and precisely what to commit. > > jeff
Thanks. I've gone ahead and committed this to trunk (r280142) after testing it separately from the rest of the kit, given that it's theoretically of use to plugin authors even without the analyzer. Dave
