diff mbox

[Fortran] convert almost all {warning,error}_now to common diagnostic

Message ID CAESRpQAM70OR57r-CqqvdFCyaku0-0grx08Ynr3wax_xitnbgQ@mail.gmail.com
State New
Headers show

Commit Message

Manuel López-Ibáñez Nov. 26, 2014, 6:06 p.m. UTC 
On 25 November 2014 at 23:42, Tobias Burnus <burnus@net-b.de> wrote:
> FX:
>>>
>>> (a) those majority which might need buffering (gfc_error, gfc_warning);
>>
>> Is there a plan for those in the longer term?
>
>
> Well, the long-term solution is of course to support them. That requires
> adding buffering+discarding support to the common machinery and to use it
> then in gfortran. It's on Manuel's to-do list, but he suffers from the
> generic problem of finding the time to actually do it. Maybe he manages to
> finish that part by early next year - which might or might not be suitable
> for GCC 5 inclusion.

I think the changes to the common diagnostics part could be as simple as:


However, I'm less familiar with the Fortran buffering mechanism. So
far, I understood that:

* gfc uses two gfc_error_buf (error_buffer, warning_buffer) and
pointer *cur_error_buffer; to switch between them. These are
equivalent to pretty-printer output_buffer, thus my idea is to create
a gfc_pretty_printer that contains three output_buffer (two for
buffering and another for the _now variants).

* There seems to be a global buffer_flag that controls when to buffer.
However, each gfc_error_buf has a variable "flag", that is also
checked sometimes. Unfortunately, nothing in gfc_error_buf has
comments, so I have no idea how these flags are supposed to interact.
It would be great if a Fortran person could explain it in detail.

* I am not sure how to handle the error/warnings counts. The Fortran
code seems to not care about how many errors or warnings are given
when buffering, only whether there was any error at all. Is that
right? It also seems to not care about -fmax-errors (similarly
-Wfatal-errors will only take care after flushing, thus potentially
many errors can be given before stopping). Is this correct? Not having
to handle those would simplify a first version a lot.

* I assume that ICEs and other such fatal_errors are never buffered.
For normal errors converted to fatal by Wfatal-errors, I just need to
be careful in the buffered variants, that is disable it before going
through the common machinery and re-enable it after returning from it.
(This is a note to myself).

Cheers,

Manuel.

Comments

Tobias Burnus Nov. 26, 2014, 7:26 p.m. UTC | #1 
Hi Manuel,

Manuel López-Ibáñez wrote:
> I think the changes to the common diagnostics part could be as simple as:
Good – that's simpler than I had feared.

> However, I'm less familiar with the Fortran buffering mechanism.

I am not sure you find someone who really is. At least I also have 
troubles – but most of the time it works as expected.

An implementation criterion would be that it doesn't regress, except 
that changes which do make sense (are better or equivalent) are still fine.

It doesn't have to be completely identical.

> So far, I understood that:
>
> * gfc uses two gfc_error_buf (error_buffer, warning_buffer) and
> pointer *cur_error_buffer; to switch between them. These are
> equivalent to pretty-printer output_buffer, thus my idea is to create
> a gfc_pretty_printer that contains three output_buffer (two for
> buffering and another for the _now variants).
>
> * There seems to be a global buffer_flag that controls when to buffer.
> However, each gfc_error_buf has a variable "flag", that is also
> checked sometimes. Unfortunately, nothing in gfc_error_buf has
> comments, so I have no idea how these flags are supposed to interact.
> It would be great if a Fortran person could explain it in detail.

Well gfc_error_buf's "flag" seems to be 1 if there is a buffered warning 
– and it is set to 0 if either the warning has been printed or dropped 
(e.g. via gfc_clear_error()). Thus, when 
gfc_error_check()/gfc_warning_check() is called, the buffer can be 
printed ("fputs (warning_buffer.message, stderr);").

The only reason that "warning_buffer.message" isn't directly used seems 
to be that that way the memory allocation for the buffer can be reused.

The buffering itself seems to be only controlled by "buffer_flag".

> * I am not sure how to handle the error/warnings counts. The Fortran
> code seems to not care about how many errors or warnings are given
> when buffering, only whether there was any error at all. Is that
> right? It also seems to not care about -fmax-errors (similarly
> -Wfatal-errors will only take care after flushing, thus potentially
> many errors can be given before stopping). Is this correct? Not having
> to handle those would simplify a first version a lot.

Looking at the code, newer errors even seem to always override previous 
ones:

The gfc_{error,warning}{,_now_1} functions all have code like:
   error_buffer.index = 0;
which effectively clears the error buffer by overriding it with the new 
string.

Thus, one has only to take care of incrementing the count when actually 
printing the buffer.

> * I assume that ICEs and other such fatal_errors are never buffered.
> For normal errors converted to fatal by Wfatal-errors, I just need to
> be careful in the buffered variants, that is disable it before going
> through the common machinery and re-enable it after returning from it.
> (This is a note to myself).

Tobias
diff mbox

Patch

Index: gcc/pretty-print.c
===================================================================
--- gcc/pretty-print.c  (revision 218090)
+++ gcc/pretty-print.c  (working copy)
@@ -677,19 +677,33 @@  pp_format_verbatim (pretty_printer *pp,

   /* Restore previous settings.  */
   pp_wrapping_mode (pp) = oldmode;
 }

-/* Flush the content of BUFFER onto the attached stream.  */
+/* Flush the content of BUFFER onto the attached stream.  This
+   function does nothing unless pp->output_buffer->flush_p.  */
 void
 pp_flush (pretty_printer *pp)
 {
+  if (!pp->output_buffer->flush_p)
+    return;
   pp_write_text_to_stream (pp);
   pp_clear_state (pp);
   fflush (pp_buffer (pp)->stream);
 }

+/* Flush the content of BUFFER onto the attached stream independently
+   of the value of pp->output_buffer->flush_p.  */
+void
+pp_really_flush (pretty_printer *pp)
+{
+  pp_write_text_to_stream (pp);
+  pp_clear_state (pp);
+  fflush (pp_buffer (pp)->stream);
+}
+
+
 /* Sets the number of maximum characters per line PRETTY-PRINTER can
    output in line-wrapping mode.  A LENGTH value 0 suppresses
    line-wrapping.  */
 void
 pp_set_line_maximum_length (pretty_printer *pp, int length)
@@ -772,11 +786,12 @@  pretty_printer::pretty_printer (const ch
     wrapping (),
     format_decoder (),
     emitted_prefix (),
     need_newline (),
     translate_identifiers (true),
-    show_color ()
+    show_color (),
+    flush_p (true)
 {
   pp_line_cutoff (this) = l;
   /* By default, we emit prefixes once per message.  */
   pp_prefixing_rule (this) = DIAGNOSTICS_SHOW_PREFIX_ONCE;
   pp_set_prefix (this, p);
Index: gcc/pretty-print.h
===================================================================
--- gcc/pretty-print.h  (revision 218090)
+++ gcc/pretty-print.h  (working copy)
@@ -98,10 +98,15 @@  struct output_buffer
   int line_length;

   /* This must be large enough to hold any printed integer or
      floating-point value.  */
   char digit_buffer[128];
+
+  /* Nonzero means that text should be flushed when
+     appropriate. Otherwise, text is buffered until either
+     pp_pp_really_flush or pp_clear_output_area are called. */
+  bool flush_p;
 };

 /* The type of pretty-printer flags passed to clients.  */
 typedef unsigned int pp_flags;