[4.4] remove diagnostic_count_diagnostic

[4.4] remove diagnostic_count_diagnostic

[Manuel López-Ibáñez]

[-- Attachment #1: Type: text/plain, Size: 361 bytes --]

The function diagnostic_count_diagnostic is unnecessary: it always
return true, the switch is not needed. This patch removes it.

Bootstrapped and regression tested on x86_64-unknown-linux-gnu

2008-02-24  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>

 * diagnostic.c (diagnostic_count_diagnostic): Delete.
 (diagnostic_report_diagnostic): Update. Handle ICEs here.

[-- Attachment #2: remove-count-diagnostic.diff --]
[-- Type: text/plain, Size: 5863 bytes --]

Index: gcc/diagnostic.c
===================================================================
--- gcc/diagnostic.c	(revision 132166)
+++ gcc/diagnostic.c	(working copy)
@@ -49,12 +49,11 @@ static void default_diagnostic_starter (
 					diagnostic_info *);
 static void default_diagnostic_finalizer (diagnostic_context *,
 					  diagnostic_info *);
 
 static void error_recursion (diagnostic_context *) ATTRIBUTE_NORETURN;
-static bool diagnostic_count_diagnostic (diagnostic_context *,
-					 diagnostic_info *);
+
 static void diagnostic_action_after_output (diagnostic_context *,
 					    diagnostic_info *);
 static void real_abort (void) ATTRIBUTE_NORETURN;
 
 /* A diagnostic_context surrogate for stderr.  */
@@ -162,58 +161,10 @@ diagnostic_build_prefix (diagnostic_info
      ? build_message_string ("%s:%d:%d: %s", s.file, s.line, s.column, text)
 #endif
      : build_message_string ("%s:%d: %s", s.file, s.line, text));
 }
 
-/* Count a diagnostic.  Return true if the message should be printed.  */
-static bool
-diagnostic_count_diagnostic (diagnostic_context *context,
-			     diagnostic_info *diagnostic)
-{
-  diagnostic_t kind = diagnostic->kind;
-  switch (kind)
-    {
-    default:
-      gcc_unreachable ();
-
-    case DK_ICE:
-#ifndef ENABLE_CHECKING
-      /* When not checking, ICEs are converted to fatal errors when an
-	 error has already occurred.  This is counteracted by
-	 abort_on_error.  */
-      if ((diagnostic_kind_count (context, DK_ERROR) > 0
-	   || diagnostic_kind_count (context, DK_SORRY) > 0)
-	  && !context->abort_on_error)
-	{
-	  expanded_location s = expand_location (diagnostic->location);
-	  fnotice (stderr, "%s:%d: confused by earlier errors, bailing out\n",
-		   s.file, s.line);
-	  exit (ICE_EXIT_CODE);
-	}
-#endif
-      if (context->internal_error)
-	(*context->internal_error) (diagnostic->message.format_spec,
-				    diagnostic->message.args_ptr);
-      /* Fall through.  */
-
-    case DK_FATAL: case DK_SORRY:
-    case DK_ANACHRONISM: case DK_NOTE:
-      ++diagnostic_kind_count (context, kind);
-      break;
-
-    case DK_WARNING:
-      ++diagnostic_kind_count (context, DK_WARNING);
-      break;
-
-    case DK_ERROR:
-      ++diagnostic_kind_count (context, DK_ERROR);
-      break;
-    }
-
-  return true;
-}
-
 /* Take any action which is expected to happen after the diagnostic
    is written out.  This function does not always return.  */
 static void
 diagnostic_action_after_output (diagnostic_context *context,
 				diagnostic_info *diagnostic)
@@ -343,10 +294,11 @@ diagnostic_classify_diagnostic (diagnost
 void
 diagnostic_report_diagnostic (diagnostic_context *context,
 			      diagnostic_info *diagnostic)
 {
   bool maybe_print_warnings_as_errors_message = false;
+  const char *saved_format_spec;
 
   /* Give preference to being able to inhibit warnings, before they
      get reclassified to something else.  */
   if (diagnostic->kind == DK_WARNING 
       && !diagnostic_report_warnings_p ())
@@ -403,31 +355,49 @@ diagnostic_report_diagnostic (diagnostic
       context->issue_warnings_are_errors_message = false;
     }
 
   context->lock++;
 
-  if (diagnostic_count_diagnostic (context, diagnostic))
+  if (diagnostic->kind == DK_ICE) 
     {
-      const char *saved_format_spec = diagnostic->message.format_spec;
-
-      if (context->show_option_requested && diagnostic->option_index)
-	diagnostic->message.format_spec
-	  = ACONCAT ((diagnostic->message.format_spec,
-		      " [", cl_options[diagnostic->option_index].opt_text, "]", NULL));
-
-      diagnostic->message.locus = &diagnostic->location;
-      diagnostic->message.abstract_origin = &diagnostic->abstract_origin;
-      diagnostic->abstract_origin = NULL;
-      pp_format (context->printer, &diagnostic->message);
-      (*diagnostic_starter (context)) (context, diagnostic);
-      pp_output_formatted_text (context->printer);
-      (*diagnostic_finalizer (context)) (context, diagnostic);
-      pp_flush (context->printer);
-      diagnostic_action_after_output (context, diagnostic);
-      diagnostic->message.format_spec = saved_format_spec;
-      diagnostic->abstract_origin = NULL;
+#ifndef ENABLE_CHECKING
+      /* When not checking, ICEs are converted to fatal errors when an
+	 error has already occurred.  This is counteracted by
+	 abort_on_error.  */
+      if ((diagnostic_kind_count (context, DK_ERROR) > 0
+	   || diagnostic_kind_count (context, DK_SORRY) > 0)
+	  && !context->abort_on_error)
+	{
+	  expanded_location s = expand_location (diagnostic->location);
+	  fnotice (stderr, "%s:%d: confused by earlier errors, bailing out\n",
+		   s.file, s.line);
+	  exit (ICE_EXIT_CODE);
+	}
+#endif
+      if (context->internal_error)
+	(*context->internal_error) (diagnostic->message.format_spec,
+				    diagnostic->message.args_ptr);
     }
+  ++diagnostic_kind_count (context, diagnostic->kind);
+  
+  saved_format_spec = diagnostic->message.format_spec;
+  if (context->show_option_requested && diagnostic->option_index)
+    diagnostic->message.format_spec
+      = ACONCAT ((diagnostic->message.format_spec,
+                  " [", cl_options[diagnostic->option_index].opt_text, "]", NULL));
+  
+  diagnostic->message.locus = &diagnostic->location;
+  diagnostic->message.abstract_origin = &diagnostic->abstract_origin;
+  diagnostic->abstract_origin = NULL;
+  pp_format (context->printer, &diagnostic->message);
+  (*diagnostic_starter (context)) (context, diagnostic);
+  pp_output_formatted_text (context->printer);
+  (*diagnostic_finalizer (context)) (context, diagnostic);
+  pp_flush (context->printer);
+  diagnostic_action_after_output (context, diagnostic);
+  diagnostic->message.format_spec = saved_format_spec;
+  diagnostic->abstract_origin = NULL;
 
   context->lock--;
 }
 
 /* Given a partial pathname as input, return another pathname that

Re: [4.4] remove diagnostic_count_diagnostic

[Gabriel Dos Reis]

On Sun, Feb 24, 2008 at 2:13 PM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> The function diagnostic_count_diagnostic is unnecessary: it always
>  return true, the switch is not needed. This patch removes it.
>
>  Bootstrapped and regression tested on x86_64-unknown-linux-gnu
>
>  2008-02-24  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
>
>   * diagnostic.c (diagnostic_count_diagnostic): Delete.
>   (diagnostic_report_diagnostic): Update. Handle ICEs here.

I believe this patch misses the semantics of the function.
It isn't just to return true.  It also count the number of
diagnostic kinds emitted.

>

Re: [4.4] remove diagnostic_count_diagnostic

[Manuel López-Ibáñez]

On 25/02/2008, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
> I believe this patch misses the semantics of the function.
>  It isn't just to return true.  It also count the number of
>  diagnostic kinds emitted.

This is done by the following line:

+  ++diagnostic_kind_count (context, diagnostic->kind);

No more is needed to count a diagnostic. Of course, we could
encapsulate that in a function:

static void
diagnostic_count_diagnostic (diagnostic_context *context,
			     diagnostic_info *diagnostic)
{
 ++diagnostic_kind_count (context, diagnostic->kind);
}

Do we really need to do that? Nobody outside diagnostics.c is using
such function.

Cheers,

Manuel.

Re: [4.4] remove diagnostic_count_diagnostic

[Gabriel Dos Reis]

On Mon, Feb 25, 2008 at 1:40 PM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 25/02/2008, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
>  > I believe this patch misses the semantics of the function.
>  >  It isn't just to return true.  It also count the number of
>  >  diagnostic kinds emitted.
>
>  This is done by the following line:
>
>  +  ++diagnostic_kind_count (context, diagnostic->kind);
>
>  No more is needed to count a diagnostic. Of course, we could
>  encapsulate that in a function:
>
>  static void
>  diagnostic_count_diagnostic (diagnostic_context *context,
>                              diagnostic_info *diagnostic)
>  {
>   ++diagnostic_kind_count (context, diagnostic->kind);
>  }
>
>  Do we really need to do that? Nobody outside diagnostics.c is using
>  such function.
>

Counting the different kinds of diagnostics is all that I would like retained.


>  Cheers,
>
>  Manuel.
>

Re: [4.4] remove diagnostic_count_diagnostic

[Manuel López-Ibáñez]

On 25/02/2008, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
> On Mon, Feb 25, 2008 at 1:40 PM, Manuel López-Ibáñez
>  <lopezibanez@gmail.com> wrote:
>  > On 25/02/2008, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
>  >  > I believe this patch misses the semantics of the function.
>  >  >  It isn't just to return true.  It also count the number of
>  >  >  diagnostic kinds emitted.
>  >
>  >  This is done by the following line:
>  >
>  >  +  ++diagnostic_kind_count (context, diagnostic->kind);
>  >
>  >  No more is needed to count a diagnostic. Of course, we could
>  >  encapsulate that in a function:
>  >
>  >  static void
>  >  diagnostic_count_diagnostic (diagnostic_context *context,
>  >                              diagnostic_info *diagnostic)
>  >  {
>  >   ++diagnostic_kind_count (context, diagnostic->kind);
>  >  }
>  >
>  >  Do we really need to do that? Nobody outside diagnostics.c is using
>  >  such function.
>  >
>
>
> Counting the different kinds of diagnostics is all that I would like retained.

But my patch does that, so what do you mean?

Cheers,

Manuel.

Re: [4.4] remove diagnostic_count_diagnostic

[Manuel López-Ibáñez]

PING: http://gcc.gnu.org/ml/gcc-patches/2008-02/msg01108.html

Gabriel,

If the unified patch is confusing you, please look at the resulting
code. No behaviour is changed at all.

Cheers,

Manuel.

On 26/02/2008, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
> On 25/02/2008, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
>  > On Mon, Feb 25, 2008 at 1:40 PM, Manuel López-Ibáñez
>  >  <lopezibanez@gmail.com> wrote:
>  >  > On 25/02/2008, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
>  >  >  > I believe this patch misses the semantics of the function.
>  >  >  >  It isn't just to return true.  It also count the number of
>  >  >  >  diagnostic kinds emitted.
>  >  >
>  >  >  This is done by the following line:
>  >  >
>  >  >  +  ++diagnostic_kind_count (context, diagnostic->kind);
>  >  >
>  >  >  No more is needed to count a diagnostic. Of course, we could
>  >  >  encapsulate that in a function:
>  >  >
>  >  >  static void
>  >  >  diagnostic_count_diagnostic (diagnostic_context *context,
>  >  >                              diagnostic_info *diagnostic)
>  >  >  {
>  >  >   ++diagnostic_kind_count (context, diagnostic->kind);
>  >  >  }
>  >  >
>  >  >  Do we really need to do that? Nobody outside diagnostics.c is using
>  >  >  such function.
>  >  >
>  >
>  >
>  > Counting the different kinds of diagnostics is all that I would like retained.
>
>
> But my patch does that, so what do you mean?
>
>  Cheers,
>
>
>  Manuel.
>

Re: [4.4] remove diagnostic_count_diagnostic

[Gabriel Dos Reis]

On Fri, Mar 14, 2008 at 4:48 AM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> PING: http://gcc.gnu.org/ml/gcc-patches/2008-02/msg01108.html

OK.