From 808e523db40335778897b413d446dff1d41d0e85 Mon Sep 17 00:00:00 2001 From: Akim Demaille Date: Thu, 14 Feb 2013 09:25:36 +0100 Subject: [PATCH] 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. --- src/complain.c | 79 +++++++++++++++++++++++++++++++++++------------- src/complain.h | 53 +++++++++++++++++++++++++-------- src/getargs.c | 81 +++++++++++++++++++++++++++++--------------------- src/main.c | 1 + tests/input.at | 4 +++ 5 files changed, 152 insertions(+), 66 deletions(-) diff --git a/src/complain.c b/src/complain.c index 80731b72..2bc276e9 100644 --- a/src/complain.c +++ b/src/complain.c @@ -30,27 +30,62 @@ #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; +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". */ - int i; + size_t 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); } -/** 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. */ -static inline void + +static void 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; - 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); } @@ -176,7 +216,6 @@ complain_args (location const *loc, warnings w, unsigned *indent, complain (loc, fatal, "too many arguments for complains"); break; } - } void diff --git a/src/complain.h b/src/complain.h index 306d2e87..c7f93e1e 100644 --- a/src/complain.h +++ b/src/complain.h @@ -28,17 +28,32 @@ | --warnings. | `-------------*/ +/** The bits assigned to each warning type. */ 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. */ @@ -51,11 +66,25 @@ typedef enum 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, ...) diff --git a/src/getargs.c b/src/getargs.c index cebfd783..844036ee 100644 --- a/src/getargs.c +++ b/src/getargs.c @@ -229,48 +229,55 @@ ARGMATCH_VERIFY (trace_args, trace_types); /** 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=". - * 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 -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 { - /* 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, ",")) - { - 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 - warnings_flag |= Wall; + warning_argmatch ("all", 0, 0); } const char * const warnings_args[] = @@ -790,8 +804,7 @@ getargs (int argc, char *argv[]) break; case 'y': - warnings_flag |= Wyacc; - errors_flag |= Wyacc; + warnings_flag[warning_yacc] = severity_error; yacc_flag = true; break; diff --git a/src/main.c b/src/main.c index 22703d1d..aa8c7bfa 100644 --- a/src/main.c +++ b/src/main.c @@ -76,6 +76,7 @@ main (int argc, char *argv[]) uniqstrs_new (); muscle_init (); + complain_init (); getargs (argc, argv); diff --git a/tests/input.at b/tests/input.at index f2c1fcb3..382532c6 100644 --- 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]) +# We used to accept any character after "-Werror", instead of ensuring +# this is "=". +AT_BISON_CHECK([-Werror?all input.y], [1], [], [ignore]) + AT_CLEANUP -- 2.45.2