Make -Werror controllable on a per-warning-class basis

Make -Werror possible to control on a per-warning-class basis.  While
I was fixing up that code anyway, merge the handling of the -w, -W and
[warning] argument and directives.

Furthermore, make *all* warnings suppressible; any warning that isn't
categorized now belong to category "other".  However, for cleanliness
sake an "other" option does not get listed in the warning messages.

Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>

6 files changed, 221 insertions, 125 deletions

diff --git a/asm/directiv.c b/asm/directiv.c
index 1c6bfa38..faee7435 100644
--- a/asm/directiv.c
+++ b/asm/directiv.c
@@ -480,37 +480,11 @@ bool process_directives(char *directive)
     }
 
     case D_WARNING:         /* [WARNING {+|-|*}warn-name] */
-    {
-	enum warn_action { WID_OFF, WID_ON, WID_RESET };
-	enum warn_action action;
-	int i;
-
-        value = nasm_skip_spaces(value);
-        switch(*value) {
-        case '-': action = WID_OFF;   value++; break;
-        case '+': action = WID_ON;    value++; break;
-        case '*': action = WID_RESET; value++; break;
-        default:  action = WID_ON;    break;
+        if (!set_warning_status(value)) {
+            nasm_error(ERR_WARNING|ERR_WARN_UNK_WARNING,
+                       "unknown warning option: %s", value);
         }
-
-        for (i = 1; i <= ERR_WARN_MAX; i++)
-            if (!nasm_stricmp(value, warnings[i].name))
-                break;
-        if (i <= ERR_WARN_MAX) {
-            switch (action) {
-            case WID_OFF:
-                warning_on[i] = false;
-                break;
-            case WID_ON:
-                warning_on[i] = true;
-                break;
-            case WID_RESET:
-                warning_on[i] = warning_on_global[i];
-                break;
-            }
-        }
-	break;
-    }
+        break;
 
     case D_CPU:         /* [CPU] */
         cpu = get_cpu(value);
diff --git a/asm/error.c b/asm/error.c
index bfe833b8..45811a8d 100644
--- a/asm/error.c
+++ b/asm/error.c
@@ -1,5 +1,5 @@
 /* ----------------------------------------------------------------------- *
- *   
+ *
  *   Copyright 1996-2017 The NASM Authors - All Rights Reserved
  *   See the file AUTHORS included with the NASM distribution for
  *   the specific copyright holders.
@@ -14,7 +14,7 @@
  *     copyright notice, this list of conditions and the following
  *     disclaimer in the documentation and/or other materials provided
  *     with the distribution.
- *     
+ *
  *     THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
  *     CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
  *     INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
@@ -44,11 +44,10 @@
 
 /*
  * Description of the suppressible warnings for the command line and
- * the [warning] directive.  Entry zero isn't an actual warning, but
- * it used for -w+error/-Werror.
+ * the [warning] directive.
  */
-const struct warning warnings[ERR_WARN_MAX+1] = {
-    {"error", "treat warnings as errors", false},
+const struct warning warnings[ERR_WARN_ALL+1] = {
+    {"other", "any warning not specifially mentioned below", true},
     {"macro-params", "macro calls with wrong parameter count", true},
     {"macro-selfref", "cyclic macro references", false},
     {"macro-defaults", "macros with more default than optional parameters", true},
@@ -67,10 +66,15 @@ const struct warning warnings[ERR_WARN_MAX+1] = {
     {"ptr", "non-NASM keyword used in other assemblers", true},
     {"bad-pragma", "empty or malformed %pragma", false},
     {"unknown-pragma", "unknown %pragma facility or directive", false},
-    {"not-my-pragma", "%pragma not applicable to this compilation", false}
+    {"not-my-pragma", "%pragma not applicable to this compilation", false},
+    {"unknown-warning", "unknown warning in -W/-w or warning directive", false},
+
+    /* THIS ENTRY MUST COME LAST */
+    {"all", "all possible warnings", false}
 };
-bool warning_on[ERR_WARN_MAX+1];        /* Current state */
-bool warning_on_global[ERR_WARN_MAX+1]; /* Command-line state, for reset */
+
+uint8_t warning_state[ERR_WARN_ALL];/* Current state */
+uint8_t warning_state_init[ERR_WARN_ALL]; /* Command-line state, for reset */
 
 vefunc nasm_verror;    /* Global error handling function */
 
@@ -108,5 +112,91 @@ no_return nasm_panic_from_macro(const char *file, int line)
 
 no_return nasm_assert_failed(const char *file, int line, const char *msg)
 {
-    nasm_fatal(0, "assertion %s failed at %s:%d", msg, file, line);
+    nasm_panic(0, "assertion %s failed at %s:%d", msg, file, line);
+}
+
+/*
+ * This is called when processing a -w or -W option, or a warning directive.
+ * Returns true if if the action was successful.
+ */
+bool set_warning_status(const char *value)
+{
+    enum warn_action { WID_OFF, WID_ON, WID_RESET };
+    enum warn_action action;
+    uint8_t mask;
+    int i;
+    bool ok = false;
+
+    value = nasm_skip_spaces(value);
+    switch (*value) {
+    case '-':
+        action = WID_OFF;
+        value++;
+        break;
+    case '+':
+        action = WID_ON;
+        value++;
+        break;
+    case '*':
+        action = WID_RESET;
+        value++;
+        break;
+    case 'N':
+    case 'n':
+        if (!nasm_strnicmp(value, "no-", 3)) {
+            action = WID_OFF;
+            value += 3;
+            break;
+        } else if (!nasm_stricmp(value, "none")) {
+            action = WID_OFF;
+            value = NULL;
+            break;
+        }
+        /* else fall through */
+    default:
+        action = WID_ON;
+        break;
+    }
+
+    mask = WARN_ST_ENABLED;
+
+    if (value && !nasm_strnicmp(value, "error", 5)) {
+        switch (value[5]) {
+        case '=':
+            mask = WARN_ST_ERROR;
+            value += 6;
+            break;
+        case '\0':
+            mask = WARN_ST_ERROR;
+            value = NULL;
+            break;
+        default:
+            /* Just an accidental prefix? */
+            break;
+        }
+    }
+
+    if (value && !nasm_stricmp(value, "all"))
+        value = NULL;
+
+    /* This is inefficient, but it shouldn't matter... */
+    for (i = 0; i < ERR_WARN_ALL; i++) {
+        if (!value || !nasm_stricmp(value, warnings[i].name)) {
+            ok = true;          /* At least one action taken */
+            switch (action) {
+            case WID_OFF:
+                warning_state[i] &= ~mask;
+                break;
+            case WID_ON:
+                warning_state[i] |= mask;
+                break;
+            case WID_RESET:
+                warning_state[i] &= ~mask;
+                warning_state[i] |= warning_state_init[i] & mask;
+                break;
+            }
+        }
+    }
+
+    return ok;
 }
diff --git a/asm/nasm.c b/asm/nasm.c
index 62291bb8..05c51ea0 100644
--- a/asm/nasm.c
+++ b/asm/nasm.c
@@ -332,6 +332,9 @@ int main(int argc, char **argv)
         return 1;
     }
 
+    /* Save away the default state of warnings */
+    memcpy(warning_state_init, warning_state, sizeof warning_state);
+
     if (!using_debug_info) {
         /* No debug info, redirect to the null backend (empty stubs) */
         dfmt = &null_debug_form;
@@ -390,8 +393,9 @@ int main(int argc, char **argv)
 
             /* pass = 1; */
             preproc->reset(inname, 3, depend_ptr);
-            memcpy(warning_on, warning_on_global,
-		   (ERR_WARN_MAX+1) * sizeof(bool));
+
+	    /* Revert all warnings to the default state */
+	    memcpy(warning_state, warning_state_init, sizeof warning_state);
 
             while ((line = preproc->getline())) {
                 /*
@@ -624,7 +628,6 @@ static bool process_arg(char *p, char *q, int pass)
     char *param;
     int i;
     bool advance = false;
-    bool do_warn;
 
     if (!p || !p[0])
         return false;
@@ -803,24 +806,28 @@ static bool process_arg(char *p, char *q, int pass)
                  "    -X<format>  specifies error reporting format (gnu or vc)\n"
                  "    -w+foo      enables warning foo (equiv. -Wfoo)\n"
                  "    -w-foo      disable warning foo (equiv. -Wno-foo)\n\n"
-                 "    -h          show invocation summary and exit\n\n"
+                 "    -w[+-]error[=foo] can be used to promote warnings to errors\n"
+                "    -h           show invocation summary and exit\n\n"
                  "--prefix,--postfix\n"
-                 "  this options prepend or append the given argument to all\n"
-                 "  extern and global variables\n"
-                 "Warnings:\n");
-            for (i = 0; i <= ERR_WARN_MAX; i++)
-                printf("    %-23s %s (default %s)\n",
+                 "                these options prepend or append the given string\n"
+                 "                to all extern and global variables\n"
+		 "\n"
+		 "Response files should contain command line parameters,\n"
+                 "one per line.\n"
+		 "\n"
+                 "Warnings for the -W/-w options:\n");
+            for (i = 0; i <= ERR_WARN_ALL; i++)
+                printf("    %-23s %s%s\n",
                        warnings[i].name, warnings[i].help,
-                       warnings[i].enabled ? "on" : "off");
-            printf
-                ("\nresponse files should contain command line parameters"
-                 ", one per line.\n");
+		       i == ERR_WARN_ALL ? "\n" :
+                       warnings[i].enabled ? " (default on)" :
+		       " (default off)");
             if (p[2] == 'f') {
-                printf("\nvalid output formats for -f are"
+                printf("valid output formats for -f are"
                        " (`*' denotes default):\n");
                 ofmt_list(ofmt, stdout);
             } else {
-                printf("\nFor a list of valid output formats, use -hf.\n");
+                printf("For a list of valid output formats, use -hf.\n");
                 printf("For a list of debug formats, use -f <form> -y.\n");
             }
             exit(0);    /* never need usage message here */
@@ -853,48 +860,15 @@ static bool process_arg(char *p, char *q, int pass)
                 preproc = &preproc_nop;
             break;
 
-        case 'W':
-            if (pass == 2) {
-                if (param[0] == 'n' && param[1] == 'o' && param[2] == '-') {
-                    do_warn = false;
-                    param += 3;
-                } else {
-                    do_warn = true;
-                }
-                goto set_warning;
-            }
-            break;
-
         case 'w':
+        case 'W':
             if (pass == 2) {
-                if (param[0] != '+' && param[0] != '-') {
-                    nasm_error(ERR_NONFATAL | ERR_NOFILE | ERR_USAGE,
-                               "invalid option to `-w'");
-                    break;
+                if (!set_warning_status(param)) {
+                    nasm_error(ERR_WARNING|ERR_NOFILE|ERR_WARN_UNK_WARNING,
+			       "unknown warning option: %s", param);
                 }
-                do_warn = (param[0] == '+');
-                param++;
-                goto set_warning;
-            }
-            break;
-
-set_warning:
-            for (i = 0; i <= ERR_WARN_MAX; i++) {
-                if (!nasm_stricmp(param, warnings[i].name))
-                    break;
-            }
-            if (i <= ERR_WARN_MAX) {
-                warning_on_global[i] = do_warn;
-            } else if (!nasm_stricmp(param, "all")) {
-                for (i = 1; i <= ERR_WARN_MAX; i++)
-                    warning_on_global[i] = do_warn;
-            } else if (!nasm_stricmp(param, "none")) {
-                for (i = 1; i <= ERR_WARN_MAX; i++)
-                    warning_on_global[i] = !do_warn;
-            } else {
-		/* Ignore invalid warning names; forward compatibility */
             }
-            break;
+        break;
 
         case 'M':
             if (pass == 2) {
@@ -1130,8 +1104,11 @@ static void parse_cmdline(int argc, char **argv, int pass)
 
     *inname = *outname = *listname = *errname = '\0';
 
-    for (i = 0; i <= ERR_WARN_MAX; i++)
-        warning_on_global[i] = warnings[i].enabled;
+    /* Initialize all the warnings to their default state */
+    for (i = 0; i < ERR_WARN_ALL; i++) {
+        warning_state_init[i] = warning_state[i] =
+	    warnings[i].enabled ? WARN_ST_ENABLED : 0;
+    }
 
     /*
      * First, process the NASMENV environment variable.
@@ -1246,7 +1223,9 @@ static void assemble_file(char *fname, StrList **depend_ptr)
             offsets = raa_init();
         }
         preproc->reset(fname, pass1, pass1 == 2 ? depend_ptr : NULL);
-        memcpy(warning_on, warning_on_global, (ERR_WARN_MAX+1) * sizeof(bool));
+
+	/* Revert all warnings to the default state */
+	memcpy(warning_state, warning_state_init, sizeof warning_state);
 
         globallineno = 0;
         if (passn == 1)
@@ -1571,17 +1550,13 @@ static void nasm_verror_vc(int severity, const char *fmt, va_list ap)
 /*
  * check to see if this is a suppressable warning
  */
-static inline bool is_suppressable_warning(int severity)
+static inline bool is_valid_warning(int severity)
 {
-    int index;
-
     /* Not a warning at all */
     if ((severity & ERR_MASK) != ERR_WARNING)
         return false;
 
-    index = WARN_IDX(severity);
-
-    return index && index <= ERR_WARN_MAX;
+    return WARN_IDX(severity) < ERR_WARN_ALL;
 }
 
 /**
@@ -1595,8 +1570,16 @@ static inline bool is_suppressable_warning(int severity)
 static bool is_suppressed_warning(int severity)
 {
     /* Might be a warning but suppresed explicitly */
-    if (is_suppressable_warning(severity))
-         return !warning_on[WARN_IDX(severity)];
+    if (is_valid_warning(severity))
+        return !(warning_state[WARN_IDX(severity)] & WARN_ST_ENABLED);
+    else
+        return false;
+}
+
+static bool warning_is_error(int severity)
+{
+    if (is_valid_warning(severity))
+        return !!(warning_state[WARN_IDX(severity)] & WARN_ST_ERROR);
     else
         return false;
 }
@@ -1654,7 +1637,7 @@ static void nasm_verror_common(int severity, const char *fmt, va_list args)
     }
 
     vsnprintf(msg, sizeof msg - 64, fmt, args);
-    if (is_suppressable_warning(severity)) {
+    if (is_valid_warning(severity) && WARN_IDX(severity) != ERR_WARN_OTHER) {
         char *p = strchr(msg, '\0');
 	snprintf(p, 64, " [-w+%s]", warnings[WARN_IDX(severity)].name);
     }
@@ -1683,7 +1666,7 @@ static void nasm_verror_common(int severity, const char *fmt, va_list args)
         break;
     case ERR_WARNING:
         /* Treat warnings as errors */
-        if (warning_on[WARN_IDX(ERR_WARN_TERM)])
+        if (warning_is_error(severity))
             terminate_after_phase = true;
         break;
     case ERR_NONFATAL:
diff --git a/doc/changes.src b/doc/changes.src
index a24ddf21..dfc3c8ee 100644
--- a/doc/changes.src
+++ b/doc/changes.src
@@ -26,6 +26,10 @@ since 2007.
 \b \c{macho} object format now supports the \c{subsections_via_symbols}
    directive, see \k{macho-ssvs}.
 
+\b All warnings can now be suppressed if desired.  Furthermore,
+   warning-as-error can now be controlled on a per-warning-class
+   basis.
+
 \S{cl-2.12.03} Version 2.12.03
 
 \b Add new warnings for certain dangerous constructs which never ought
diff --git a/doc/nasmdoc.src b/doc/nasmdoc.src
index d2e9bd24..0df4911f 100644
--- a/doc/nasmdoc.src
+++ b/doc/nasmdoc.src
@@ -77,6 +77,8 @@
 \IR{-u} \c{-u} option
 \IR{-v} \c{-v} option
 \IR{-W} \c{-W} option
+\IR{-Werror} \c{-Werror} option
+\IR{-Wno-error} \c{-Wno-error} option
 \IR{-w} \c{-w} option
 \IR{-y} \c{-y} option
 \IR{-Z} \c{-Z} option
@@ -910,7 +912,10 @@ name, for example \c{orphan-labels}; you can enable warnings of
 this class by the command-line option \c{-w+orphan-labels} and
 disable it by \c{-w-orphan-labels}.
 
-The \i{suppressible warning} classes are:
+The current \i{warning classes} are:
+
+\b \i\c{other} specifies any warning not otherwise specified in any
+class.
 
 \b \i\c{macro-params} covers warnings about \i{multi-line macros}
 being invoked with the wrong number of parameters. This warning
@@ -969,22 +974,36 @@ to limitations in the output format.
 indicate a mistake in the source code.  Currently only the MASM
 \c{PTR} keyword is recognized.
 
-\b \i\c{error} causes warnings to be treated as errors.  Disabled by
-default.
+\b \i\c{bad-pragma} warns about a malformed or otherwise unparsable
+\c{%pragma} directive.  Disabled by default.
 
-\b \i\c{all} is an alias for \e{all} suppressible warning classes (not
-including \c{error}).  Thus, \c{-w+all} enables all available warnings.
+\b \i\c{unknown-pragma} warns about an unknown \c{%pragma} directive.
+This is not yet implemented.  Disabled by default.
 
-In addition, you can control warnings in the source code itself, using
-the \i\c{[warning]} directive.
-Warning classes may be enabled with \i\c{[warning +warning-name]},
-disabled with \i\c{[warning -warning-name]} or reset to their
-original value with \i\c{[warning *warning-name]}. No "user form"
-(without the brackets) exists.
+\b \i\c{not-my-pragma} warns about a \c{%pragma} directive which is
+not applicable to this particular assembly session.  This is not yet
+implemented.  Disabled by default.
+
+\b \i\c{unknown-warning} warns about a \c{-w} or \c{-W} option or a
+\c{[WARNING]} directive that contains an unknown warning name or is
+otherwise not possible to process.
+
+\b \i\c{all} is an alias for \e{all} suppressible warning classes.
+Thus, \c{-w+all} enables all available warnings, and \c{-w-all}
+disables warnings entirely (since NASM 2.13).
 
 Since version 2.00, NASM has also supported the gcc-like syntax
-\c{-Wwarning} and \c{-Wno-warning} instead of \c{-w+warning} and
-\c{-w-warning}, respectively.
+\c{-Wwarning-class} and \c{-Wno-warning-class} instead of \c{-w+warning-class} and
+\c{-w-warning-class}, respectively; both syntaxes work identically.
+
+The option \c{-w+error} or \i\c{-Werror} can be used to treat warnings
+as errors.  This can be controlled on a per warning class basis
+(\c{-w+error=}\e{warning-class}); if no \e{warning-class} is specified
+NASM treats it as \c{-w+error=all}; the same applies to \c{-w-error}
+or \i\c{-Wno-error}, of course.
+
+In addition, you can control warnings in the source code itself, using
+the \i\c{[WARNING]} directive.  See \k{asmdir-warning}.
 
 
 \S{opt-v} The \i\c{-v} Option: Display \i{Version} Info
@@ -4776,6 +4795,22 @@ has avoided the use of the brackeded primitive form, (\c{[FLOAT]}).
 value can be saved away and invoked later to restore the setting.
 
 
+\H{asmdir-warning} \i\c{[WARNING]}: Enable or disable warnings
+
+The \c{[WARNING]} directive can be used to enable or disable classes
+of warnings in the same way as the \c{-w} option, see \k{opt-w} for
+more details about warning classes.
+
+Warning classes may be enabled with \c{[warning +]\e{warning-class}\c{]}, disabled
+with \c{[warning -}\e{warning-class}\c{]}, or reset to their original value (as
+specified on the command line) with \c{[warning *}\e{warning-class}{]}.
+
+The \c{[WARNING]} directive also accepts the \c{all}, \c{error} and
+\c{error=}\e{warning-class} specifiers.
+
+No "user form" (without the brackets) currently exists.
+
+
 \C{outfmt} \i{Output Formats}
 
 NASM is a portable assembler, designed to be able to compile on any
diff --git a/include/error.h b/include/error.h
index 18a50073..b1e42ba9 100644
--- a/include/error.h
+++ b/include/error.h
@@ -89,7 +89,7 @@ static inline vefunc nasm_set_verror(vefunc ve)
 #define WARN(x)         ((x) << ERR_WARN_SHR)
 #define WARN_IDX(x)     (((x) & ERR_WARN_MASK) >> ERR_WARN_SHR)
 
-#define ERR_WARN_TERM           WARN( 0) /* treat warnings as errors */
+#define ERR_WARN_OTHER          WARN( 0) /* any noncategorized warning */
 #define ERR_WARN_MNP            WARN( 1) /* macro-num-parameters warning */
 #define ERR_WARN_MSR            WARN( 2) /* macro self-reference */
 #define ERR_WARN_MDP            WARN( 3) /* macro default parameters check */
@@ -110,16 +110,26 @@ static inline vefunc nasm_set_verror(vefunc ve)
 #define ERR_WARN_BAD_PRAGMA	WARN(17) /* malformed pragma */
 #define ERR_WARN_UNKNOWN_PRAGMA	WARN(18) /* unknown pragma */
 #define ERR_WARN_NOTMY_PRAGMA	WARN(19) /* pragma inapplicable */
-#define ERR_WARN_MAX            19       /* the highest numbered one */
+#define ERR_WARN_UNK_WARNING	WARN(20) /* unknown warning */
+
+/* The "all" warning acts as a global switch, it must come last */
+#define ERR_WARN_ALL            21 /* Do not use WARN() here */
 
 struct warning {
     const char *name;
     const char *help;
     bool enabled;
 };
-extern const struct warning warnings[ERR_WARN_MAX+1];
-extern bool warning_on[ERR_WARN_MAX+1];        /* Current state */
-extern bool warning_on_global[ERR_WARN_MAX+1]; /* Command-line state */
-void init_warnings(void);
+extern const struct warning warnings[ERR_WARN_ALL+1];
+
+/* This is a bitmask */
+#define WARN_ST_ENABLED      1   /* Warning is currently enabled */
+#define WARN_ST_ERROR        2   /* Treat this warning as an error */
+
+extern uint8_t warning_state[ERR_WARN_ALL];
+extern uint8_t warning_state_init[ERR_WARN_ALL];
+
+/* Process a warning option or directive */
+bool set_warning_status(const char *);
 
 #endif /* NASM_ERROR_H */