From bf69043a1d961a829fa1a03f84fa41fc5e9f493a Mon Sep 17 00:00:00 2001 From: Rangi42 Date: Sat, 19 Jul 2025 13:44:58 -0400 Subject: [PATCH] Reduce deep nesting some more --- include/diagnostics.hpp | 46 +++---- include/helpers.hpp | 2 +- src/asm/charmap.cpp | 6 +- src/asm/lexer.cpp | 276 ++++++++++++++++++++-------------------- src/fix/main.cpp | 3 +- src/gfx/pal_packing.cpp | 8 +- 6 files changed, 167 insertions(+), 174 deletions(-) diff --git a/include/diagnostics.hpp b/include/diagnostics.hpp index 7de45e01..a529d2f7 100644 --- a/include/diagnostics.hpp +++ b/include/diagnostics.hpp @@ -131,8 +131,8 @@ std::string Diagnostics::processWarningFlag(char const *flag) { auto [flagState, param] = getInitialWarningState(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 + // If there was an equals sign, it will have set `param`; if not, `param` will be 0, + // which applies to all levels for (ParamWarning const ¶mWarning : paramWarnings) { W baseID = paramWarning.firstID; uint8_t maxParam = paramWarning.lastID - baseID + 1; @@ -173,33 +173,35 @@ std::string Diagnostics::processWarningFlag(char const *flag) { return rootFlag; } - // Try to match against a non-parametric warning, unless there was an equals sign - if (!param.has_value()) { - // Try to match against a "meta" warning - for (WarningFlag const &metaWarning : metaWarnings) { - if (rootFlag != metaWarning.name) { - continue; - } + if (param.has_value()) { + warnx("Unknown warning flag parameter \"%s\"", rootFlag.c_str()); + return rootFlag; + } - // Set each of the warning flags that meets this level - for (W id : EnumSeq(W::NB_WARNINGS)) { - if (metaWarning.level >= warningFlags[id].level) { - state.metaStates[id].update(flagState); - } - } - return rootFlag; + // Try to match against a "meta" warning + for (WarningFlag const &metaWarning : metaWarnings) { + if (rootFlag != metaWarning.name) { + continue; } - // Try to match the flag against a "normal" flag - for (W id : EnumSeq(W::NB_PLAIN_WARNINGS)) { - if (rootFlag == warningFlags[id].name) { - state.flagStates[id].update(flagState); - return rootFlag; + // Set each of the warning flags that meets this level + for (W id : EnumSeq(W::NB_WARNINGS)) { + if (metaWarning.level >= warningFlags[id].level) { + state.metaStates[id].update(flagState); } } + return rootFlag; + } + + // Try to match against a "normal" flag + for (W id : EnumSeq(W::NB_PLAIN_WARNINGS)) { + if (rootFlag == warningFlags[id].name) { + state.flagStates[id].update(flagState); + return rootFlag; + } } - warnx("Unknown warning flag \"%s\"", flag); + warnx("Unknown warning flag \"%s\"", rootFlag.c_str()); return rootFlag; } diff --git a/include/helpers.hpp b/include/helpers.hpp index 1fc732cc..63b75c57 100644 --- a/include/helpers.hpp +++ b/include/helpers.hpp @@ -24,7 +24,7 @@ static inline void unreachable_() { #ifdef _MSC_VER #define assume(x) __assume(x) #else - // `[[gnu::assume()]]` for GCC or compatible also has insufficient support (GCC 13+ only) + // `[[gnu::assume()]]` for GCC or compatible also has insufficient support (GCC 13+ only) #define assume(x) \ do { \ if (!(x)) { \ diff --git a/src/asm/charmap.cpp b/src/asm/charmap.cpp index 80cc7c65..124267b6 100644 --- a/src/asm/charmap.cpp +++ b/src/asm/charmap.cpp @@ -41,10 +41,8 @@ struct Charmap { auto [nodeIdx, mapping] = std::move(prefixes.top()); prefixes.pop(); CharmapNode const &node = nodes[nodeIdx]; - if (node.isTerminal()) { - if (!callback(nodeIdx, mapping)) { - return false; - } + if (node.isTerminal() && !callback(nodeIdx, mapping)) { + return false; } for (unsigned c = 0; c < std::size(node.next); c++) { if (size_t nextIdx = node.next[c]; nextIdx) { diff --git a/src/asm/lexer.cpp b/src/asm/lexer.cpp index 20e17c03..38a191fe 100644 --- a/src/asm/lexer.cpp +++ b/src/asm/lexer.cpp @@ -641,7 +641,6 @@ static uint32_t readBracketedMacroArgNum() { } std::string symName; - for (; continuesIdentifier(c); c = peek()) { symName += c; shiftChar(); @@ -883,14 +882,21 @@ static void shiftChar() { static int nextChar() { int c = peek(); - - // If not at EOF, advance read position if (c != EOF) { shiftChar(); } return c; } +template +static int skipChars(P predicate) { + int c = peek(); + for (; predicate(c); c = peek()) { + shiftChar(); + } + return c; +} + static void handleCRLF(int c) { if (c == '\r' && peek() == '\n') { shiftChar(); @@ -1032,10 +1038,7 @@ static uint32_t readFractionalPart(uint32_t integer) { if (divisor > (UINT32_MAX - (c - '0')) / 10) { warning(WARNING_LARGE_CONSTANT, "Precision of fixed-point constant is too large"); // Discard any additional digits - shiftChar(); - while (c = peek(), (c >= '0' && c <= '9') || c == '_') { - shiftChar(); - } + skipChars([](int d) { return (d >= '0' && d <= '9') || d == '_'; }); break; } value = value * 10 + (c - '0'); @@ -1443,84 +1446,8 @@ static void appendExpandedString(std::string &str, std::string const &expanded) static void appendCharInLiteral(std::string &str, int c) { bool rawMode = lexerState->mode == LEXER_RAW; - switch (c) { - case '\\': // Character escape or macro arg - c = peek(); - switch (c) { - // Character escape - case '\\': - case '"': - case '\'': - case '{': - case '}': - if (rawMode) { - str += '\\'; - } - str += c; - shiftChar(); - break; - case 'n': - str += rawMode ? "\\n" : "\n"; - shiftChar(); - break; - case 'r': - str += rawMode ? "\\r" : "\r"; - shiftChar(); - break; - case 't': - str += rawMode ? "\\t" : "\t"; - shiftChar(); - break; - case '0': - if (rawMode) { - str += "\\0"; - } else { - str += '\0'; - } - shiftChar(); - break; - - // Line continuation - case ' ': - case '\t': - case '\r': - case '\n': - discardLineContinuation(); - break; - - // Macro arg - case '@': - case '#': - case '1': - case '2': - case '3': - case '4': - case '5': - case '6': - case '7': - case '8': - case '9': - case '<': - shiftChar(); - if (std::shared_ptr arg = readMacroArg(c); arg) { - appendExpandedString(str, *arg); - } - break; - - case EOF: // Can't really print that one - error("Illegal character escape at end of input"); - str += '\\'; - break; - - default: - error("Illegal character escape %s", printChar(c)); - str += c; - shiftChar(); - break; - } - break; - - case '{': // Symbol interpolation + // Symbol interpolation + if (c == '{') { // We'll be exiting the string/character scope, so re-enable expansions // (Not interpolations, since they're handled by the function itself...) lexerState->disableMacroArgs = false; @@ -1528,10 +1455,86 @@ static void appendCharInLiteral(std::string &str, int c) { appendExpandedString(str, *interpolation); } lexerState->disableMacroArgs = true; + return; + } + + // Regular characters will just get copied + if (c != '\\') { + str += c; + return; + } + + c = peek(); + switch (c) { + // Character escape + case '\\': + case '"': + case '\'': + case '{': + case '}': + if (rawMode) { + str += '\\'; + } + str += c; + shiftChar(); + break; + case 'n': + str += rawMode ? "\\n" : "\n"; + shiftChar(); + break; + case 'r': + str += rawMode ? "\\r" : "\r"; + shiftChar(); + break; + case 't': + str += rawMode ? "\\t" : "\t"; + shiftChar(); + break; + case '0': + if (rawMode) { + str += "\\0"; + } else { + str += '\0'; + } + shiftChar(); break; - default: // Regular characters will just get copied + // Line continuation + case ' ': + case '\t': + case '\r': + case '\n': + discardLineContinuation(); + break; + + // Macro arg + case '@': + case '#': + case '1': + case '2': + case '3': + case '4': + case '5': + case '6': + case '7': + case '8': + case '9': + case '<': + shiftChar(); + if (std::shared_ptr arg = readMacroArg(c); arg) { + appendExpandedString(str, *arg); + } + break; + + case EOF: // Can't really print that one + error("Illegal character escape at end of input"); + str += '\\'; + break; + + default: + error("Illegal character escape %s", printChar(c)); str += c; + shiftChar(); break; } } @@ -1583,37 +1586,38 @@ static void readString(std::string &str, bool rawString) { continue; } - // Close the string and return if it's terminated - if (c == '"') { - if (!multiline) { - if (rawMode) { - str += c; - } - return; - } - // Only """ ends a multi-line string - if (peek() != '"') { + if (c != '"') { + // Append the character or handle special ones + if (rawString) { str += c; - continue; + } else { + appendCharInLiteral(str, c); } - shiftChar(); - if (peek() != '"') { - str += "\"\""; - continue; - } - shiftChar(); + continue; + } + + // Close the string and return if it's terminated + if (!multiline) { if (rawMode) { - str += "\"\"\""; + str += c; } return; } - - // Append the character or handle special ones - if (rawString) { + // Only """ ends a multi-line string + if (peek() != '"') { str += c; - } else { - appendCharInLiteral(str, c); + continue; } + shiftChar(); + if (peek() != '"') { + str += "\"\""; + continue; + } + shiftChar(); + if (rawMode) { + str += "\"\"\""; + } + return; } } @@ -1629,27 +1633,25 @@ static void readCharacter(std::string &str) { } for (;;) { - int c = peek(); - - // '\r', '\n' or EOF ends a character early - if (c == EOF || c == '\r' || c == '\n') { + switch (int c = peek(); c) { + case '\r': + case '\n': + case EOF: + // '\r', '\n' or EOF ends a character early error("Unterminated character"); return; - } - - // We'll be staying in the character, so we can safely consume the char - shiftChar(); - - // Close the character and return if it's terminated - if (c == '\'') { + case '\'': + // Close the character and return if it's terminated + shiftChar(); if (rawMode) { str += c; } return; + default: + // Append the character or handle special ones + shiftChar(); + appendCharInLiteral(str, c); } - - // Append the character or handle special ones - appendCharInLiteral(str, c); } } @@ -2347,14 +2349,7 @@ static Token yylex_SKIP_TO_ENDR() { } } - // Skip whitespace - for (;;) { - c = peek(); - if (!isWhitespace(c)) { - break; - } - shiftChar(); - } + c = skipChars(isWhitespace); if (!startsIdentifier(c)) { continue; @@ -2489,19 +2484,18 @@ Capture lexer_CaptureRept() { case T_(POP_REPT): case T_(POP_FOR): depth++; - // Ignore the rest of that line - break; + break; // Ignore the rest of that line case T_(POP_ENDR): - if (!depth) { - endCapture(capture); - // The final ENDR has been captured, but we don't want it! - // We know we have read exactly "ENDR", not e.g. an EQUS - capture.span.size -= literal_strlen("ENDR"); - return capture; + if (depth) { + depth--; + break; // Ignore the rest of that line } - depth--; - break; + endCapture(capture); + // The final ENDR has been captured, but we don't want it! + // We know we have read exactly "ENDR", not e.g. an EQUS + capture.span.size -= literal_strlen("ENDR"); + return capture; default: break; diff --git a/src/fix/main.cpp b/src/fix/main.cpp index b3e62b1b..e94c2330 100644 --- a/src/fix/main.cpp +++ b/src/fix/main.cpp @@ -1032,8 +1032,7 @@ static void // This should be guaranteed from the size cap... static_assert(0x10000 * BANK_SIZE <= SSIZE_MAX, "Max input file size too large for OS"); // Compute number of banks and ROMX len from file size - nbBanks = (fileSize + (BANK_SIZE - 1)) / BANK_SIZE; - // = ceil(totalRomxLen / BANK_SIZE) + nbBanks = (fileSize + (BANK_SIZE - 1)) / BANK_SIZE; // ceil(fileSize / BANK_SIZE) totalRomxLen = fileSize >= BANK_SIZE ? fileSize - BANK_SIZE : 0; } else if (rom0Len == BANK_SIZE) { // Copy ROMX when reading a pipe, and we're not at EOF yet diff --git a/src/gfx/pal_packing.cpp b/src/gfx/pal_packing.cpp index 82ea820e..e88a0fc9 100644 --- a/src/gfx/pal_packing.cpp +++ b/src/gfx/pal_packing.cpp @@ -20,12 +20,12 @@ // The solvers here are picked from the paper at https://arxiv.org/abs/1605.00558: // "Algorithms for the Pagination Problem, a Bin Packing with Overlapping Items" -// Their formulation of the problem consists in packing "tiles" into "pages"; here is a -// correspondence table for our application of it: +// Their formulation of the problem consists in packing "tiles" into "pages". +// Here is a correspondence table for our application of it: // Paper | RGBGFX // ------+------- -// Tile | Proto-palette -// Page | Palette +// Tile | Proto-palette +// Page | Palette // A reference to a proto-palette, and attached attributes for sorting purposes struct ProtoPalAttrs {