Re: [patch, fortran] pr21061 - gfortran ignores -Werror

[Bernhard Fischer <rep dot nop at aon dot at>]

On Tue, Nov 07, 2006 at 01:43:09AM -0800, Brooks Moses wrote:
>Bernhard Fischer wrote:
>>Attached is a revised and much simpler fix for PR fortran/21061 where
>>-Werror was ignored. The attached patch makes sure that we exit(1)
>>properly even for warnings.
>
>I have a couple of belated question about this patch to error.c:
>
>In the following code (gfc_warning, but gfc_warning_now is identical), 
>it increments both errors and warnings when warnings_are_errors is true:
>
>>>@@ -475,7 +475,12 @@ gfc_warning (const char *nocmsgid, ...)
>>>
>>>  va_start (argp, nocmsgid);
>>>  if (buffer_flag == 0)
>>>+  {
>>>    warnings++;
>>>+    if (warnings_are_errors)
>>>+      errors++;
>>>+  }
>
>However, in gfc_notify_std, when it would normally produce a warning but 
>warnings_are_errors is true, it only increments errors, not warnings:
>
>>>-      if (warning)
>>>+      if (warning && !warnings_are_errors)
>>>	warnings++;
>>>      else
>>>	errors++;
>
>Is this discrepancy intentional, or should one of these be changed to be 
>consistent with the other?

That's not intentional, but does diagnostic.c do anything with our warningcount?
We have a gfc_get_errors() but we do not use the warnings at all there.
>
>
>Also, there is this hunk in gfc_notify_std:
>>>@@ -518,14 +523,15 @@ gfc_notify_std (int std, const char *noc
>>> 
>>>   if (gfc_suppress_error)
>>>     return warning ? SUCCESS : FAILURE;
>>>-  
>>>-  cur_error_buffer = warning ? &warning_buffer : &error_buffer;
>>>+
>>>+  cur_error_buffer = (warning && !warnings_are_errors)
>>>+    ? &warning_buffer : &error_buffer;
>>>   cur_error_buffer->flag = 1;
>>>   cur_error_buffer->index = 0;
>>> 
>
>I don't follow why this is changed.  In gfc_warning, when warnings are 
>treated as errors, they're still buffered into warning_buffer rather 
>than into error_buffer.  Why buffer warnings into error_buffer here?

I thought that there was a reason to do this, but after rereading it,
you're right that it doesn't make sense.

Index: error.c
===================================================================
--- error.c     (revision 118542)
+++ error.c     (working copy)
@@ -525,17 +525,18 @@
   if (gfc_suppress_error)
     return warning ? SUCCESS : FAILURE;
 
-  cur_error_buffer = (warning && !warnings_are_errors)
-    ? &warning_buffer : &error_buffer;
+  cur_error_buffer = warning ? &warning_buffer : &error_buffer;
   cur_error_buffer->flag = 1;
   cur_error_buffer->index = 0;
 
   if (buffer_flag == 0)
     {
-      if (warning && !warnings_are_errors)
+      if (warning)
        warnings++;
       else
        errors++;
+      if (warning && warnings_are_errors)
+       errors++;
     }
   va_start (argp, nocmsgid);
   if (warning)


Bernhard Fischer  <>
	* error.c (gfc_notify_std): Disregard warnings_are_errors when
	deciding which logbuffer to use. Also increment warnings if
	warnings_are_errors is set.

Would that be ok?
>
>(In particular, I'm wondering if this has to do with lines 2606-2610 of 
>match.c, which implement a gfc_notify_std_now sort of thing assuming 
>that if gfc_notify_std has returned FAILURE that the result is in 
>error_buffer, or whether there's a more global reason for it...?)

The initial reasoning from my part was to use the error buffer even for
warnings if Werror was set. This was dropped for the sake of simplicity
and since it was not really needed AFAICS.

match.c lines 2599-2604 really look like they want gfc_error_now instead
of the gfc_error, gfc_error_check combo. Or is there a subtile reason
for the combo?

The lines 2606-2610 look like it want's a gfc_notify_std_now, to print
an eventual error ASAP, agree.

thanks,