On 11/10/20 9:08 AM, David Malcolm via Gcc-patches wrote:
> Here's an updated version of the HTML output idea from:
>   https://gcc.gnu.org/pipermail/gcc-patches/2020-October/556848.html
>
> I reworked the HTML path output to show stack frames as well as just
> runs of events, using drop-shadows to give a 3D look.  The idea is to try
> to highlight the stack of frames as if it were an actual stack of
> overlapping cards.
>
> Updated HTML for the example from the earlier patch can be seen here:
>   
> https://dmalcolm.fedorapeople.org/gcc/2020-11-05/html-examples/test.c.path-1.html
> As before, other examples can be seen in that directory, such as:
>   Signal handler issue:
>     
> https://dmalcolm.fedorapeople.org/gcc/2020-11-05/html-examples/signal-1.c.path-1.html
>   Leak due to longjmp past a "free":
>     
> https://dmalcolm.fedorapeople.org/gcc/2020-11-05/html-examples/setjmp-7.c.path-1.html
>
> Other changes in v2:
> * switched j and k in keyboard navigation so that j is "next event"
> and k is "previous event"
> * fixed event element IDs, fixing a bug where it assumed they were in
>   ascending order
> * moved HTML printing code out of path_summary and event_range
> * more selftest coverage
>
> As before, this approach merely emits the path information; it doesn't
> capture the associated diagnostic.  I'm working on an alternate approach
> for v3 of the patch that does that; doing that requires reworking
> pretty_printer.
>
> I'm not sure on exactly the correct approach here; for v3 I'm
> experimenting with an "html" option for -fdiagnostics-format= which
> when selected would supplement the existing text output, by additionally
> writing out an HTML file for each diagnostic group, with path
> information captured there.  Doing it per group means that the classic
> "warning"/"note" pairs would be grouped together in that output.
>
> I'm not sure what that ought to do with paths though; part of the point
> of doing it is to have less verbose output on stderr whilst capturing
> the pertinent information "on the side" in the HTML file.
>
> Thoughts?
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu, FWIW.
>
> gcc/ChangeLog:
>       * common.opt (diagnostic_path_format): Add DPF_HTML.
>       * diagnostic-format-json.cc: Include "selftest.h".
>       * diagnostic-show-locus.c (colorizer::m_context): Replace with...
>       (colorizer::m_pp): ...this new field.
>       (layout::m_context): Make const.
>       (layout::m_is_html): New field.
>       (layout::m_html_label_writer): New field.
>       (colorizer::colorizer): Update for use of pp rather than context.
>       (colorizer::begin_state): Likewise.
>       (colorizer::finish_state): Likewise.
>       (colorizer::get_color_by_name): Likewise.
>       (layout::layout): Make "context" param const.  Add "pp", "is_html"
>       and "label_writer" params, with defaults.  Use them to initialize
>       new fields.  Update for change to m_colorizer.
>       (layout::print_source_line): Add HTML support.
>       (layout::start_annotation_line): Likewise.
>       (layout::print_annotation_line): Likewise.
>       (line_label::line_label): Make context const.  Add
>       "original_range_idx" param and use it to initialize...
>       (line_label::m_original_range_idx): ...this new field.
>       (layout::print_any_labels): Pass original index of range to
>       line_label ctor.  Add HTML support.
>       (get_affected_range): Make context const.
>       (get_printed_columns): Likewise.
>       (line_corrections::line_corrections): Likewise.
>       (line_corrections::m_context): Likewise.
>       (layout::print_line): Don't print fix-it hints for HTML support.
>       (diagnostic_show_locus_as_html): New.
>       (selftest::assert_html_eq): New.
>       (ASSERT_HTML_EQ): New.
>       (selftest::test_one_liner_simple_caret): Verify the HTML output.
>       (selftest::test_diagnostic_show_locus_fixit_lines): Likewise.
>       (selftest::test_html): New.
>       (selftest::diagnostic_show_locus_c_tests): Call it.
>       * diagnostic.c (diagnostic_show_any_path): Pass the location to
>       the print_path callback.
>       * diagnostic.h (enum diagnostic_path_format): Add DPF_HTML.
>       (diagnostic_context::num_html_paths): New field.
>       (diagnostic_context::print_path): Add location_t param.
>       (class html_label_writer): New.
>       (diagnostic_show_locus_as_html): New decl.
>       * doc/invoke.texi (Diagnostic Message Formatting Options): Add
>       "html" to -fdiagnostics-path-format=.
>       (-fdiagnostics-path-format=): Add html.
>       * pretty-print.c (pp_write_text_as_html_to_stream): New.
>       * pretty-print.h (pp_write_text_as_html_to_stream): New decl.
>       * selftest-diagnostic.c (selftest::html_printer::html_printer):
>       New ctor.
>       (selftest::html_printer::get_html_output): New.
>       * selftest-diagnostic.h (class selftest::html_printer): New.
>       * tree-diagnostic-path.cc: Define GCC_DIAG_STYLE where possible
>       and necessary.  Include "options.h" for dump_base_name.
>       (path_label::path_label): Add "colorize" param and use it to
>       initialize m_colorize.
>       (path_label::get_text): Use m_colorizer rather than accessing
>       global_dc's printer.
>       (path_label::m_colorize): New field.
>       (event_range::event_range): Add "colorize" param and use it to
>       initialize m_path_label.
>       (event_range::get_filename): New.
>       (path_summary::path_summary): Add "colorize" param and use it when
>       creating event_ranges.
>       (class html_path_label_writer): New.
>       (struct stack_frame): New.
>       (begin_html_stack_frame): New.
>       (end_html_stack_frame): New.
>       (emit_svg_arrow): New.
>       (print_event_range_as_html): New.
>       (print_path_summary_as_html): New.
>       (HTML_STYLE): New.
>       (HTML_SCRIPT): New.
>       (write_html_for_path): New.
>       (default_tree_diagnostic_path_printer): Add "loc" param.
>       Update DPF_INLINE_EVENTS for new "colorize" param of
>       path_summary's ctor.  Add DPF_HTML.
>       (selftest::test_empty_path): Update for new param of path_summary
>       ctor.
>       (selftest::test_intraprocedural_path): Likewise.
>       (selftest::test_interprocedural_path_1): Likewise.
>       (selftest::test_interprocedural_path_2): Likewise.  Add test of
>       HTML output.
>       (selftest::test_recursion): Update for new param of path_summary
>       ctor.
>       * tree-diagnostic.h (default_tree_diagnostic_path_printer): Add
>       location_t param.
>
> gcc/testsuite/ChangeLog:
>       * gcc.dg/plugin/diagnostic-path-format-html.c: New test.
>       * gcc.dg/plugin/plugin.exp (plugin_test_list): Add it.
As I mentioned in our call today, I think this one should be your
choice.  I can certainly see the value in using html for paths instead
of ascii-art.   Though we probably need to make sure the final
diagnostic gets into the html file so someone looking at the HTML knows
what ultimately triggered the diagnostic.


This also starts us down the path of potentially having diagnostics as
artifacts that the CI or build systems could save/scan.  For various
reasons I think this would be helpful ;-)

jeff

Reply via email to