On Wed, 2024-10-23 at 11:03 +0200, Tobias Burnus wrote: > David Malcolm wrote: > > In order to handle various awkward 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 patch moves responsibility for such buffering of diagnostics > > from > > fortran's error.cc to the diagnostic subsystem. > ... > > Does this look OK from the Fortran side? The Fortran changes are > > essentially all specific to error.cc, converting from manipulations > > of > > output_buffer to those of diagnostic_buffer. > > Yes, LGTM. (I only looked at the Fortran changes.)
Thanks; I've gone ahead and pushed it to trunk (as r15-4575- gf565063a0602ad). > > Thanks, > > Tobias > > PS: I guess we eventually want to have in the testsuite some Fortran > SARIF tests, which for actual Fortran errors/warnings and not "just" > for > #error. The fortran testsuite currently has test coverage for the following case: * json #error and #warning (unbuffered diagnostics) * sarif #error (unbuffered diagnostic) * json and sarif coverage for PR105916 (buffered diagnostics that are discarded) but I realize we don't have coverage there for the case where buffered diagnostics are flushed/emitted. I can add this, but my Fortran skills are non-existent, so is there a trivial case that would hit this path? Ideas for test coverage would be most welcome. There is some test coverage for this case in the selftests in diagnostic.cc, but not yet a DejaGnu end-to-end test of it in terms of invoking gfortran and checking the .sarif output. Note that I've deprecated the "json" output format, so new testcases for machine-readable diagnostics should be for sarif. > > > I'm hoping to get this in as I have followup work to support having > > e.g. > > both text *and* SARIF at once (PR other/116613), and fixing this is > > a > > prerequisite for that work. Thanks again for the review; I'll get back to working on PR other/116613. I'll try to add a Fortran test case for that, e.g. for outputting both sarif 2.1 *and* sarif 2.2 from the same invocation. Dave > > > > Thanks > > Dave > > > > gcc/ChangeLog: > > PR fortran/105916 > > * diagnostic-buffer.h: New file. > ... > > 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". > ... > > * 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. >
