Skip to content
Snippets Groups Projects
Commit f565063a authored by David Malcolm's avatar David Malcolm
Browse files

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: default avatarDavid Malcolm <dmalcolm@redhat.com>
parent de2dc623
No related branches found
No related tags found
Loading
Showing
with 1069 additions and 191 deletions
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment