fix -fmax-errors & notes

David Malcolm dmalcolm@redhat.com
Tue Oct 11 20:07:00 GMT 2016


On Tue, 2016-10-11 at 06:34 -0400, Nathan Sidwell wrote:
> Hi,
> Jonathan & I were chatting at the cauldron about how -fmax-errors
> kills any 
> notes about the final error.  That's because we bail out just after
> emitting the 
> final error.  This patch fixes the problem by bailing out just before
> emitting 
> the error or warning after that.
> 
> Sure, we'll do slightly more compilation than asked for, but this is
> the fatal 
> error path, so who cares how long it takes.  Better to get notes to
> the user.
> 
> I augmented the fmax-errors testcase so that the final emitted error
> has a 
> trailing note (which we now emit), and is followed by a warning
> (which causes us 
> to bail).
> 
> The same logic applies to -Wfatal-errors handling.
> 
> ok?

Thanks.  Various comments inline below.

> 2016-10-11  Nathan Sidwell  <nathan@acm.org>
> 
> 	* diagnostic.c (diagnostic_action_after_output): Remove fatal
> 	and max error handling here ....
> 	(diagnostic_report_diagnostic): ... do it here instead.
> 
> 	testsuite/
> 	* c-c++-common/fmax-errors.c: Add error with note.
> 
> Index: diagnostic.c
> ===================================================================
> --- diagnostic.c	(revision 240920)
> +++ diagnostic.c	(working copy)
> @@ -464,24 +464,6 @@ diagnostic_action_after_output (diagnost
>      case DK_SORRY:
>        if (context->abort_on_error)
>  	real_abort ();
> -      if (context->fatal_errors)
> -	{
> -	  fnotice (stderr, "compilation terminated due to -Wfatal
-errors.\n");
> -	  diagnostic_finish (context);
> -	  exit (FATAL_EXIT_CODE);
> -	}
> -      if (context->max_errors != 0
> -	  && ((unsigned) (diagnostic_kind_count (context, DK_ERROR)
> -			  + diagnostic_kind_count (context,
DK_SORRY)
> -			  + diagnostic_kind_count (context,
DK_WERROR))
> -	      >= context->max_errors))
> -	{
> -	  fnotice (stderr,
> -		   "compilation terminated due to -fmax
-errors=%u.\n",
> -		   context->max_errors);
> -	  diagnostic_finish (context);
> -	  exit (FATAL_EXIT_CODE);
> -	}
>        break;
>  
>      case DK_ICE:
> @@ -890,6 +872,25 @@ diagnostic_report_diagnostic (diagnostic
>  	return false;
>      }
>  
> +  if (diagnostic->kind != DK_NOTE
> +      && (unsigned)(diagnostic_kind_count (context, DK_ERROR)
> +		    + diagnostic_kind_count (context, DK_SORRY)
> +		    + diagnostic_kind_count (context, DK_WERROR))
> +      >= (context->fatal_errors ? 1
> +	  : context->max_errors ? context->max_errors : ~0u))

Please simplify this conditional logic by breaking if up into multiple
nested if statements:

   if (diagnostic->kind != DK_NOTE)
     {
        int count =
(diagnostic_kind_count (context, DK_ERROR)
              	     +
diagnostic_kind_count (context, DK_SORRY)
                     +
diagnostic_kind_count (context, DK_WERROR);
or somesuch...

> +      >= (context->fatal_errors ? 1
> +	  : context->max_errors ? context->max_errors : ~0u))
> +    {
> +      /* Check before emitting the diagnostic that would exceed the
> +	 limit.  This way we will emit notes relevant to the final
> +	 emitted error.  */
> +      fnotice (stderr,
> +	       context->fatal_errors
> +	       ? "compilation terminated due to -Wfatal-errors.\n"
> +	       : "compilation terminated due to -fmax-errors=%u.\n",
> +	       context->max_errors);
> +      diagnostic_finish (context);
> +      exit (FATAL_EXIT_CODE);
> +    }

This logic is running when the next diagnostic is about to be emitted.
But what if the user has selected -Wfatal-errors and there's a single
error and no further diagnostics?  Could this change the observable
behavior?  (I'm trying to think of a case here, but failing).


>    context->lock++;
>  
>    if (diagnostic->kind == DK_ICE || diagnostic->kind == DK_ICE_NOBT)
> Index: testsuite/c-c++-common/fmax-errors.c
> ===================================================================
> --- testsuite/c-c++-common/fmax-errors.c	(revision 240920)
> +++ testsuite/c-c++-common/fmax-errors.c	(working copy)
> @@ -1,11 +1,13 @@
>  /* PR c/44782 */
>  /* { dg-do compile } */
> -/* { dg-options "-fmax-errors=3" } */
> +/* { dg-options "-fmax-errors=3 -Wall" } */
>  
>  void foo (unsigned int i, unsigned int j)
>  {
>    (i) ();			/* { dg-error "" } */
>    (j) ();			/* { dg-error "" } */
> -  (i+j) ();			/* { dg-error "" } */
> +  (k) ();			/* { dg-error "" } */
> +  /* { dg-message "identifier" "" { target c } 9 } */
> +  i + j; /* no warning.  */
>    (i*j) ();			/* no error here due to -fmax
-errors */
>  } /* { dg-prune-output "compilation terminated" } */

Please can you add a comment about the dg-message directive, explaining
the intent.  ("Verify that with -fmax-errors that a note associated
with the final error is still emitted", or somesuch").

Please can you add a testcase for -Wfatal-errors.

Thanks
Dave




More information about the Gcc-patches mailing list