warnings; move WARN_OTHER to the end; allow non-warnings to be suppressed

Putting WARN_OTHER at the end of the list creates a number of
advantages and simplifications:

1. It is more user friendly! It is far more of a logical location for
   the default case to be at the end of the printed list.
2. The value 0 can be used in a number of places to indicate a
   non-suppressible event. By having warning_state[0] always contain
   WARN_ST_ENABLED, we can always do the table lookup, even.
3. It means non-warnings (except fatal/panic) can now be conditioned
   on warning states. In those cases, WARN_*, including WARN_OTHER,
   can be added to the mask for any category. This is especially
   useful for notes.

The only downside is that we have to explicitly detect the case where
we have ERR_WARNING but no WARN_ flag. This is a trivial test.

Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>

3 files changed, 102 insertions, 92 deletions

diff --git a/asm/error.c b/asm/error.c
index c61ee390..e5e0a1cc 100644
--- a/asm/error.c
+++ b/asm/error.c
@@ -47,7 +47,7 @@
  * the [warning] directive.
  */
 const struct warning warnings[WARN_ALL+1] = {
-    {"other", "any warning not specifially mentioned below", true},
+    {NULL, NULL, true},	/* must be true - used for unconditional enable */
     {"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},
@@ -71,7 +71,8 @@ const struct warning warnings[WARN_ALL+1] = {
     {"negative-rep", "regative %rep count", true},
     {"phase", "phase error during stabilization", false},
 
-    /* THIS ENTRY MUST COME LAST */
+    /* THESE ENTRIES SHOULD COME LAST */
+    {"other", "any warning not specifially mentioned below", true},
     {"all", "all possible warnings", false}
 };
 
@@ -182,7 +183,7 @@ bool set_warning_status(const char *value)
         value = NULL;
 
     /* This is inefficient, but it shouldn't matter... */
-    for (i = 0; i < WARN_ALL; i++) {
+    for (i = 1; i < WARN_ALL; i++) {
         if (!value || !nasm_stricmp(value, warnings[i].name)) {
             ok = true;          /* At least one action taken */
             switch (action) {
diff --git a/asm/nasm.c b/asm/nasm.c
index 42d3c6c6..bf76fb97 100644
--- a/asm/nasm.c
+++ b/asm/nasm.c
@@ -76,7 +76,6 @@ struct forwrefinfo {            /* info held on forward refs. */
 
 static void parse_cmdline(int, char **, int);
 static void assemble_file(const char *, StrList **);
-static bool is_suppressed_warning(int severity);
 static bool skip_this_pass(int severity);
 static void nasm_verror_gnu(int severity, const char *fmt, va_list args);
 static void nasm_verror_vc(int severity, const char *fmt, va_list args);
@@ -1343,7 +1342,10 @@ static void parse_cmdline(int argc, char **argv, int pass)
     char *envreal, *envcopy = NULL, *p;
     int i;
 
-    /* Initialize all the warnings to their default state */
+    /*
+     * Initialize all the warnings to their default state, including
+     * warning index 0 used for "always on".
+     */
     for (i = 0; i < WARN_ALL; i++) {
         warning_state_init[i] = warning_state[i] =
 	    warnings[i].enabled ? WARN_ST_ENABLED : 0;
@@ -1723,6 +1725,70 @@ static void assemble_file(const char *fname, StrList **depend_ptr)
 }
 
 /**
+ * get warning index; 0 if this is non-suppressible.
+ */
+static size_t warn_index(int severity)
+{
+    size_t index;
+
+    if ((severity & ERR_MASK) >= ERR_FATAL)
+        return 0;               /* Fatal errors are never suppressible */
+
+    /* If this is a warning and no index is provided, it is WARN_OTHER */
+    if ((severity & (ERR_MASK|WARN_MASK)) == ERR_WARNING)
+        severity |= WARN_OTHER;
+
+    index = WARN_IDX(severity);
+    nasm_assert(index < WARN_ALL);
+
+    return index;
+}
+
+static bool skip_this_pass(int severity)
+{
+    /*
+     * See if it's a pass-specific error or warning which should be skipped.
+     * We can never skip fatal errors as by definition they cannot be
+     * resumed from.
+     */
+    if ((severity & ERR_MASK) >= ERR_FATAL)
+	return false;
+
+    /*
+     * passn is 1 on the very first pass only.
+     * pass0 is 2 on the code-generation (final) pass only.
+     * These are the passes we care about in this case.
+     */
+    return (((severity & ERR_PASS1) && passn != 1) ||
+	    ((severity & ERR_PASS2) && pass0 != 2));
+}
+
+/**
+ * check for suppressed message (usually warnings or notes)
+ *
+ * @param severity the severity of the warning or error
+ * @return true if we should abort error/warning printing
+ */
+static bool is_suppressed(int severity)
+{
+    return !(warning_state[warn_index(severity)] & WARN_ST_ENABLED);
+}
+
+/**
+ * check if we have a warning that should be promoted to an error
+ *
+ * @param severity the severity of the warning or error
+ * @return true if we should promote to error
+ */
+static bool warning_is_error(int severity)
+{
+    if ((severity & ERR_MASK) != ERR_WARNING)
+        return false;           /* Other message types  */
+
+    return !!(warning_state[warn_index(severity)] & WARN_ST_ERROR);
+}
+
+/**
  * gnu style error reporting
  * This function prints an error message to error_file in the
  * style used by GNU. An example would be:
@@ -1741,7 +1807,7 @@ static void nasm_verror_gnu(int severity, const char *fmt, va_list ap)
     const char *currentfile = NULL;
     int32_t lineno = 0;
 
-    if (is_suppressed_warning(severity))
+    if (is_suppressed(severity))
         return;
 
     if (!(severity & ERR_NOFILE)) {
@@ -1782,7 +1848,7 @@ static void nasm_verror_vc(int severity, const char *fmt, va_list ap)
     const char *currentfile = NULL;
     int32_t lineno = 0;
 
-    if (is_suppressed_warning(severity))
+    if (is_suppressed(severity))
         return;
 
     if (!(severity & ERR_NOFILE))
@@ -1799,62 +1865,6 @@ static void nasm_verror_vc(int severity, const char *fmt, va_list ap)
     nasm_verror_common(severity, fmt, ap);
 }
 
-/*
- * check to see if this is a suppressable warning
- */
-static inline bool is_valid_warning(int severity)
-{
-    /* Not a warning at all */
-    if ((severity & ERR_MASK) != ERR_WARNING)
-        return false;
-
-    return WARN_IDX(severity) < WARN_ALL;
-}
-
-/**
- * check for suppressed warning
- * checks for suppressed warning or pass one only warning and we're
- * not in pass 1
- *
- * @param severity the severity of the warning or error
- * @return true if we should abort error/warning printing
- */
-static bool is_suppressed_warning(int severity)
-{
-    /* Might be a warning but suppresed explicitly */
-    if (is_valid_warning(severity) && !(severity & ERR_USAGE))
-        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;
-}
-
-static bool skip_this_pass(int severity)
-{
-    /*
-     * See if it's a pass-specific error or warning which should be skipped.
-     * We cannot skip errors stronger than ERR_NONFATAL as by definition
-     * they cannot be resumed from.
-     */
-    if ((severity & ERR_MASK) > ERR_NONFATAL)
-	return false;
-
-    /*
-     * passn is 1 on the very first pass only.
-     * pass0 is 2 on the code-generation (final) pass only.
-     * These are the passes we care about in this case.
-     */
-    return (((severity & ERR_PASS1) && passn != 1) ||
-	    ((severity & ERR_PASS2) && pass0 != 2));
-}
-
 /**
  * common error reporting
  * This is the common back end of the error reporting schemes currently
@@ -1870,7 +1880,6 @@ static void nasm_verror_common(int severity, const char *fmt, va_list args)
     char msg[1024];
     const char *pfx;
     bool warn_is_err = warning_is_error(severity);
-    bool warn_is_other = WARN_IDX(severity) == WARN_OTHER;
 
     switch (severity & (ERR_MASK|ERR_NO_SEVERITY)) {
     case ERR_NOTE:
@@ -1900,11 +1909,11 @@ static void nasm_verror_common(int severity, const char *fmt, va_list args)
     }
 
     vsnprintf(msg, sizeof msg - 64, fmt, args);
-    if (is_valid_warning(severity) && (warn_is_err || !warn_is_other)) {
+    if ((severity & ERR_MASK) == ERR_WARNING && !is_suppressed(severity)) {
         char *p = strchr(msg, '\0');
 	snprintf(p, 64, " [-w+%s%s]",
                  warn_is_err ? "error=" : "",
-                 warnings[WARN_IDX(severity)].name);
+                 warnings[warn_index(severity)].name);
     }
 
     if (!skip_this_pass(severity))
@@ -2044,14 +2053,13 @@ static void help(const char xopt)
         }
     }
 
-    printf("\nWarnings for the -W/-w options:\n");
+    printf("\nWarnings for the -W/-w options: (default in brackets)\n");
 
-    for (i = 0; i <= WARN_ALL; i++)
+    for (i = 1; i <= WARN_ALL; i++)
         printf("    %-23s %s%s\n",
                warnings[i].name, warnings[i].help,
                i == WARN_ALL ? "\n" :
-               warnings[i].enabled ? " (default on)" :
-               " (default off)");
+               warnings[i].enabled ? " [on]" : " [off]");
 
     if (xopt == 'f') {
         printf("valid output formats for -f are"
diff --git a/include/error.h b/include/error.h
index 63ec12b6..1d904849 100644
--- a/include/error.h
+++ b/include/error.h
@@ -83,41 +83,42 @@ static inline vefunc nasm_set_verror(vefunc ve)
 
 /*
  * These codes define specific types of suppressible warning.
+ * They are assumed to occupy the most significant bits of the
+ * severity code.
  */
 
-#define WARN_MASK   0xFFFFF000      /* the mask for this feature */
-#define WARN_SHR    12              /* how far to shift right */
+#define WARN_SHR		12              /* how far to shift right */
+#define WARN(x)         	((x) << WARN_SHR)
+#define WARN_MASK		WARN(~0)
+#define WARN_IDX(x)     	((x) >> WARN_SHR)
 
-#define WARN(x)         ((x) << WARN_SHR)
-#define WARN_IDX(x)     (((x) & WARN_MASK) >> WARN_SHR)
-
-#define WARN_OTHER          WARN( 0) /* any noncategorized warning */
-#define WARN_MNP            WARN( 1) /* macro-num-parameters warning */
-#define WARN_MSR            WARN( 2) /* macro self-reference */
-#define WARN_MDP            WARN( 3) /* macro default parameters check */
-#define WARN_OL             WARN( 4) /* orphan label (no colon, and
+#define WARN_MNP            	WARN( 1) /* macro-num-parameters warning */
+#define WARN_MSR            	WARN( 2) /* macro self-reference */
+#define WARN_MDP            	WARN( 3) /* macro default parameters check */
+#define WARN_OL             	WARN( 4) /* orphan label (no colon, and
                                           * alone on line) */
-#define WARN_NOV            WARN( 5) /* numeric overflow */
-#define WARN_GNUELF         WARN( 6) /* using GNU ELF extensions */
-#define WARN_FL_OVERFLOW    WARN( 7) /* FP overflow */
-#define WARN_FL_DENORM      WARN( 8) /* FP denormal */
-#define WARN_FL_UNDERFLOW   WARN( 9) /* FP underflow */
-#define WARN_FL_TOOLONG     WARN(10) /* FP too many digits */
-#define WARN_USER           WARN(11) /* %warning directives */
+#define WARN_NOV            	WARN( 5) /* numeric overflow */
+#define WARN_GNUELF         	WARN( 6) /* using GNU ELF extensions */
+#define WARN_FL_OVERFLOW    	WARN( 7) /* FP overflow */
+#define WARN_FL_DENORM      	WARN( 8) /* FP denormal */
+#define WARN_FL_UNDERFLOW   	WARN( 9) /* FP underflow */
+#define WARN_FL_TOOLONG     	WARN(10) /* FP too many digits */
+#define WARN_USER           	WARN(11) /* %warning directives */
 #define WARN_LOCK		WARN(12) /* bad LOCK prefixes */
 #define WARN_HLE		WARN(13) /* bad HLE prefixes */
 #define WARN_BND		WARN(14) /* bad BND prefixes */
-#define WARN_ZEXTRELOC	WARN(15) /* relocation zero-extended */
+#define WARN_ZEXTRELOC		WARN(15) /* relocation zero-extended */
 #define WARN_PTR		WARN(16) /* not a NASM keyword */
-#define WARN_BAD_PRAGMA	WARN(17) /* malformed pragma */
+#define WARN_BAD_PRAGMA		WARN(17) /* malformed pragma */
 #define WARN_UNKNOWN_PRAGMA	WARN(18) /* unknown pragma */
 #define WARN_NOTMY_PRAGMA	WARN(19) /* pragma inapplicable */
 #define WARN_UNK_WARNING	WARN(20) /* unknown warning */
-#define WARN_NEG_REP	WARN(21) /* negative repeat count */
+#define WARN_NEG_REP		WARN(21) /* negative repeat count */
 #define WARN_PHASE		WARN(22) /* phase error in pass 1 */
 
-/* The "all" warning acts as a global switch, it must come last */
-#define WARN_ALL            23 /* Do not use WARN() here */
+/* These two should come last */
+#define WARN_ALL		(22+2) /* Do not use WARN() here */
+#define WARN_OTHER		WARN(WARN_ALL-1) /* any noncategorized warning */
 
 struct warning {
     const char *name;