https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105916
--- Comment #3 from GCC Commits <cvs-commit at gcc dot gnu.org> --- The master branch has been updated by David Malcolm <dmalc...@gcc.gnu.org>: https://gcc.gnu.org/g:f565063a0602ad3b48ac687f575dea7a90cd4105 commit r15-4575-gf565063a0602ad3b48ac687f575dea7a90cd4105 Author: David Malcolm <dmalc...@redhat.com> Date: Wed Oct 23 10:54:42 2024 -0400 diagnostics: implement buffering for non-textual formats [PR105916] PR fortran/105916 reports stray diagnostics appearing in JSON and SARIF output from gfortran. In order to handle various awkard parsing issues, the Fortran frontend implements buffering of diagnostics, so that diagnostics reported to global_dc can be either: (a) immediately issued, or (b) speculatively reported to global_dc, and stored in a buffer, to either be issued later or discarded. This buffering code in gcc/fortran/error.cc directly manipulates implementation details of the diagnostic_context such as the pretty_printer's buffer, and the counts of how many diagnostics have been issued. The issue is that this manipulation of pretty_printer's buffer doesn't work for formats such as JSON and SARIF where diagnostics are handled in a different way (such as by accumulating json::object instances in an array). This patch moves responsibility for such buffering of diagnostics from fortran's error.cc to the diagnostic subsystem. It introduces a new class diagnostic_buffer representing a particular buffer of diagnostics that have been reported but not yet issued. Each diagnostic output format implements buffering in a different way, and so there is a new class hierarchy, diagnostic_per_format_buffer, representing the various format-specific ways that buffering is to be implemented. This is hidden as an implementation detail of diagnostic_buffer. The patch also updates how diagnostics of each kind (e.g. warnings vs errors) are counted, so that if buffering is enabled, the count is incremented within the buffer, and the counts in the diagnostic_context are only updated if and when the buffer is flushed; checking for max_errors is similarly updated to support both buffered and unbuffered cases. For ease of debugging, the patch extends the "dump" functions within the diagnostics subsystem, so that e.g. global_dc->dump () now prints the buffering status, e.g.: (gdb) call global_dc->dump() diagnostic_context: counts: (none) output format: sarif_output_format printer: m_show_color: false m_url_format: bel m_buffer: m_formatted_obstack current object: length 0: m_chunk_obstack current object: length 0: diagnostic buffer: m_per_format_buffer: counts: error: 1 diagnostic_sarif_format_buffer: result[0]: {"ruleId": "error", "level": "error", "message": {"text": "Function âprogramâ requires an argument list at (1)"}, "locations": [{"physicalLocation": {"artifactLocation": {"uri": "../../src/gcc/testsuite/gfortran.dg/pr105954.f90", "uriBaseId": "PWD"}, "region": {"startLine": 6, "startColumn": 8, "endColumn": 9}, "contextRegion": {"startLine": 6, "snippet": {"text": "program p\n"}}}}]} which shows that no diagnostics have been issued yet, but the active diagnostic_buffer has a single error buffered within it, in SARIF form. Similarly, it's possible to use "dump" on a diagnostic_buffer to directly query its contents; here's the same example, this time with the text output format: (gdb) call error_buffer.buffer.dump() m_per_format_buffer: counts: error: 1 diagnostic_text_format_buffer: m_formatted_obstack current object: length 232: 00000000: 1b 5b 30 31 6d 1b 5b 4b 2e 2e 2f 2e 2e 2f 73 72 | .[01m.[K../../sr 00000010: 63 2f 67 63 63 2f 74 65 73 74 73 75 69 74 65 2f | c/gcc/testsuite/ 00000020: 67 66 6f 72 74 72 61 6e 2e 64 67 2f 70 72 31 30 | gfortran.dg/pr10 00000030: 35 39 35 34 2e 66 39 30 3a 36 3a 38 3a 1b 5b 6d | 5954.f90:6:8:.[m 00000040: 1b 5b 4b 0a 0a 20 20 20 20 36 20 7c 20 70 72 6f | .[K.. 6 | pro 00000050: 67 72 61 6d 20 70 0a 20 20 20 20 20 20 7c 20 20 | gram p. | 00000060: 20 20 20 20 20 20 1b 5b 30 31 3b 33 31 6d 1b 5b | .[01;31m.[ 00000070: 4b 31 1b 5b 6d 1b 5b 4b 0a 1b 5b 30 31 3b 33 31 | K1.[m.[K..[01;31 00000080: 6d 1b 5b 4b 45 72 72 6f 72 3a 1b 5b 6d 1b 5b 4b | m.[KError:.[m.[K 00000090: 20 46 75 6e 63 74 69 6f 6e 20 e2 80 98 1b 5b 30 | Function ....[0 000000a0: 31 6d 1b 5b 4b 70 72 6f 67 72 61 6d 1b 5b 6d 1b | 1m.[Kprogram.[m. 000000b0: 5b 4b e2 80 99 20 72 65 71 75 69 72 65 73 20 61 | [K... requires a 000000c0: 6e 20 61 72 67 75 6d 65 6e 74 20 6c 69 73 74 20 | n argument list 000000d0: 61 74 20 1b 5b 30 31 3b 33 31 6d 1b 5b 4b 28 31 | at .[01;31m.[K(1 000000e0: 29 1b 5b 6d 1b 5b 4b 0a | ).[m.[K. m_chunk_obstack current object: length 0: showing that we have an error in error_buffer, with colorized text. gcc/ChangeLog: PR fortran/105916 * diagnostic-buffer.h: New file. * diagnostic-format-json.cc: Define INCLUDE_VECTOR. Include "diagnostic-buffer.h". (class diagnostic_json_format_buffer): New subclass. (class json_output_format): Add friend class diagnostic_json_format_buffer. (json_output_format::make_per_format_buffer): New vfunc implementation. (json_output_format::set_buffer): New vfunc implementation. (json_output_format::json_output_format): Initialize m_buffer. (json_output_format::m_buffer): New field. (diagnostic_json_format_buffer::dump): New. (diagnostic_json_format_buffer::empty_p): New. (diagnostic_json_format_buffer::move_to): New. (diagnostic_json_format_buffer::clear): New. (diagnostic_json_format_buffer::flush): New. (json_output_format::on_report_diagnostic): Implement optional buffering. * diagnostic-format-sarif.cc: Include "diagnostic-buffer.h". (class diagnostic_sarif_format_buffer): New subclass. (class sarif_builder): Add friend class diagnostic_sarif_format_buffer. (sarif_builder::num_results): New accessor. (sarif_builder::get_result): New accessor. (sarif_builder::on_report_diagnostic): Add param "buffer"; use it to implement optional buffering. (diagnostic_sarif_format_buffer::dump): New. (diagnostic_sarif_format_buffer::empty_p): New. (diagnostic_sarif_format_buffer::move_to): New. (diagnostic_sarif_format_buffer::clear): New. (diagnostic_sarif_format_buffer::flush): New. (sarif_output_format::make_per_format_buffer): New vfunc implementation. (sarif_output_format::set_buffer): New vfunc implementation. (sarif_output_format::on_report_diagnostic): Pass m_buffer to sarif_builder::on_report_diagnostic. (sarif_output_format::num_results): New accessor. (sarif_output_format::get_result): New accessor. (diagnostic_output_format::diagnostic_output_format): Initialize m_buffer. (diagnostic_output_format::m_buffer): New field. (diagnostic_output_format::num_results): Get accessor. (diagnostic_output_format::get_result): Get accessor. (selftest::get_message_from_result): New. (selftest::test_buffering): New. (selftest::diagnostic_format_sarif_cc_tests): Call it. * diagnostic-format-text.cc: Include "diagnostic-client-data-hooks.h". (class diagnostic_text_format_buffer): New subclass. (diagnostic_text_format_buffer::diagnostic_text_format_buffer): New. (diagnostic_text_format_buffer::dump): New. (diagnostic_text_format_buffer::empty_p): New. (diagnostic_text_format_buffer::move_to): New. (diagnostic_text_format_buffer::clear): New. (diagnostic_text_format_buffer::flush): New. (diagnostic_text_output_format::dump): Dump m_saved_output_buffer. (diagnostic_text_output_format::set_buffer): New. (diagnostic_text_output_format::make_per_format_buffer): New. * diagnostic-format-text.h (diagnostic_text_output_format::diagnostic_text_output_format): Initialize m_saved_output_buffer. (diagnostic_text_output_format::set_buffer): New decl. (diagnostic_text_output_format::make_per_format_buffer): New decl. (diagnostic_text_output_format::m_saved_output_buffer): New field. * diagnostic-format.h (class diagnostic_per_format_buffer): New forward decl. (diagnostic_output_format::make_per_format_buffer): New vfunc. (diagnostic_output_format::set_buffer): New vfunc. * diagnostic.cc: Include "diagnostic-buffer.h". (diagnostic_context::initialize): Replace memset with call to "clear" on m_diagnostic_counters. Initializer m_diagnostic_buffer. (diagnostic_context::finish): Call set_diagnostic_buffer with nullptr. (diagnostic_context::dump): Update for encapsulation of counts into m_diagnostic_counters. Dump m_diagnostic_buffer. (diagnostic_context::execution_failed_p): Update for encapsulation of counts into m_diagnostic_counters. (diagnostic_context::check_max_errors): Likewise. (diagnostic_context::report_diagnostic): Likewise. Eliminate diagnostic_check_max_errors in favor of check_max_errors. Update increment of counter to support buffering. Eliminate diagnostic_action_after_output in favor of action_after_output. Only add fixits to m_edit_context_ptr if buffering is disabled. Only call diagnostic_output_format::after_diagnostic if buffering is disabled. (diagnostic_context::error_recursion): Eliminate diagnostic_action_after_output in favor of action_after_output. (diagnostic_context::set_diagnostic_buffer): New. (diagnostic_context::clear_diagnostic_buffer): New. (diagnostic_context::flush_diagnostic_buffer): New. (diagnostic_counters::diagnostic_counters): New. (diagnostic_counters::dump): New. (diagnostic_counters::move_to): New. (diagnostic_counters::clear): New. (diagnostic_buffer::diagnostic_buffer): New. (diagnostic_buffer::~diagnostic_buffer): New. (diagnostic_buffer::dump): New. (diagnostic_buffer::empty_p): New. (diagnostic_buffer::move_to): New. (diagnostic_buffer::ensure_per_format_buffer): New. (c_diagnostic_cc_tests): Remove stray newline. * diagnostic.h (class diagnostic_buffer): New forward decl. (struct diagnostic_counters): New. (diagnostic_context::check_max_errors): Make private. (diagnostic_context::action_after_output): Make private. (diagnostic_context::get_output_format): Make non-const. (diagnostic_context::diagnostic_count): Update for change to m_diagnostic_counters. (diagnostic_context::set_diagnostic_buffer): New decl. (diagnostic_context::get_diagnostic_buffer): New decl. (diagnostic_context::clear_diagnostic_buffer): New decl. (diagnostic_context::flush_diagnostic_buffer): New decl. (diagnostic_context::m_diagnostic_count): Replace array with... (diagnostic_context::m_diagnostic_counters): ...this. (diagnostic_context::m_diagnostic_buffer): New field. (diagnostic_action_after_output): Delete. (diagnostic_check_max_errors): Delete. gcc/fortran/ChangeLog: PR fortran/105916 * error.cc (pp_error_buffer, pp_warning_buffer): Convert from output_buffer * to diagnostic_buffer *. (warningcount_buffered, werrorcount_buffered): Eliminate. (gfc_error_buffer::gfc_error_buffer): Move constructor definition here, and initialize "buffer" using *global_dc. (gfc_output_buffer_empty_p): Delete in favor of diagnostic_buffer::empty_p. (gfc_clear_pp_buffer): Replace with... (gfc_clear_diagnostic_buffer): ...this, moving implementation details to diagnostic_context::clear_diagnostic_buffer. (gfc_warning): Replace buffering implementation with calls to global_dc->get_diagnostic_buffer and global_dc->set_diagnostic_buffer. (gfc_clear_warning): Update for renaming of gfc_clear_pp_buffer and elimination of warningcount_buffered and werrorcount_buffered. (gfc_warning_check): Replace buffering implementation with calls to pp_warning_buffer->empty_p and global_dc->flush_diagnostic_buffer. (gfc_error_opt): Replace buffering implementation with calls to global_dc->get_diagnostic_buffer and set_diagnostic_buffer. (gfc_clear_error): Update for renaming of gfc_clear_pp_buffer. (gfc_error_flag_test): Replace call to gfc_output_buffer_empty_p with call to diagnostic_buffer::empty_p. (gfc_error_check): Replace buffering implementation with calls to pp_error_buffer->empty_p and global_dc->flush_diagnostic_buffer. (gfc_move_error_buffer_from_to): Replace buffering implementation with usage of diagnostic_buffer. (gfc_free_error): Update for renaming of gfc_clear_pp_buffer. (gfc_diagnostics_init): Use "new" directly when creating pp_warning_buffer. Remove setting of m_flush_p on the two buffers, as this is handled by diagnostic_buffer and by diagnostic_text_format_buffer's constructor. * gfortran.h: Replace #include "pretty-print.h" for output_buffer with #include "diagnostic-buffer.h" for diagnostic_buffer. (struct gfc_error_buffer): Change type of field "buffer" from output_buffer to diagnostic_buffer. Move definition of constructor into error.cc so that it can use global_dc. gcc/testsuite/ChangeLog: PR fortran/105916 * gcc.dg/plugin/diagnostic_plugin_xhtml_format.c: Include "diagnostic-buffer.h". (class diagnostic_xhtml_format_buffer): New subclass. (class xhtml_builder): Add friend class diagnostic_xhtml_format_buffer. (diagnostic_xhtml_format_buffer::dump): New. (diagnostic_xhtml_format_buffer::empty_p): New. (diagnostic_xhtml_format_buffer::move_to): New. (diagnostic_xhtml_format_buffer::clear): New. (diagnostic_xhtml_format_buffer::flush): New. (xhtml_builder::on_report_diagnostic): Add "buffer" param, and use it. (xhtml_output_format::dump): Fix typo. (xhtml_output_format::make_per_format_buffer): New. (xhtml_output_format::set_buffer): New. (xhtml_output_format::on_report_diagnostic): Fix whitespace. Pass m_buffer to xhtml_builder::on_report_diagnostic. (xhtml_output_format::xhtml_output_format): Initialize m_buffer. (xhtml_output_format::m_buffer): New field. * gfortran.dg/diagnostic-format-json-pr105916.F90: New test. * gfortran.dg/diagnostic-format-sarif-1.F90: New test. * gfortran.dg/diagnostic-format-sarif-1.py: New support script. * gfortran.dg/diagnostic-format-sarif-pr105916.f90: New test. Signed-off-by: David Malcolm <dmalc...@redhat.com>
