Refactoring and enhancements to RGBASM warnings (#1526)

* Allow a `no-` prefix to negate "meta" warnings
  (`-Wno-all`, `-Wno-extra`, `-Wno-everything`)
* Allow `-Wno-error=...` to override `-Werror`
  (including for "meta" warnings)
This commit is contained in:
Sylvie
2024-10-04 15:52:40 -04:00
committed by GitHub
parent a9e49a09fd
commit cf85146353
5 changed files with 234 additions and 289 deletions

View File

@@ -5,8 +5,6 @@
extern unsigned int nbErrors, maxErrors;
enum WarningState { WARNING_DEFAULT, WARNING_DISABLED, WARNING_ENABLED, WARNING_ERROR };
enum WarningID {
WARNING_ASSERT, // Assertions
WARNING_BACKWARDS_FOR, // `for` loop with backwards range
@@ -26,10 +24,9 @@ enum WarningID {
NB_PLAIN_WARNINGS,
// Warnings past this point are "parametric" warnings, only mapping to a single flag
#define PARAM_WARNINGS_START NB_PLAIN_WARNINGS
// Warnings past this point are "parametric" warnings, only mapping to a single flag
// Treating string as number may lose some bits
WARNING_NUMERIC_STRING_1 = PARAM_WARNINGS_START,
WARNING_NUMERIC_STRING_1 = NB_PLAIN_WARNINGS,
WARNING_NUMERIC_STRING_2,
// Purging an exported symbol or label
WARNING_PURGE_1,
@@ -41,20 +38,24 @@ enum WarningID {
WARNING_UNMAPPED_CHAR_1,
WARNING_UNMAPPED_CHAR_2,
NB_PLAIN_AND_PARAM_WARNINGS,
#define NB_PARAM_WARNINGS (NB_PLAIN_AND_PARAM_WARNINGS - PARAM_WARNINGS_START)
// Warnings past this point are "meta" warnings
#define META_WARNINGS_START NB_PLAIN_AND_PARAM_WARNINGS
WARNING_ALL = META_WARNINGS_START,
WARNING_EXTRA,
WARNING_EVERYTHING,
NB_WARNINGS,
#define NB_META_WARNINGS (NB_WARNINGS - META_WARNINGS_START)
};
extern WarningState warningStates[NB_PLAIN_AND_PARAM_WARNINGS];
enum WarningAbled { WARNING_DEFAULT, WARNING_ENABLED, WARNING_DISABLED };
struct WarningState {
WarningAbled state;
WarningAbled error;
void update(WarningState other);
};
struct Diagnostics {
WarningState flagStates[NB_WARNINGS];
WarningState metaStates[NB_WARNINGS];
};
extern Diagnostics warningStates;
extern bool warningsAreErrors;
void processWarningFlag(char const *flag);

View File

@@ -212,13 +212,19 @@ The following options alter the way warnings are processed.
.Bl -tag -width Ds
.It Fl Werror
Make all warnings into errors.
This can be negated as
.Fl Wno-error
to prevent turning all warnings into errors.
.It Fl Werror=
Make the specified warning into an error.
Make the specified warning or meta warning into an error.
A warning's name is appended
.Pq example: Fl Werror=obsolete ,
and this warning is implicitly enabled and turned into an error.
This is an error if used with a meta warning, such as
.Fl Werror=all .
This can be negated as
.Fl Wno-error=
to prevent turning a specified warning into an error, even if
.Fl Werror
is in effect.
.El
.Pp
The following warnings are
@@ -240,6 +246,10 @@ Note that each of these flag also has a negation (for example,
.Fl Wcharmap-redef
enables the warning that
.Fl Wno-charmap-redef
disables; and
.Fl Wall
enables every warning that
.Fl Wno-all
disables).
Only the non-default flag is listed here.
Ignoring the

View File

@@ -293,7 +293,7 @@ int main(int argc, char *argv[]) {
break;
case 'W':
processWarningFlag(musl_optarg);
opt_W(musl_optarg);
break;
case 'w':

View File

@@ -13,8 +13,6 @@
#include "asm/section.hpp"
#include "asm/warning.hpp"
static constexpr size_t numWarningStates = sizeof(warningStates);
struct OptStackEntry {
char binary[2];
char gbgfx[4];
@@ -22,7 +20,7 @@ struct OptStackEntry {
uint8_t fillByte;
bool warningsAreErrors;
size_t maxRecursionDepth;
WarningState warningStates[numWarningStates];
Diagnostics warningStates;
};
static std::stack<OptStackEntry> stack;
@@ -160,7 +158,7 @@ void opt_Push() {
// Both of these pulled from warning.hpp
entry.warningsAreErrors = warningsAreErrors;
memcpy(entry.warningStates, warningStates, numWarningStates);
entry.warningStates = warningStates;
entry.maxRecursionDepth = maxRecursionDepth; // Pulled from fstack.h
@@ -184,5 +182,5 @@ void opt_Pop() {
// opt_W does not apply a whole warning state; it processes one flag string
warningsAreErrors = entry.warningsAreErrors;
memcpy(warningStates, entry.warningStates, numWarningStates);
warningStates = entry.warningStates;
}

View File

@@ -10,7 +10,7 @@
#include <string.h>
#include "error.hpp"
#include "helpers.hpp" // QUOTEDSTRLEN
#include "helpers.hpp"
#include "itertools.hpp"
#include "asm/fstack.hpp"
@@ -20,266 +20,163 @@
unsigned int nbErrors = 0;
unsigned int maxErrors = 0;
static WarningState const defaultWarnings[ARRAY_SIZE(warningStates)] = {
WARNING_ENABLED, // WARNING_ASSERT
WARNING_DISABLED, // WARNING_BACKWARDS_FOR
WARNING_DISABLED, // WARNING_BUILTIN_ARG
WARNING_DISABLED, // WARNING_CHARMAP_REDEF
WARNING_DISABLED, // WARNING_DIV
WARNING_DISABLED, // WARNING_EMPTY_DATA_DIRECTIVE
WARNING_DISABLED, // WARNING_EMPTY_MACRO_ARG
WARNING_DISABLED, // WARNING_EMPTY_STRRPL
WARNING_DISABLED, // WARNING_LARGE_CONSTANT
WARNING_DISABLED, // WARNING_MACRO_SHIFT
WARNING_ENABLED, // WARNING_NESTED_COMMENT
WARNING_ENABLED, // WARNING_OBSOLETE
WARNING_DISABLED, // WARNING_SHIFT
WARNING_DISABLED, // WARNING_SHIFT_AMOUNT
WARNING_ENABLED, // WARNING_USER
Diagnostics warningStates;
bool warningsAreErrors;
WARNING_DISABLED, // WARNING_NUMERIC_STRING_1
WARNING_DISABLED, // WARNING_NUMERIC_STRING_2
WARNING_ENABLED, // WARNING_PURGE_1
WARNING_DISABLED, // WARNING_PURGE_2
WARNING_ENABLED, // WARNING_TRUNCATION_1
WARNING_DISABLED, // WARNING_TRUNCATION_2
WARNING_ENABLED, // WARNING_UNMAPPED_CHAR_1
WARNING_DISABLED, // WARNING_UNMAPPED_CHAR_2
enum WarningLevel {
LEVEL_DEFAULT, // Warnings that are enabled by default
LEVEL_ALL, // Warnings that probably indicate an error
LEVEL_EXTRA, // Warnings that are less likely to indicate an error
LEVEL_EVERYTHING, // Literally every warning
};
WarningState warningStates[ARRAY_SIZE(warningStates)];
struct WarningFlag {
char const *name;
WarningLevel level;
};
bool warningsAreErrors; // Set if `-Werror` was specified
static WarningState warningState(WarningID id) {
// Check if warnings are globally disabled
if (!warnings)
return WARNING_DISABLED;
// Get the actual state
WarningState state = warningStates[id];
if (state == WARNING_DEFAULT)
// The state isn't set, grab its default state
state = defaultWarnings[id];
if (warningsAreErrors && state == WARNING_ENABLED)
state = WARNING_ERROR;
return state;
}
static char const * const warningFlags[NB_WARNINGS] = {
"assert",
"backwards-for",
"builtin-args",
"charmap-redef",
"div",
"empty-data-directive",
"empty-macro-arg",
"empty-strrpl",
"large-constant",
"macro-shift",
"nested-comment",
"obsolete",
"shift",
"shift-amount",
"user",
static const WarningFlag metaWarnings[] = {
{"all", LEVEL_ALL },
{"extra", LEVEL_EXTRA },
{"everything", LEVEL_EVERYTHING},
};
static const WarningFlag warningFlags[NB_WARNINGS] = {
{"assert", LEVEL_DEFAULT },
{"backwards-for", LEVEL_ALL },
{"builtin-args", LEVEL_ALL },
{"charmap-redef", LEVEL_ALL },
{"div", LEVEL_EVERYTHING},
{"empty-data-directive", LEVEL_ALL },
{"empty-macro-arg", LEVEL_EXTRA },
{"empty-strrpl", LEVEL_ALL },
{"large-constant", LEVEL_ALL },
{"macro-shift", LEVEL_EXTRA },
{"nested-comment", LEVEL_DEFAULT },
{"obsolete", LEVEL_DEFAULT },
{"shift", LEVEL_EVERYTHING},
{"shift-amount", LEVEL_EVERYTHING},
{"user", LEVEL_DEFAULT },
// Parametric warnings
"numeric-string",
"numeric-string",
"purge",
"purge",
"truncation",
"truncation",
"unmapped-char",
"unmapped-char",
// Meta warnings
"all",
"extra",
"everything", // Especially useful for testing
{"numeric-string", LEVEL_EVERYTHING},
{"numeric-string", LEVEL_EVERYTHING},
{"purge", LEVEL_DEFAULT },
{"purge", LEVEL_ALL },
{"truncation", LEVEL_DEFAULT },
{"truncation", LEVEL_EXTRA },
{"unmapped-char", LEVEL_DEFAULT },
{"unmapped-char", LEVEL_ALL },
};
static const struct {
char const *name;
uint8_t nbLevels;
WarningID firstID;
WarningID lastID;
uint8_t defaultLevel;
} paramWarnings[] = {
{"numeric-string", 2, 1},
{"purge", 2, 1},
{"truncation", 2, 2},
{"unmapped-char", 2, 1},
{WARNING_NUMERIC_STRING_1, WARNING_NUMERIC_STRING_2, 1},
{WARNING_PURGE_1, WARNING_PURGE_2, 1},
{WARNING_TRUNCATION_1, WARNING_TRUNCATION_2, 2},
{WARNING_UNMAPPED_CHAR_1, WARNING_UNMAPPED_CHAR_2, 1},
};
static bool tryProcessParamWarning(char const *flag, uint8_t param, WarningState state) {
WarningID baseID = PARAM_WARNINGS_START;
enum WarningBehavior { DISABLED, ENABLED, ERROR };
for (size_t i = 0; i < ARRAY_SIZE(paramWarnings); i++) {
uint8_t maxParam = paramWarnings[i].nbLevels;
static WarningBehavior getWarningBehavior(WarningID id) {
// Check if warnings are globally disabled
if (!warnings)
return WarningBehavior::DISABLED;
if (!strcmp(flag, paramWarnings[i].name)) { // Match!
if (!strcmp(flag, "numeric-string"))
warning(WARNING_OBSOLETE, "Warning flag \"numeric-string\" is deprecated\n");
// Get the state of this warning flag
WarningState const &flagState = warningStates.flagStates[id];
WarningState const &metaState = warningStates.metaStates[id];
// If making the warning an error but param is 0, set to the maximum
// This accommodates `-Werror=flag`, but also `-Werror=flag=0`, which is
// thus filtered out by the caller.
// A param of 0 makes sense for disabling everything, but neither for
// enabling nor "erroring". Use the default for those.
if (param == 0 && state != WARNING_DISABLED) {
param = paramWarnings[i].defaultLevel;
} else if (param > maxParam) {
if (param != 255) // Don't warn if already capped
warnx(
"Got parameter %" PRIu8
" for warning flag \"%s\", but the maximum is %" PRIu8 "; capping.\n",
param,
flag,
maxParam
);
param = maxParam;
}
// If subsequent checks determine that the warning flag is enabled, this checks whether it has
// -Werror without -Wno-error=<flag> or -Wno-error=<meta>, which makes it into an error
bool warningIsError = warningsAreErrors && flagState.error != WARNING_DISABLED
&& metaState.error != WARNING_DISABLED;
WarningBehavior enabledBehavior =
warningIsError ? WarningBehavior::ERROR : WarningBehavior::ENABLED;
// Set the first <param> to enabled/error, and disable the rest
for (uint8_t ofs = 0; ofs < maxParam; ofs++) {
warningStates[baseID + ofs] = ofs < param ? state : WARNING_DISABLED;
}
return true;
}
// First, check the state of the specific warning flag
if (flagState.state == WARNING_DISABLED) // -Wno-<flag>
return WarningBehavior::DISABLED;
if (flagState.error == WARNING_ENABLED) // -Werror=<flag>
return WarningBehavior::ERROR;
if (flagState.state == WARNING_ENABLED) // -W<flag>
return enabledBehavior;
baseID = (WarningID)(baseID + maxParam);
}
return false;
// If no flag is specified, check the state of the "meta" flags that affect this warning flag
if (metaState.state == WARNING_DISABLED) // -Wno-<meta>
return WarningBehavior::DISABLED;
if (metaState.error == WARNING_ENABLED) // -Werror=<meta>
return WarningBehavior::ERROR;
if (metaState.state == WARNING_ENABLED) // -W<meta>
return enabledBehavior;
// If no meta flag is specified, check the default state of this warning flag
if (warningFlags[id].level == LEVEL_DEFAULT) // enabled by default
return enabledBehavior;
// No flag enables this warning, explicitly or implicitly
return WarningBehavior::DISABLED;
}
enum MetaWarningCommand { META_WARNING_DONE = NB_WARNINGS };
// Warnings that probably indicate an error
static uint8_t const _wallCommands[] = {
WARNING_BACKWARDS_FOR,
WARNING_BUILTIN_ARG,
WARNING_CHARMAP_REDEF,
WARNING_EMPTY_DATA_DIRECTIVE,
WARNING_EMPTY_STRRPL,
WARNING_LARGE_CONSTANT,
WARNING_NESTED_COMMENT,
WARNING_OBSOLETE,
WARNING_PURGE_1,
WARNING_PURGE_2,
WARNING_UNMAPPED_CHAR_1,
META_WARNING_DONE,
};
// Warnings that are less likely to indicate an error
static uint8_t const _wextraCommands[] = {
WARNING_EMPTY_MACRO_ARG,
WARNING_MACRO_SHIFT,
WARNING_NESTED_COMMENT,
WARNING_OBSOLETE,
WARNING_PURGE_1,
WARNING_PURGE_2,
WARNING_TRUNCATION_1,
WARNING_TRUNCATION_2,
WARNING_UNMAPPED_CHAR_1,
WARNING_UNMAPPED_CHAR_2,
META_WARNING_DONE,
};
// Literally everything. Notably useful for testing
static uint8_t const _weverythingCommands[] = {
WARNING_BACKWARDS_FOR,
WARNING_BUILTIN_ARG,
WARNING_CHARMAP_REDEF,
WARNING_DIV,
WARNING_EMPTY_DATA_DIRECTIVE,
WARNING_EMPTY_MACRO_ARG,
WARNING_EMPTY_STRRPL,
WARNING_LARGE_CONSTANT,
WARNING_MACRO_SHIFT,
WARNING_NESTED_COMMENT,
WARNING_OBSOLETE,
WARNING_PURGE_1,
WARNING_PURGE_2,
WARNING_SHIFT,
WARNING_SHIFT_AMOUNT,
WARNING_TRUNCATION_1,
WARNING_TRUNCATION_2,
WARNING_UNMAPPED_CHAR_1,
WARNING_UNMAPPED_CHAR_2,
// WARNING_USER,
META_WARNING_DONE,
};
static uint8_t const *metaWarningCommands[NB_META_WARNINGS] = {
_wallCommands,
_wextraCommands,
_weverythingCommands,
};
void WarningState::update(WarningState other) {
if (other.state != WARNING_DEFAULT)
state = other.state;
if (other.error != WARNING_DEFAULT)
error = other.error;
}
void processWarningFlag(char const *flag) {
static bool setError = false;
std::string rootFlag = flag;
// First, try to match against a "meta" warning
for (WarningID id : EnumSeq(META_WARNINGS_START, NB_WARNINGS)) {
// TODO: improve the matching performance?
if (!strcmp(flag, warningFlags[id])) {
// We got a match!
if (setError)
errx("Cannot make meta warning \"%s\" into an error", flag);
for (uint8_t const *ptr = metaWarningCommands[id - META_WARNINGS_START];
*ptr != META_WARNING_DONE;
ptr++) {
// Warning flag, set without override
if (warningStates[*ptr] == WARNING_DEFAULT)
warningStates[*ptr] = WARNING_ENABLED;
}
return;
}
}
// If it's not a meta warning, specially check against `-Werror`
if (!strncmp(flag, "error", QUOTEDSTRLEN("error"))) {
char const *errorFlag = flag + QUOTEDSTRLEN("error");
switch (*errorFlag) {
case '\0':
// `-Werror`
// Check for `-Werror` or `-Wno-error` to return early
if (rootFlag == "error") {
// `-Werror` promotes warnings to errors
warningsAreErrors = true;
return;
case '=':
// `-Werror=XXX`
setError = true;
processWarningFlag(errorFlag + 1); // Skip the `=`
setError = false;
} else if (rootFlag == "no-error") {
// `-Wno-error` disables promotion of warnings to errors
warningsAreErrors = false;
return;
// Otherwise, allow parsing as another flag
}
}
// Well, it's either a normal warning or a mistake
// Check for prefixes that affect what the flag does
WarningState state;
if (rootFlag.starts_with("error=")) {
// `-Werror=<flag>` enables the flag as an error
state = {.state = WARNING_ENABLED, .error = WARNING_ENABLED};
rootFlag.erase(0, QUOTEDSTRLEN("error="));
} else if (rootFlag.starts_with("no-error=")) {
// `-Wno-error=<flag>` prevents the flag from being an error,
// without affecting whether it is enabled
state = {.state = WARNING_DEFAULT, .error = WARNING_DISABLED};
rootFlag.erase(0, QUOTEDSTRLEN("no-error="));
} else if (rootFlag.starts_with("no-")) {
// `-Wno-<flag>` disables the flag
state = {.state = WARNING_DISABLED, .error = WARNING_DEFAULT};
rootFlag.erase(0, QUOTEDSTRLEN("no-"));
} else {
// `-W<flag>` enables the flag
state = {.state = WARNING_ENABLED, .error = WARNING_DEFAULT};
}
WarningState state = setError ? WARNING_ERROR
// Not an error, then check if this is a negation
: strncmp(flag, "no-", QUOTEDSTRLEN("no-")) ? WARNING_ENABLED
: WARNING_DISABLED;
char const *rootFlag = state == WARNING_DISABLED ? flag + QUOTEDSTRLEN("no-") : flag;
// Is this a "parametric" warning?
if (state != WARNING_DISABLED) { // The `no-` form cannot be parametrized
// Check for an `=` parameter to process as a parametric warning
// `-Wno-<flag>` and `-Wno-error=<flag>` negation cannot have an `=` parameter, but without a
// parameter, the 0 value will apply to all levels of a parametric warning
uint8_t param = 0;
bool hasParam = false;
if (state.state == WARNING_ENABLED) {
// First, check if there is an "equals" sign followed by a decimal number
char const *equals = strchr(rootFlag, '=');
// Ignore an equal sign at the very end of the string
if (auto equals = rootFlag.find('=');
equals != rootFlag.npos && equals != rootFlag.size() - 1) {
hasParam = true;
if (equals && equals[1] != '\0') { // Ignore an equal sign at the very end as well
// Is the rest of the string a decimal number?
// We want to avoid `strtoul`'s whitespace and sign, so we parse manually
uint8_t param = 0;
char const *ptr = equals + 1;
char const *ptr = rootFlag.c_str() + equals + 1;
bool warned = false;
// The `if`'s condition above ensures that this will run at least once
@@ -290,7 +187,7 @@ void processWarningFlag(char const *flag) {
// Avoid overflowing!
if (param > UINT8_MAX - (*ptr - '0')) {
if (!warned)
warnx("Invalid warning flag \"%s\": capping parameter at 255\n", flag);
warnx("Invalid warning flag \"%s\": capping parameter at 255", flag);
warned = true; // Only warn once, cap always
param = 255;
continue;
@@ -300,40 +197,83 @@ void processWarningFlag(char const *flag) {
ptr++;
} while (*ptr);
// If we managed to the end of the string, check that the warning indeed
// accepts a parameter
// If we reached the end of the string, truncate it at the '='
if (*ptr == '\0') {
if (setError && param == 0) {
warnx("Ignoring nonsensical warning flag \"%s\"\n", flag);
return;
rootFlag.resize(equals);
// `-W<flag>=0` is equivalent to `-Wno-<flag>`
if (param == 0)
state.state = WARNING_DISABLED;
}
}
}
std::string truncFlag = rootFlag;
// Try to match the flag against a parametric warning
// If there was an equals sign, it will have set `param`; if not, `param` will be 0, which
// applies to all levels
for (auto const &paramWarning : paramWarnings) {
WarningID baseID = paramWarning.firstID;
uint8_t maxParam = paramWarning.lastID - baseID + 1;
assume(paramWarning.defaultLevel <= maxParam);
truncFlag.resize(equals - rootFlag); // Truncate the param at the '='
if (tryProcessParamWarning(
truncFlag.c_str(), param, param == 0 ? WARNING_DISABLED : state
))
if (rootFlag == warningFlags[baseID].name) { // Match!
if (rootFlag == "numeric-string")
warning(WARNING_OBSOLETE, "Warning flag \"numeric-string\" is deprecated\n");
// If making the warning an error but param is 0, set to the maximum
// This accommodates `-Werror=<flag>`, but also `-Werror=<flag>=0`, which is
// thus filtered out by the caller.
// A param of 0 makes sense for disabling everything, but neither for
// enabling nor "erroring". Use the default for those.
if (param == 0) {
param = paramWarning.defaultLevel;
} else if (param > maxParam) {
if (param != 255) // Don't warn if already capped
warnx(
"Invalid parameter %" PRIu8
" for warning flag \"%s\"; capping at maximum %" PRIu8,
param,
rootFlag.c_str(),
maxParam
);
param = maxParam;
}
// Set the first <param> to enabled/error, and disable the rest
for (uint8_t ofs = 0; ofs < maxParam; ofs++) {
WarningState &warning = warningStates.flagStates[baseID + ofs];
if (ofs < param)
warning.update(state);
else
warning.state = WARNING_DISABLED;
}
return;
}
}
// Try to match against a non-parametric warning, unless there was an equals sign
if (!hasParam) {
// Try to match against a "meta" warning
for (WarningFlag const &metaWarning : metaWarnings) {
if (rootFlag == metaWarning.name) {
// Set each of the warning flags that meets this level
for (WarningID id : EnumSeq(NB_WARNINGS)) {
if (metaWarning.level >= warningFlags[id].level)
warningStates.metaStates[id].update(state);
}
return;
}
}
// Try to match the flag against a "normal" flag
for (WarningID id : EnumSeq(NB_PLAIN_WARNINGS)) {
if (!strcmp(rootFlag, warningFlags[id])) {
// We got a match!
warningStates[id] = state;
if (rootFlag == warningFlags[id].name) {
warningStates.flagStates[id].update(state);
return;
}
}
}
// Lastly, this might be a "parametric" warning without an equals sign
// If it is, treat the param as 1 if enabling, or 0 if disabling
if (tryProcessParamWarning(rootFlag, 0, state))
return;
warnx("Unknown warning `%s`", flag);
warnx("Unknown warning flag \"%s\"", flag);
}
void printDiag(
@@ -377,26 +317,22 @@ void error(char const *fmt, ...) {
}
void warning(WarningID id, char const *fmt, ...) {
char const *flag = warningFlags[id];
char const *flag = warningFlags[id].name;
va_list args;
va_start(args, fmt);
switch (warningState(id)) {
case WARNING_DISABLED:
switch (getWarningBehavior(id)) {
case WarningBehavior::DISABLED:
break;
case WARNING_ENABLED:
case WarningBehavior::ENABLED:
printDiag(fmt, args, "warning", ": [-W%s]", flag);
break;
case WARNING_ERROR:
case WarningBehavior::ERROR:
printDiag(fmt, args, "error", ": [-Werror=%s]", flag);
break;
case WARNING_DEFAULT:
unreachable_();
// Not reached
}
va_end(args);