diff mbox

[Diagnostic] Clean-up diagnostic facilities in diagnostic.c

Message ID 57553A79.4060508@oracle.com
State New
Headers show

Commit Message

Paolo Carlini June 6, 2016, 8:55 a.m. UTC 
Hi,

yesterday I had the idea of this small clean-up: move the work done by 
emit_diagnostic to a new non-variadic diagnostic_impl and use it to 
implement the former and all the various inform, warning, permerror, etc 
(lately we have the *_at_rich_loc variants too). Something similar can 
be done for inform_n, warning_n and errror_n. There is the minor 
subtlety of the declarations decorated with ATTRIBUTE_GCC_DIAG when 
available to suppress the build-time warning: I think I did it the right 
way, I took inspiration from the declarations of diagnostic_set_info, 
etc, in diagnostic.h.

Tested x86_64-linux.

Thanks,
Paolo.

////////////////////////////////
2016-06-06  Paolo Carlini  <paolo.carlini@oracle.com>

	* diagnostic.c (diagnostic_impl, diagnostic_n_impl): New.
	(inform, inform_at_rich_loc, inform_n, warning, warning_at,
	warning_at_rich_loc, warning_n, pedwarn, permerror,
	permerror_at_rich_loc, error, error_n, error_at, error_at_rich_loc,
	sorry, fatal_error, internal_error, internal_error_no_backtrace):
	Use the above.

Comments

David Malcolm June 6, 2016, 8:26 p.m. UTC | #1 
On Mon, 2016-06-06 at 10:55 +0200, Paolo Carlini wrote:
> Hi,
> 
> yesterday I had the idea of this small clean-up: move the work done
> by 
> emit_diagnostic to a new non-variadic diagnostic_impl and use it to 
> implement the former and all the various inform, warning, permerror,
> etc 
> (lately we have the *_at_rich_loc variants too). Something similar
> can 
> be done for inform_n, warning_n and errror_n. There is the minor 
> subtlety of the declarations decorated with ATTRIBUTE_GCC_DIAG when 
> available to suppress the build-time warning: I think I did it the
> right 
> way, I took inspiration from the declarations of diagnostic_set_info,
> etc, in diagnostic.h.
> 
> Tested x86_64-linux.

Thanks, looks like a nice simplification.

I see the new prototypes are wrapped in #ifdef ATTRIBUTE_GCC_DIAG.
Isn't that redundant?  Looking at diagnostic-core.h, isn't it always
defined? (perhaps to the empty string)

Also the new functions are missing comments.

Other than that, LGTM.
diff mbox

Patch

Index: diagnostic.c
===================================================================
--- diagnostic.c	(revision 237117)
+++ diagnostic.c	(working copy)
@@ -46,6 +46,13 @@  along with GCC; see the file COPYING3.  If not see
 #define permissive_error_option(DC) ((DC)->opt_permissive)
 
 /* Prototypes.  */
+#ifdef ATTRIBUTE_GCC_DIAG
+static bool diagnostic_impl (rich_location *, int, const char *,
+			     va_list *, diagnostic_t) ATTRIBUTE_GCC_DIAG(3,0);
+static bool diagnostic_n_impl (location_t, int, int, const char *,
+			       const char *, va_list *,
+			       diagnostic_t) ATTRIBUTE_GCC_DIAG(5,0);
+#endif
 static void error_recursion (diagnostic_context *) ATTRIBUTE_NORETURN;
 
 static void real_abort (void) ATTRIBUTE_NORETURN;
@@ -913,29 +920,51 @@  diagnostic_append_note (diagnostic_context *contex
   va_end (ap);
 }
 
-bool
-emit_diagnostic (diagnostic_t kind, location_t location, int opt,
-		 const char *gmsgid, ...)
+static bool
+diagnostic_impl (rich_location *richloc, int opt,
+		 const char *gmsgid,
+		 va_list *ap, diagnostic_t kind)
 {
   diagnostic_info diagnostic;
-  va_list ap;
-  bool ret;
-  rich_location richloc (line_table, location);
-
-  va_start (ap, gmsgid);
   if (kind == DK_PERMERROR)
     {
-      diagnostic_set_info (&diagnostic, gmsgid, &ap, &richloc,
+      diagnostic_set_info (&diagnostic, gmsgid, ap, richloc,
 			   permissive_error_kind (global_dc));
       diagnostic.option_index = permissive_error_option (global_dc);
     }
-  else {
-      diagnostic_set_info (&diagnostic, gmsgid, &ap, &richloc, kind);
+  else
+    {
+      diagnostic_set_info (&diagnostic, gmsgid, ap, richloc, kind);
       if (kind == DK_WARNING || kind == DK_PEDWARN)
 	diagnostic.option_index = opt;
-  }
+    }
+  return report_diagnostic (&diagnostic);
+}
 
-  ret = report_diagnostic (&diagnostic);
+static bool
+diagnostic_n_impl (location_t location, int opt, int n,
+		   const char *singular_gmsgid,
+		   const char *plural_gmsgid,
+		   va_list *ap, diagnostic_t kind)
+{
+  diagnostic_info diagnostic;
+  rich_location richloc (line_table, location);
+  diagnostic_set_info_translated (&diagnostic,
+                                  ngettext (singular_gmsgid, plural_gmsgid, n),
+                                  ap, &richloc, kind);
+  if (kind == DK_WARNING)
+    diagnostic.option_index = opt;
+  return report_diagnostic (&diagnostic);
+}
+
+bool
+emit_diagnostic (diagnostic_t kind, location_t location, int opt,
+		 const char *gmsgid, ...)
+{
+  va_list ap;
+  va_start (ap, gmsgid);
+  rich_location richloc (line_table, location);
+  bool ret = diagnostic_impl (&richloc, opt, gmsgid, &ap, kind);
   va_end (ap);
   return ret;
 }
@@ -945,13 +974,10 @@  diagnostic_append_note (diagnostic_context *contex
 void
 inform (location_t location, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
+  va_start (ap, gmsgid);
   rich_location richloc (line_table, location);
-
-  va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, &richloc, DK_NOTE);
-  report_diagnostic (&diagnostic);
+  diagnostic_impl (&richloc, -1, gmsgid, &ap, DK_NOTE);
   va_end (ap);
 }
 
@@ -959,12 +985,9 @@  inform (location_t location, const char *gmsgid, .
 void
 inform_at_rich_loc (rich_location *richloc, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
-
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, richloc, DK_NOTE);
-  report_diagnostic (&diagnostic);
+  diagnostic_impl (richloc, -1, gmsgid, &ap, DK_NOTE);
   va_end (ap);
 }
 
@@ -974,15 +997,10 @@  void
 inform_n (location_t location, int n, const char *singular_gmsgid,
           const char *plural_gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
-  rich_location richloc (line_table, location);
-
   va_start (ap, plural_gmsgid);
-  diagnostic_set_info_translated (&diagnostic,
-                                  ngettext (singular_gmsgid, plural_gmsgid, n),
-                                  &ap, &richloc, DK_NOTE);
-  report_diagnostic (&diagnostic);
+  diagnostic_n_impl (location, -1, n, singular_gmsgid, plural_gmsgid,
+		     &ap, DK_NOTE);
   va_end (ap);
 }
 
@@ -992,16 +1010,10 @@  inform_n (location_t location, int n, const char *
 bool
 warning (int opt, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
-  bool ret;
+  va_start (ap, gmsgid);
   rich_location richloc (line_table, input_location);
-
-  va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, &richloc, DK_WARNING);
-  diagnostic.option_index = opt;
-
-  ret = report_diagnostic (&diagnostic);
+  bool ret = diagnostic_impl (&richloc, opt, gmsgid, &ap, DK_WARNING);
   va_end (ap);
   return ret;
 }
@@ -1013,15 +1025,10 @@  warning (int opt, const char *gmsgid, ...)
 bool
 warning_at (location_t location, int opt, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
-  bool ret;
+  va_start (ap, gmsgid);
   rich_location richloc (line_table, location);
-
-  va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, &richloc, DK_WARNING);
-  diagnostic.option_index = opt;
-  ret = report_diagnostic (&diagnostic);
+  bool ret = diagnostic_impl (&richloc, opt, gmsgid, &ap, DK_WARNING);
   va_end (ap);
   return ret;
 }
@@ -1031,14 +1038,9 @@  warning_at (location_t location, int opt, const ch
 bool
 warning_at_rich_loc (rich_location *richloc, int opt, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
-  bool ret;
-
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, richloc, DK_WARNING);
-  diagnostic.option_index = opt;
-  ret = report_diagnostic (&diagnostic);
+  bool ret = diagnostic_impl (richloc, opt, gmsgid, &ap, DK_WARNING);
   va_end (ap);
   return ret;
 }
@@ -1051,18 +1053,11 @@  bool
 warning_n (location_t location, int opt, int n, const char *singular_gmsgid,
 	   const char *plural_gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
-  bool ret;
-  rich_location richloc (line_table, location);
-
   va_start (ap, plural_gmsgid);
-  diagnostic_set_info_translated (&diagnostic,
-                                  ngettext (singular_gmsgid, plural_gmsgid, n),
-                                  &ap, &richloc, DK_WARNING
-);
-  diagnostic.option_index = opt;
-  ret = report_diagnostic (&diagnostic);
+  bool ret = diagnostic_n_impl (location, opt, n,
+				singular_gmsgid, plural_gmsgid,
+				&ap, DK_WARNING);
   va_end (ap);
   return ret;
 }
@@ -1083,15 +1078,10 @@  warning_n (location_t location, int opt, int n, co
 bool
 pedwarn (location_t location, int opt, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
-  bool ret;
+  va_start (ap, gmsgid);
   rich_location richloc (line_table, location);
-
-  va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, &richloc,  DK_PEDWARN);
-  diagnostic.option_index = opt;
-  ret = report_diagnostic (&diagnostic);
+  bool ret = diagnostic_impl (&richloc, opt, gmsgid, &ap, DK_PEDWARN);
   va_end (ap);
   return ret;
 }
@@ -1106,16 +1096,10 @@  pedwarn (location_t location, int opt, const char
 bool
 permerror (location_t location, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
-  bool ret;
+  va_start (ap, gmsgid);
   rich_location richloc (line_table, location);
-
-  va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, &richloc,
-                       permissive_error_kind (global_dc));
-  diagnostic.option_index = permissive_error_option (global_dc);
-  ret = report_diagnostic (&diagnostic);
+  bool ret = diagnostic_impl (&richloc, -1, gmsgid, &ap, DK_PERMERROR);
   va_end (ap);
   return ret;
 }
@@ -1125,15 +1109,9 @@  permerror (location_t location, const char *gmsgid
 bool
 permerror_at_rich_loc (rich_location *richloc, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
-  bool ret;
-
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, richloc,
-                       permissive_error_kind (global_dc));
-  diagnostic.option_index = permissive_error_option (global_dc);
-  ret = report_diagnostic (&diagnostic);
+  bool ret = diagnostic_impl (richloc, -1, gmsgid, &ap, DK_PERMERROR);
   va_end (ap);
   return ret;
 }
@@ -1143,13 +1121,10 @@  permerror_at_rich_loc (rich_location *richloc, con
 void
 error (const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
+  va_start (ap, gmsgid);
   rich_location richloc (line_table, input_location);
-
-  va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, &richloc, DK_ERROR);
-  report_diagnostic (&diagnostic);
+  diagnostic_impl (&richloc, -1, gmsgid, &ap, DK_ERROR);
   va_end (ap);
 }
 
@@ -1159,29 +1134,21 @@  void
 error_n (location_t location, int n, const char *singular_gmsgid,
          const char *plural_gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
-  rich_location richloc (line_table, location);
-
   va_start (ap, plural_gmsgid);
-  diagnostic_set_info_translated (&diagnostic,
-                                  ngettext (singular_gmsgid, plural_gmsgid, n),
-                                  &ap, &richloc, DK_ERROR);
-  report_diagnostic (&diagnostic);
+  diagnostic_n_impl (location, -1, n, singular_gmsgid, plural_gmsgid,
+		     &ap, DK_ERROR);
   va_end (ap);
 }
 
-/* Same as ebove, but use location LOC instead of input_location.  */
+/* Same as above, but use location LOC instead of input_location.  */
 void
 error_at (location_t loc, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
+  va_start (ap, gmsgid);
   rich_location richloc (line_table, loc);
-
-  va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, &richloc, DK_ERROR);
-  report_diagnostic (&diagnostic);
+  diagnostic_impl (&richloc, -1, gmsgid, &ap, DK_ERROR);
   va_end (ap);
 }
 
@@ -1188,15 +1155,11 @@  error_at (location_t loc, const char *gmsgid, ...)
 /* Same as above, but use RICH_LOC.  */
 
 void
-error_at_rich_loc (rich_location *rich_loc, const char *gmsgid, ...)
+error_at_rich_loc (rich_location *richloc, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
-
   va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, rich_loc,
-		       DK_ERROR);
-  report_diagnostic (&diagnostic);
+  diagnostic_impl (richloc, -1, gmsgid, &ap, DK_ERROR);
   va_end (ap);
 }
 
@@ -1206,13 +1169,10 @@  void
 void
 sorry (const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
+  va_start (ap, gmsgid);
   rich_location richloc (line_table, input_location);
-
-  va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, &richloc, DK_SORRY);
-  report_diagnostic (&diagnostic);
+  diagnostic_impl (&richloc, -1, gmsgid, &ap, DK_SORRY);
   va_end (ap);
 }
 
@@ -1230,13 +1190,10 @@  seen_error (void)
 void
 fatal_error (location_t loc, const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
+  va_start (ap, gmsgid);
   rich_location richloc (line_table, loc);
-
-  va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, &richloc, DK_FATAL);
-  report_diagnostic (&diagnostic);
+  diagnostic_impl (&richloc, -1, gmsgid, &ap, DK_FATAL);
   va_end (ap);
 
   gcc_unreachable ();
@@ -1249,13 +1206,10 @@  fatal_error (location_t loc, const char *gmsgid, .
 void
 internal_error (const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
+  va_start (ap, gmsgid);
   rich_location richloc (line_table, input_location);
-
-  va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, &richloc, DK_ICE);
-  report_diagnostic (&diagnostic);
+  diagnostic_impl (&richloc, -1, gmsgid, &ap, DK_ICE);
   va_end (ap);
 
   gcc_unreachable ();
@@ -1267,13 +1221,10 @@  internal_error (const char *gmsgid, ...)
 void
 internal_error_no_backtrace (const char *gmsgid, ...)
 {
-  diagnostic_info diagnostic;
   va_list ap;
+  va_start (ap, gmsgid);
   rich_location richloc (line_table, input_location);
-
-  va_start (ap, gmsgid);
-  diagnostic_set_info (&diagnostic, gmsgid, &ap, &richloc, DK_ICE_NOBT);
-  report_diagnostic (&diagnostic);
+  diagnostic_impl (&richloc, -1, gmsgid, &ap, DK_ICE_NOBT);
   va_end (ap);
 
   gcc_unreachable ();