diagnostics: revamp the handling of -Werror

Recent discussions with Joel E. Denny
(http://lists.gnu.org/archive/html/bison-patches/2013-02/msg00026.html)
show that it is desirable to tell the difference between an option
that was explicitly disabled with -Wno-foo, as opposed to be left
unset.  The current framework does not allow this.

Instead of having a first int to store which options are enabled, and
another to store which are turned into errors, use an array that for
each warning category tells its status: disabled, unset, warning,
error.

* src/complain.h, src/complain.c (warning_bit): New enum.
(warnings): Use it.
(severity): New enum.
(warnings_flag): Now an array of severity.
(errors_flag): Remove, now done by warnings_flag.
(complain_init): New function, to initialie warnings_flag.
(warnings_are_errors): New Boolean, for -Werror.
* src/complain.c (warning_severity): New.
(warnings_print_categories, complains): Use it.
* src/getargs.c (warning_argmatch): Adjust to use warnings_flag.
(warnings_argmatch): Ditto.
Handle -Werror and -Wno-error here.
(getargs): Adjust.
* src/main.c (main): Call complain_init.
* tests/input.at (Invalid options): Add more corner cases.

diff --git a/src/complain.c b/src/complain.c
index 80731b723848e1f20bb1cb582650670293c9683c..2bc276e95a4717fc6ff34e32d07ca289148432dc 100644 (file)
--- a/src/complain.c
+++ b/src/complain.c
@@ -30,27 +30,62 @@
 #include "getargs.h"
 #include "quote.h"
 
 #include "getargs.h"
 #include "quote.h"
 

-warnings warnings_flag =
-  Wconflicts_sr | Wconflicts_rr | Wdeprecated | Wother;
+err_status complaint_status = status_none;

 
 

-warnings errors_flag;
+bool warnings_are_errors = false;
+severity warnings_flag[warnings_size];

 
 

-err_status complaint_status = status_none;

 static unsigned *indent_ptr = 0;
 
 static unsigned *indent_ptr = 0;
 

+void
+complain_init (void)
+{
+  warnings warnings_default =
+    Wconflicts_sr | Wconflicts_rr | Wdeprecated | Wother;
+
+  size_t b;
+  for (b = 0; b < warnings_size; ++b)
+    warnings_flag[b] = (1 << b & warnings_default
+                        ? severity_warning
+                        : severity_unset);
+}
+
+static severity
+warning_severity (warnings flags)
+{
+  if (flags & fatal)
+    return severity_fatal;
+  else if (flags & complaint)
+    return severity_error;
+  else
+    {
+      severity res = severity_disabled;
+      size_t b;
+      for (b = 0; b < warnings_size; ++b)
+        if (flags & 1 << b)
+          res = res < warnings_flag[b] ? warnings_flag[b] : res;
+      if (res == severity_warning && warnings_are_errors)
+        res = severity_error;
+      return res;
+    }
+}
+
+

 /** Display a "[-Wyacc]" like message on \a f.  */
 
 static void
 warnings_print_categories (warnings warn_flags, FILE *f)
 {
   /* Display only the first match, the second is "-Wall".  */
 /** Display a "[-Wyacc]" like message on \a f.  */
 
 static void
 warnings_print_categories (warnings warn_flags, FILE *f)
 {
   /* Display only the first match, the second is "-Wall".  */

-  int i;
+  size_t i;

   for (i = 0; warnings_args[i]; ++i)
     if (warn_flags & warnings_types[i])
       {
   for (i = 0; warnings_args[i]; ++i)
     if (warn_flags & warnings_types[i])
       {

-        bool err = warn_flags & errors_flag;
-        fprintf (f, " [-W%s%s]", err ? "error=" : "" , warnings_args[i]);
-        break;
+        severity s = warning_severity (warnings_types[i]);
+        fprintf (f, " [-W%s%s]",
+                 s == severity_error ? "error=" : "",
+                 warnings_args[i]);
+        return;

       }
 }
 
       }
 }
 


@@ -109,23 +144,28 @@ error_message (const location *loc, warnings flags, const char *prefix,
   fflush (stderr);
 }
 
   fflush (stderr);
 }
 

-/** Raise a complaint. That can be a fatal error, a complaint or just a
+/** Raise a complaint. That can be a fatal error, an error or just a

     warning.  */
     warning.  */

-static inline void
+
+static void

 complains (const location *loc, warnings flags, const char *message,
            va_list args)
 {
 complains (const location *loc, warnings flags, const char *message,
            va_list args)
 {

-  const char* prefix =
-    flags & fatal ? _("fatal error")
-    : flags & (errors_flag | complaint) ? _("error")
-    : _("warning");
-
+  severity s = warning_severity (flags);

   if ((flags & complaint) && complaint_status < status_complaint)
     complaint_status = status_complaint;
   if ((flags & complaint) && complaint_status < status_complaint)
     complaint_status = status_complaint;

-  else if ((flags & (warnings_flag & errors_flag)) && ! complaint_status)
-    complaint_status = status_warning_as_error;
-  if (flags & (warnings_flag | fatal | complaint))
-    error_message (loc, flags, prefix, message, args);
+
+  if (severity_warning <= s)
+    {
+      const char* prefix =
+        s == severity_fatal ? _("fatal error")
+        : s == severity_error ? _("error")
+        : _("warning");
+      if (severity_error <= s && ! complaint_status)
+        complaint_status = status_warning_as_error;
+      error_message (loc, flags, prefix, message, args);
+    }
+

   if (flags & fatal)
     exit (EXIT_FAILURE);
 }
   if (flags & fatal)
     exit (EXIT_FAILURE);
 }


@@ -176,7 +216,6 @@ complain_args (location const *loc, warnings w, unsigned *indent,
     complain (loc, fatal, "too many arguments for complains");
     break;
   }
     complain (loc, fatal, "too many arguments for complains");
     break;
   }

-

 }
 
 void
 }
 
 void

diff --git a/src/complain.h b/src/complain.h
index 306d2e8746fb901c64496e6348a0db2d8360dfca..c7f93e1e9c7e2bd33a8df7cce55f9aabdf98492b 100644 (file)
--- a/src/complain.h
+++ b/src/complain.h
@@ -28,17 +28,32 @@
 | --warnings.  |
 `-------------*/
 
 | --warnings.  |
 `-------------*/
 

+/** The bits assigned to each warning type.  */

 typedef enum
   {
 typedef enum
   {

-    Wnone             = 0,       /**< Issue no warnings.  */
-    Wmidrule_values   = 1 << 0,  /**< Unset or unused midrule values.  */
-    Wyacc             = 1 << 1,  /**< POSIXME.  */
-    Wconflicts_sr     = 1 << 2,  /**< S/R conflicts.  */
-    Wconflicts_rr     = 1 << 3,  /**< R/R conflicts.  */
-    Wdeprecated       = 1 << 4,  /**< Obsolete constructs.  */
-    Wprecedence       = 1 << 5,  /**< Useless precedence and associativity.  */
+    warning_midrule_values, /**< Unset or unused midrule values.  */
+    warning_yacc,           /**< POSIXME.  */
+    warning_conflicts_sr,   /**< S/R conflicts.  */
+    warning_conflicts_rr,   /**< R/R conflicts.  */
+    warning_deprecated,     /**< Obsolete constructs.  */
+    warning_precedence,     /**< Useless precedence and associativity.  */
+    warning_other,          /**< All other warnings.  */

 
 

-    Wother            = 1 << 6,  /**< All other warnings.  */
+    warnings_size           /**< The number of warnings.  Must be last.  */
+  } warning_bit;
+
+typedef enum
+  {
+    /**< Issue no warnings.  */
+    Wnone             = 0,
+
+    Wmidrule_values   = 1 << warning_midrule_values,
+    Wyacc             = 1 << warning_yacc,
+    Wconflicts_sr     = 1 << warning_conflicts_sr,
+    Wconflicts_rr     = 1 << warning_conflicts_rr,
+    Wdeprecated       = 1 << warning_deprecated,
+    Wprecedence       = 1 << warning_precedence,
+    Wother            = 1 << warning_other,

 
     Werror            = 1 << 10, /** This bit is no longer used. */
 
 
     Werror            = 1 << 10, /** This bit is no longer used. */
 


@@ -51,11 +66,25 @@ typedef enum
     Wall              = ~complaint & ~fatal & ~silent
   } warnings;
 
     Wall              = ~complaint & ~fatal & ~silent
   } warnings;
 

-/** What warnings are issued.  */
-extern warnings warnings_flag;

 
 

-/** What warnings are made errors.  */
-extern warnings errors_flag;
+/** For each warning type, its severity.  */
+typedef enum
+  {
+    severity_disabled = 0,
+    severity_unset = 1,
+    severity_warning = 2,
+    severity_error = 3,
+    severity_fatal = 4
+  } severity;
+
+/** Whether -Werror was set. */
+extern bool warnings_are_errors;
+
+/** For each warning type, its severity.  */
+extern severity warnings_flag[];
+
+/** Initialize this module.  */
+void complain_init (void);

 
 /** Make a complaint, with maybe a location.  */
 void complain (location const *loc, warnings flags, char const *message, ...)
 
 /** Make a complaint, with maybe a location.  */
 void complain (location const *loc, warnings flags, char const *message, ...)

diff --git a/src/getargs.c b/src/getargs.c
index cebfd783a8997b871bf8438834eb4eb9369118a5..844036ee8c2551153aa7863e3b4389bd41f19eda 100644 (file)
--- a/src/getargs.c
+++ b/src/getargs.c
@@ -229,48 +229,55 @@ ARGMATCH_VERIFY (trace_args, trace_types);
 
 /** Decode a single argument from -W.
  *
 
 /** Decode a single argument from -W.
  *

- *  \param flags    the flags to update

  *  \param arg      the subarguments to decode.
  *                  If null, then activate all the flags.
  *  \param no       length of the potential "no-" prefix.
  *                  Can be 0 or 3. If 3, negate the action of the subargument.
  *  \param err      length of a potential "error=".
  *  \param arg      the subarguments to decode.
  *                  If null, then activate all the flags.
  *  \param no       length of the potential "no-" prefix.
  *                  Can be 0 or 3. If 3, negate the action of the subargument.
  *  \param err      length of a potential "error=".

- *                  Can be 0 or 5. If 5, treat the subargument as a CATEGORY.
+ *                  Can be 0 or 6. If 6, treat the subargument as a CATEGORY.

  *
  *  If VALUE != 0 then KEY sets flags and no-KEY clears them.
  *  If VALUE == 0 then KEY clears all flags from \c all and no-KEY sets all
  *  flags from \c all.  Thus no-none = all and no-all = none.
  */
 static void
  *
  *  If VALUE != 0 then KEY sets flags and no-KEY clears them.
  *  If VALUE == 0 then KEY clears all flags from \c all and no-KEY sets all
  *  flags from \c all.  Thus no-none = all and no-all = none.
  */
 static void

-warning_argmatch (int *flags, char *arg, size_t no, size_t err)
+warning_argmatch (char const *arg, size_t no, size_t err)

 {
 {

-  int value = 0;
-  if (!err || arg[no + err++] != '\0')
-    value = XARGMATCH ("--warning", arg + no + err,
-                       warnings_args, warnings_types);
+  int value = XARGMATCH ("--warning", arg + no + err,
+                         warnings_args, warnings_types);

 
 

-  if (value)
+  /* -Wnone == -Wno-all, and -Wno-none == -Wall.  */
+  if (!value)

     {
     {

-      if (no)
-        *flags &= ~value;
-      else
-        {
-          if (err)
-            warnings_flag |= value;
-          *flags |= value;
-        }
+      value = Wall;
+      no = !no;
+    }
+
+  if (no)
+    {
+      size_t b;
+      for (b = 0; b < warnings_size; ++b)
+        if (value & 1 << b)
+          {
+            if (err)
+              {
+                /* -Wno-error=foo: if foo enabled as an error,
+                   make it a warning.  */
+                if (warnings_flag[b] == severity_error)
+                  warnings_flag[b] = severity_warning;
+              }
+            else
+              /* -Wno-foo.  */
+              warnings_flag[b] = severity_disabled;
+          }

     }
   else
     {
     }
   else
     {

-      /* With a simpler 'if (no)' version, -Werror means -Werror=all
-         (or rather, -Werror=no-none, but that syntax is invalid).
-         The difference is:
-         - Werror activates all errors, but not the warnings
-         - Werror=all activates errors, and all warnings */
-      if (no ? !err : err)
-        *flags |= Wall;
-      else
-        *flags &= ~Wall;
+      size_t b;
+      for (b = 0; b < warnings_size; ++b)
+        if (value & 1 << b)
+          /* -Wfoo and -Werror=foo. */
+          warnings_flag[b] = err ? severity_error : severity_warning;

     }
 }
 
     }
 }
 


@@ -284,15 +291,22 @@ warnings_argmatch (char *args)
 {
   if (args)
     for (args = strtok (args, ","); args; args = strtok (NULL, ","))
 {
   if (args)
     for (args = strtok (args, ","); args; args = strtok (NULL, ","))

-      {
-        size_t no = STRPREFIX_LIT ("no-", args) ? 3 : 0;
-        size_t err = STRPREFIX_LIT ("error", args + no) ? 5 : 0;
+      if (STREQ (args, "error"))
+        warnings_are_errors = true;
+      else if (STREQ (args, "no-error"))
+        {
+          warnings_are_errors = false;
+          warning_argmatch ("no-error=all", 3, 6);
+        }
+      else
+        {
+          size_t no = STRPREFIX_LIT ("no-", args) ? 3 : 0;
+          size_t err = STRPREFIX_LIT ("error=", args + no) ? 6 : 0;

 
 

-        warning_argmatch (err ? &errors_flag : &warnings_flag,
-                          args, no, err);
-      }
+          warning_argmatch (args, no, err);
+        }

   else
   else

-    warnings_flag |= Wall;
+    warning_argmatch ("all", 0, 0);

 }
 
 const char * const warnings_args[] =
 }
 
 const char * const warnings_args[] =


@@ -790,8 +804,7 @@ getargs (int argc, char *argv[])
         break;
 
       case 'y':
         break;
 
       case 'y':

-        warnings_flag |= Wyacc;
-        errors_flag |= Wyacc;
+        warnings_flag[warning_yacc] = severity_error;

         yacc_flag = true;
         break;
 
         yacc_flag = true;
         break;
 

diff --git a/src/main.c b/src/main.c
index 22703d1dd67eefb441294ba3d2cfa89d8fb9857b..aa8c7bfa058793bdf5747761334348fb6fc0a2f5 100644 (file)
--- a/src/main.c
+++ b/src/main.c
@@ -76,6 +76,7 @@ main (int argc, char *argv[])
 
   uniqstrs_new ();
   muscle_init ();
 
   uniqstrs_new ();
   muscle_init ();

+  complain_init ();

 
   getargs (argc, argv);
 
 
   getargs (argc, argv);
 

diff --git a/tests/input.at b/tests/input.at
index f2c1fcb330d1467d80507de0ae080a7f445231a7..382532c60091d4bc372fc67435fabc6d672561bd 100644 (file)
--- a/tests/input.at
+++ b/tests/input.at
@@ -36,6 +36,10 @@ exp: '0'
 AT_BISON_CHECK([-ferror=caret input.y], [1], [], [ignore])
 AT_BISON_CHECK([--report=error=itemsets input.y], [1], [], [ignore])
 
 AT_BISON_CHECK([-ferror=caret input.y], [1], [], [ignore])
 AT_BISON_CHECK([--report=error=itemsets input.y], [1], [], [ignore])
 

+# We used to accept any character after "-Werror", instead of ensuring
+# this is "=".
+AT_BISON_CHECK([-Werror?all input.y], [1], [], [ignore])
+

 AT_CLEANUP
 
 
 AT_CLEANUP