From 1d2bfa2ba78e92d0498de14e7c4b8438c683dd4c Mon Sep 17 00:00:00 2001 From: Rangi Date: Sat, 4 Apr 2026 18:12:57 -0400 Subject: [PATCH 1/7] Refactor lexing of fixed-point numbers --- src/asm/lexer.cpp | 166 +++++++++++++++++--------------- test/asm/fixed-point-syntax.err | 6 +- test/asm/invalid-numbers.err | 2 +- 3 files changed, 91 insertions(+), 83 deletions(-) diff --git a/src/asm/lexer.cpp b/src/asm/lexer.cpp index 145adbc44..90c264bba 100644 --- a/src/asm/lexer.cpp +++ b/src/asm/lexer.cpp @@ -935,87 +935,95 @@ static std::string readAnonLabelRef(char c) { return sym_MakeAnonLabelName(n, c == '-'); } -static void checkDigitSeparator(bool nonDigit) { - if (nonDigit) { +static void checkDigitSeparator(bool prevSeparator) { + if (prevSeparator) { error("Invalid integer constant, '_' after another '_'"); } } -static void checkIntegerConstantSuffix(bool empty, bool nonDigit, char const *prefix) { +static void checkIntegerConstantSuffix(bool empty, bool prevSeparator, char const *prefix) { if (empty) { error("Invalid integer constant, no digits after %s", prefix); } - if (nonDigit) { + if (prevSeparator) { error("Invalid integer constant, trailing '_'"); } } -static uint32_t readFractionalPart(uint32_t integer) { - uint32_t value = 0, divisor = 1; - uint8_t precision = 0; - enum { - READFRACTIONALPART_DIGITS, - READFRACTIONALPART_PRECISION, - READFRACTIONALPART_PRECISION_DIGITS, - } state = READFRACTIONALPART_DIGITS; - bool anyDigit = false; - bool nonDigit = false; +static bool readFractionDigits(uint32_t ÷nd, uint32_t &divisor) { + bool seenDigit = false; + bool prevSeparator = false; for (int c = peek();; c = nextChar()) { - if (state == READFRACTIONALPART_DIGITS) { - if (c == '_') { - if (nonDigit) { - error("Invalid fixed-point constant, '_' after another '_'"); - } else if (!anyDigit) { - error("Invalid fixed-point constant, '_' after '.'"); - } - nonDigit = true; - continue; + if (c == '_') { + if (!seenDigit) { + error("Invalid fixed-point constant, '_' after '.'"); } - - if (c == 'q' || c == 'Q') { - state = READFRACTIONALPART_PRECISION; - nonDigit = false; // '_' is allowed before 'q'/'Q' - continue; - } else if (!isDigit(c)) { - break; + if (prevSeparator) { + error("Invalid fixed-point constant, '_' after another '_'"); } - c -= '0'; - anyDigit = true; - nonDigit = false; + prevSeparator = true; + continue; + } + if (isDigit(c)) { + seenDigit = true; + prevSeparator = false; + c -= '0'; if (divisor > (UINT32_MAX - c) / 10) { warning(WARNING_LARGE_CONSTANT, "Precision of fixed-point constant is too large"); // Discard any additional digits skipChars([](int d) { return isDigit(d) || d == '_'; }); - break; + return false; } - value = value * 10 + c; + dividend = dividend * 10 + c; divisor *= 10; - } else { - if (c == '.' && state == READFRACTIONALPART_PRECISION) { - state = READFRACTIONALPART_PRECISION_DIGITS; - continue; - } else if (!isDigit(c)) { - break; - } + continue; + } - precision = precision * 10 + (c - '0'); + // '_' is allowed before 'q'/'Q' + if (c == 'q' || c == 'Q') { + shiftChar(); + return true; + } + if (prevSeparator) { + error("Invalid fixed-point constant, trailing '_'"); } + return false; } +} - if (precision == 0) { - if (state >= READFRACTIONALPART_PRECISION) { - error("Invalid fixed-point constant, no significant digits after 'q'"); - } - precision = options.fixPrecision; - } else if (precision > 31) { +static uint8_t readPrecisionSuffix() { + if (peek() == '.') { + shiftChar(); + } + + uint8_t precision = 0; + bool empty = true; + + // '_' is not allowed after 'q'/'Q' + for (int c = peek(); isDigit(c); c = nextChar()) { + empty = false; + precision = precision * 10 + (c - '0'); + } + + if (empty) { + error("Invalid fixed-point constant, no digits after 'q'"); + return options.fixPrecision; + } + + return precision; +} + +static uint32_t readFixedPointMantissa(uint32_t integer) { + uint32_t dividend = 0, divisor = 1; + bool hasPrecisionSuffix = readFractionDigits(dividend, divisor); + uint8_t precision = hasPrecisionSuffix ? readPrecisionSuffix() : options.fixPrecision; + + if (precision < 1 || precision > 31) { error("Fixed-point constant precision must be between 1 and 31"); precision = options.fixPrecision; } - if (nonDigit) { - error("Invalid fixed-point constant, trailing '_'"); - } if (integer >= (1ULL << (32 - precision))) { warning(WARNING_LARGE_CONSTANT, "Magnitude of fixed-point constant is too large"); @@ -1024,7 +1032,7 @@ static uint32_t readFractionalPart(uint32_t integer) { // Cast to unsigned avoids undefined overflow behavior uint32_t fractional = - static_cast(round(static_cast(value) / divisor * pow(2.0, precision))); + static_cast(round(static_cast(dividend) / divisor * pow(2.0, precision))); return (integer << precision) | fractional; } @@ -1077,12 +1085,12 @@ void lexer_SetGfxDigits(char const digits[4]) { static uint32_t readBinaryNumber(char const *prefix) { uint32_t value = 0; bool empty = true; - bool nonDigit = false; + bool prevSeparator = false; for (int c = peek();; c = nextChar()) { if (c == '_') { - checkDigitSeparator(nonDigit); - nonDigit = true; + checkDigitSeparator(prevSeparator); + prevSeparator = true; continue; } @@ -1095,7 +1103,7 @@ static uint32_t readBinaryNumber(char const *prefix) { break; } empty = false; - nonDigit = false; + prevSeparator = false; if (value > (UINT32_MAX - bit) / 2) { warning(WARNING_LARGE_CONSTANT, "Integer constant is too large"); @@ -1106,19 +1114,19 @@ static uint32_t readBinaryNumber(char const *prefix) { value = value * 2 + bit; } - checkIntegerConstantSuffix(empty, nonDigit, prefix); + checkIntegerConstantSuffix(empty, prevSeparator, prefix); return value; } static uint32_t readOctalNumber(char const *prefix) { uint32_t value = 0; bool empty = true; - bool nonDigit = false; + bool prevSeparator = false; for (int c = peek();; c = nextChar()) { if (c == '_') { - checkDigitSeparator(nonDigit); - nonDigit = true; + checkDigitSeparator(prevSeparator); + prevSeparator = true; continue; } @@ -1127,7 +1135,7 @@ static uint32_t readOctalNumber(char const *prefix) { } c -= '0'; empty = false; - nonDigit = false; + prevSeparator = false; if (value > (UINT32_MAX - c) / 8) { warning(WARNING_LARGE_CONSTANT, "Integer constant is too large"); @@ -1138,19 +1146,19 @@ static uint32_t readOctalNumber(char const *prefix) { value = value * 8 + c; } - checkIntegerConstantSuffix(empty, nonDigit, prefix); + checkIntegerConstantSuffix(empty, prevSeparator, prefix); return value; } static uint32_t readDecimalNumber(int initial) { assume(isDigit(initial)); uint32_t value = initial - '0'; - bool nonDigit = false; + bool prevSeparator = false; for (int c = peek();; c = nextChar()) { if (c == '_') { - checkDigitSeparator(nonDigit); - nonDigit = true; + checkDigitSeparator(prevSeparator); + prevSeparator = true; continue; } @@ -1158,7 +1166,7 @@ static uint32_t readDecimalNumber(int initial) { break; } c -= '0'; - nonDigit = false; + prevSeparator = false; if (value > (UINT32_MAX - c) / 10) { warning(WARNING_LARGE_CONSTANT, "Integer constant is too large"); @@ -1169,19 +1177,19 @@ static uint32_t readDecimalNumber(int initial) { value = value * 10 + c; } - checkIntegerConstantSuffix(false, nonDigit, nullptr); + checkIntegerConstantSuffix(false, prevSeparator, nullptr); return value; } static uint32_t readHexNumber(char const *prefix) { uint32_t value = 0; bool empty = true; - bool nonDigit = false; + bool prevSeparator = false; for (int c = peek();; c = nextChar()) { if (c == '_') { - checkDigitSeparator(nonDigit); - nonDigit = true; + checkDigitSeparator(prevSeparator); + prevSeparator = true; continue; } @@ -1190,7 +1198,7 @@ static uint32_t readHexNumber(char const *prefix) { } c = parseHexDigit(c); empty = false; - nonDigit = false; + prevSeparator = false; if (value > (UINT32_MAX - c) / 16) { warning(WARNING_LARGE_CONSTANT, "Integer constant is too large"); @@ -1201,19 +1209,19 @@ static uint32_t readHexNumber(char const *prefix) { value = value * 16 + c; } - checkIntegerConstantSuffix(empty, nonDigit, prefix); + checkIntegerConstantSuffix(empty, prevSeparator, prefix); return value; } static uint32_t readGfxConstant() { uint32_t bitPlaneLower = 0, bitPlaneUpper = 0; uint8_t width = 0; - bool nonDigit = false; + bool prevSeparator = false; for (int c = peek();; c = nextChar()) { if (c == '_') { - checkDigitSeparator(nonDigit); - nonDigit = true; + checkDigitSeparator(prevSeparator); + prevSeparator = true; continue; } @@ -1229,7 +1237,7 @@ static uint32_t readGfxConstant() { } else { break; } - nonDigit = false; + prevSeparator = false; if (width < 8) { bitPlaneLower = bitPlaneLower << 1 | (pixel & 1); @@ -1247,7 +1255,7 @@ static uint32_t readGfxConstant() { WARNING_LARGE_CONSTANT, "Graphics constant is too large; only first 8 pixels considered" ); } - if (nonDigit) { + if (prevSeparator) { error("Invalid graphics constant, trailing '_'"); } @@ -1833,7 +1841,7 @@ static Token yylex_NORMAL() { if (peek() == '.') { shiftChar(); - n = readFractionalPart(n); + n = readFixedPointMantissa(n); } return Token(T_(NUMBER), n); } diff --git a/test/asm/fixed-point-syntax.err b/test/asm/fixed-point-syntax.err index 388d7f50a..82d15d4b9 100644 --- a/test/asm/fixed-point-syntax.err +++ b/test/asm/fixed-point-syntax.err @@ -1,6 +1,6 @@ -error: Invalid fixed-point constant, no significant digits after 'q' +error: Fixed-point constant precision must be between 1 and 31 at fixed-point-syntax.asm(10) -error: Invalid fixed-point constant, no significant digits after 'q' +error: Invalid fixed-point constant, no digits after 'q' at fixed-point-syntax.asm(11) error: syntax error, unexpected symbol at fixed-point-syntax.asm(11) @@ -10,6 +10,6 @@ error: Invalid integer constant, trailing '_' at fixed-point-syntax.asm(13) error: Invalid fixed-point constant, '_' after '.' at fixed-point-syntax.asm(14) -error: Invalid fixed-point constant, no significant digits after 'q' +error: Invalid fixed-point constant, no digits after 'q' at fixed-point-syntax.asm(15) Assembly aborted with 7 errors diff --git a/test/asm/invalid-numbers.err b/test/asm/invalid-numbers.err index 853cf0eab..2ee197704 100644 --- a/test/asm/invalid-numbers.err +++ b/test/asm/invalid-numbers.err @@ -20,7 +20,7 @@ warning: Graphics constant is too large; only first 8 pixels considered [-Wlarge at invalid-numbers.asm::try(2) <- invalid-numbers.asm(22) warning: Magnitude of fixed-point constant is too large [-Wlarge-constant] at invalid-numbers.asm::try(2) <- invalid-numbers.asm(23) -error: Invalid fixed-point constant, no significant digits after 'q' +error: Invalid fixed-point constant, no digits after 'q' at invalid-numbers.asm::try(2) <- invalid-numbers.asm(26) error: Fixed-point constant precision must be between 1 and 31 at invalid-numbers.asm::try(2) <- invalid-numbers.asm(29) From 398bb62fcf2562de8805e14c28d881791a354fb4 Mon Sep 17 00:00:00 2001 From: Rangi Date: Sun, 5 Apr 2026 16:11:41 -0400 Subject: [PATCH 2/7] Update based on review comments --- src/asm/lexer.cpp | 101 +++++++++++++++++++++++----------------------- 1 file changed, 50 insertions(+), 51 deletions(-) diff --git a/src/asm/lexer.cpp b/src/asm/lexer.cpp index 90c264bba..2d9b74ef4 100644 --- a/src/asm/lexer.cpp +++ b/src/asm/lexer.cpp @@ -935,40 +935,40 @@ static std::string readAnonLabelRef(char c) { return sym_MakeAnonLabelName(n, c == '-'); } -static void checkDigitSeparator(bool prevSeparator) { - if (prevSeparator) { - error("Invalid integer constant, '_' after another '_'"); +static void checkDigitSeparator(bool prevWasSeparator, char const *name = "integer") { + if (prevWasSeparator) { + error("Invalid %s constant, '_' after another '_'", name); } } -static void checkIntegerConstantSuffix(bool empty, bool prevSeparator, char const *prefix) { +static void checkDigitsEnding( + bool empty, char const *prefix, bool prevWasSeparator, char const *name = "integer" +) { if (empty) { - error("Invalid integer constant, no digits after %s", prefix); + error("Invalid %s constant, no digits after %s", name, prefix); } - if (prevSeparator) { - error("Invalid integer constant, trailing '_'"); + if (prevWasSeparator) { + error("Invalid %s constant, trailing '_'", name); } } static bool readFractionDigits(uint32_t ÷nd, uint32_t &divisor) { bool seenDigit = false; - bool prevSeparator = false; + bool prevWasSeparator = false; for (int c = peek();; c = nextChar()) { if (c == '_') { if (!seenDigit) { error("Invalid fixed-point constant, '_' after '.'"); } - if (prevSeparator) { - error("Invalid fixed-point constant, '_' after another '_'"); - } - prevSeparator = true; + checkDigitSeparator(prevWasSeparator, "fixed-point"); + prevWasSeparator = true; continue; } if (isDigit(c)) { seenDigit = true; - prevSeparator = false; + prevWasSeparator = false; c -= '0'; if (divisor > (UINT32_MAX - c) / 10) { warning(WARNING_LARGE_CONSTANT, "Precision of fixed-point constant is too large"); @@ -983,12 +983,10 @@ static bool readFractionDigits(uint32_t ÷nd, uint32_t &divisor) { // '_' is allowed before 'q'/'Q' if (c == 'q' || c == 'Q') { - shiftChar(); return true; } - if (prevSeparator) { - error("Invalid fixed-point constant, trailing '_'"); - } + + checkDigitsEnding(false, nullptr, prevWasSeparator, "fixed-point"); return false; } } @@ -1015,10 +1013,15 @@ static uint8_t readPrecisionSuffix() { return precision; } -static uint32_t readFixedPointMantissa(uint32_t integer) { +static uint32_t finishReadingFixedPoint(uint32_t integer) { uint32_t dividend = 0, divisor = 1; + uint8_t precision = options.fixPrecision; + bool hasPrecisionSuffix = readFractionDigits(dividend, divisor); - uint8_t precision = hasPrecisionSuffix ? readPrecisionSuffix() : options.fixPrecision; + if (hasPrecisionSuffix) { + shiftChar(); // Skip the 'q'/'Q' + precision = readPrecisionSuffix(); + } if (precision < 1 || precision > 31) { error("Fixed-point constant precision must be between 1 and 31"); @@ -1032,7 +1035,7 @@ static uint32_t readFixedPointMantissa(uint32_t integer) { // Cast to unsigned avoids undefined overflow behavior uint32_t fractional = - static_cast(round(static_cast(dividend) / divisor * pow(2.0, precision))); + static_cast(round(static_cast(dividend) / divisor * (1ULL << precision))); return (integer << precision) | fractional; } @@ -1085,12 +1088,12 @@ void lexer_SetGfxDigits(char const digits[4]) { static uint32_t readBinaryNumber(char const *prefix) { uint32_t value = 0; bool empty = true; - bool prevSeparator = false; + bool prevWasSeparator = false; for (int c = peek();; c = nextChar()) { if (c == '_') { - checkDigitSeparator(prevSeparator); - prevSeparator = true; + checkDigitSeparator(prevWasSeparator); + prevWasSeparator = true; continue; } @@ -1103,7 +1106,7 @@ static uint32_t readBinaryNumber(char const *prefix) { break; } empty = false; - prevSeparator = false; + prevWasSeparator = false; if (value > (UINT32_MAX - bit) / 2) { warning(WARNING_LARGE_CONSTANT, "Integer constant is too large"); @@ -1114,19 +1117,19 @@ static uint32_t readBinaryNumber(char const *prefix) { value = value * 2 + bit; } - checkIntegerConstantSuffix(empty, prevSeparator, prefix); + checkDigitsEnding(empty, prefix, prevWasSeparator); return value; } static uint32_t readOctalNumber(char const *prefix) { uint32_t value = 0; bool empty = true; - bool prevSeparator = false; + bool prevWasSeparator = false; for (int c = peek();; c = nextChar()) { if (c == '_') { - checkDigitSeparator(prevSeparator); - prevSeparator = true; + checkDigitSeparator(prevWasSeparator); + prevWasSeparator = true; continue; } @@ -1135,7 +1138,7 @@ static uint32_t readOctalNumber(char const *prefix) { } c -= '0'; empty = false; - prevSeparator = false; + prevWasSeparator = false; if (value > (UINT32_MAX - c) / 8) { warning(WARNING_LARGE_CONSTANT, "Integer constant is too large"); @@ -1146,19 +1149,19 @@ static uint32_t readOctalNumber(char const *prefix) { value = value * 8 + c; } - checkIntegerConstantSuffix(empty, prevSeparator, prefix); + checkDigitsEnding(empty, prefix, prevWasSeparator); return value; } static uint32_t readDecimalNumber(int initial) { assume(isDigit(initial)); uint32_t value = initial - '0'; - bool prevSeparator = false; + bool prevWasSeparator = false; for (int c = peek();; c = nextChar()) { if (c == '_') { - checkDigitSeparator(prevSeparator); - prevSeparator = true; + checkDigitSeparator(prevWasSeparator); + prevWasSeparator = true; continue; } @@ -1166,7 +1169,7 @@ static uint32_t readDecimalNumber(int initial) { break; } c -= '0'; - prevSeparator = false; + prevWasSeparator = false; if (value > (UINT32_MAX - c) / 10) { warning(WARNING_LARGE_CONSTANT, "Integer constant is too large"); @@ -1177,19 +1180,19 @@ static uint32_t readDecimalNumber(int initial) { value = value * 10 + c; } - checkIntegerConstantSuffix(false, prevSeparator, nullptr); + checkDigitsEnding(false, nullptr, prevWasSeparator); return value; } static uint32_t readHexNumber(char const *prefix) { uint32_t value = 0; bool empty = true; - bool prevSeparator = false; + bool prevWasSeparator = false; for (int c = peek();; c = nextChar()) { if (c == '_') { - checkDigitSeparator(prevSeparator); - prevSeparator = true; + checkDigitSeparator(prevWasSeparator); + prevWasSeparator = true; continue; } @@ -1198,7 +1201,7 @@ static uint32_t readHexNumber(char const *prefix) { } c = parseHexDigit(c); empty = false; - prevSeparator = false; + prevWasSeparator = false; if (value > (UINT32_MAX - c) / 16) { warning(WARNING_LARGE_CONSTANT, "Integer constant is too large"); @@ -1209,19 +1212,19 @@ static uint32_t readHexNumber(char const *prefix) { value = value * 16 + c; } - checkIntegerConstantSuffix(empty, prevSeparator, prefix); + checkDigitsEnding(empty, prefix, prevWasSeparator); return value; } static uint32_t readGfxConstant() { uint32_t bitPlaneLower = 0, bitPlaneUpper = 0; uint8_t width = 0; - bool prevSeparator = false; + bool prevWasSeparator = false; for (int c = peek();; c = nextChar()) { if (c == '_') { - checkDigitSeparator(prevSeparator); - prevSeparator = true; + checkDigitSeparator(prevWasSeparator); + prevWasSeparator = true; continue; } @@ -1237,7 +1240,7 @@ static uint32_t readGfxConstant() { } else { break; } - prevSeparator = false; + prevWasSeparator = false; if (width < 8) { bitPlaneLower = bitPlaneLower << 1 | (pixel & 1); @@ -1248,16 +1251,12 @@ static uint32_t readGfxConstant() { } } - if (width == 0) { - error("Invalid graphics constant, no digits after '`'"); - } else if (width == 9) { + checkDigitsEnding(width == 0, "'`'", prevWasSeparator, "graphics"); + if (width == 9) { warning( WARNING_LARGE_CONSTANT, "Graphics constant is too large; only first 8 pixels considered" ); } - if (prevSeparator) { - error("Invalid graphics constant, trailing '_'"); - } return bitPlaneUpper << 8 | bitPlaneLower; } @@ -1841,7 +1840,7 @@ static Token yylex_NORMAL() { if (peek() == '.') { shiftChar(); - n = readFixedPointMantissa(n); + n = finishReadingFixedPoint(n); } return Token(T_(NUMBER), n); } From 54bc7a182ff06828db9896c943e96fbba7f913c6 Mon Sep 17 00:00:00 2001 From: Rangi Date: Sun, 5 Apr 2026 17:57:19 -0400 Subject: [PATCH 3/7] Use an `if/else` chain --- src/asm/lexer.cpp | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/asm/lexer.cpp b/src/asm/lexer.cpp index 2d9b74ef4..d9f568e8a 100644 --- a/src/asm/lexer.cpp +++ b/src/asm/lexer.cpp @@ -964,9 +964,7 @@ static bool readFractionDigits(uint32_t ÷nd, uint32_t &divisor) { checkDigitSeparator(prevWasSeparator, "fixed-point"); prevWasSeparator = true; continue; - } - - if (isDigit(c)) { + } else if (isDigit(c)) { seenDigit = true; prevWasSeparator = false; c -= '0'; @@ -979,15 +977,13 @@ static bool readFractionDigits(uint32_t ÷nd, uint32_t &divisor) { dividend = dividend * 10 + c; divisor *= 10; continue; - } - - // '_' is allowed before 'q'/'Q' - if (c == 'q' || c == 'Q') { + } else if (c == 'q' || c == 'Q') { + // '_' is allowed before 'q'/'Q' return true; + } else { + checkDigitsEnding(false, nullptr, prevWasSeparator, "fixed-point"); + return false; } - - checkDigitsEnding(false, nullptr, prevWasSeparator, "fixed-point"); - return false; } } From 7d6b98a2f2d30233dca23173815f3f41c79bb160 Mon Sep 17 00:00:00 2001 From: Rangi Date: Sun, 5 Apr 2026 22:37:19 -0400 Subject: [PATCH 4/7] Avoid using default parameters --- src/asm/lexer.cpp | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/asm/lexer.cpp b/src/asm/lexer.cpp index d9f568e8a..8efe60ae0 100644 --- a/src/asm/lexer.cpp +++ b/src/asm/lexer.cpp @@ -935,15 +935,14 @@ static std::string readAnonLabelRef(char c) { return sym_MakeAnonLabelName(n, c == '-'); } -static void checkDigitSeparator(bool prevWasSeparator, char const *name = "integer") { +static void checkDigitSeparator(bool prevWasSeparator, char const *name) { if (prevWasSeparator) { error("Invalid %s constant, '_' after another '_'", name); } } -static void checkDigitsEnding( - bool empty, char const *prefix, bool prevWasSeparator, char const *name = "integer" -) { +static void + checkDigitsEnding(bool empty, char const *prefix, bool prevWasSeparator, char const *name) { if (empty) { error("Invalid %s constant, no digits after %s", name, prefix); } @@ -1088,7 +1087,7 @@ static uint32_t readBinaryNumber(char const *prefix) { for (int c = peek();; c = nextChar()) { if (c == '_') { - checkDigitSeparator(prevWasSeparator); + checkDigitSeparator(prevWasSeparator, "integer"); prevWasSeparator = true; continue; } @@ -1113,7 +1112,7 @@ static uint32_t readBinaryNumber(char const *prefix) { value = value * 2 + bit; } - checkDigitsEnding(empty, prefix, prevWasSeparator); + checkDigitsEnding(empty, prefix, prevWasSeparator, "integer"); return value; } @@ -1124,7 +1123,7 @@ static uint32_t readOctalNumber(char const *prefix) { for (int c = peek();; c = nextChar()) { if (c == '_') { - checkDigitSeparator(prevWasSeparator); + checkDigitSeparator(prevWasSeparator, "integer"); prevWasSeparator = true; continue; } @@ -1145,7 +1144,7 @@ static uint32_t readOctalNumber(char const *prefix) { value = value * 8 + c; } - checkDigitsEnding(empty, prefix, prevWasSeparator); + checkDigitsEnding(empty, prefix, prevWasSeparator, "integer"); return value; } @@ -1156,7 +1155,7 @@ static uint32_t readDecimalNumber(int initial) { for (int c = peek();; c = nextChar()) { if (c == '_') { - checkDigitSeparator(prevWasSeparator); + checkDigitSeparator(prevWasSeparator, "integer"); prevWasSeparator = true; continue; } @@ -1176,7 +1175,7 @@ static uint32_t readDecimalNumber(int initial) { value = value * 10 + c; } - checkDigitsEnding(false, nullptr, prevWasSeparator); + checkDigitsEnding(false, nullptr, prevWasSeparator, "integer"); return value; } @@ -1187,7 +1186,7 @@ static uint32_t readHexNumber(char const *prefix) { for (int c = peek();; c = nextChar()) { if (c == '_') { - checkDigitSeparator(prevWasSeparator); + checkDigitSeparator(prevWasSeparator, "integer"); prevWasSeparator = true; continue; } @@ -1208,7 +1207,7 @@ static uint32_t readHexNumber(char const *prefix) { value = value * 16 + c; } - checkDigitsEnding(empty, prefix, prevWasSeparator); + checkDigitsEnding(empty, prefix, prevWasSeparator, "integer"); return value; } @@ -1219,7 +1218,7 @@ static uint32_t readGfxConstant() { for (int c = peek();; c = nextChar()) { if (c == '_') { - checkDigitSeparator(prevWasSeparator); + checkDigitSeparator(prevWasSeparator, "integer"); prevWasSeparator = true; continue; } From 6065f9dbd61d8df6f5e4985f998c6556741fc1b6 Mon Sep 17 00:00:00 2001 From: Rangi Date: Sun, 5 Apr 2026 22:51:34 -0400 Subject: [PATCH 5/7] Move the 'q'/'Q' check from `readFractionDigits` to `finishReadingFixedPoint` This incidentally fixes a bug with too-long fixed-point literals that have precision suffixes. --- src/asm/lexer.cpp | 21 +++++++++++---------- test/asm/fixed-point-syntax.asm | 2 ++ test/asm/fixed-point-syntax.err | 4 ++++ test/asm/fixed-point-syntax.out | 2 ++ 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/asm/lexer.cpp b/src/asm/lexer.cpp index 8efe60ae0..acd965535 100644 --- a/src/asm/lexer.cpp +++ b/src/asm/lexer.cpp @@ -970,18 +970,16 @@ static bool readFractionDigits(uint32_t ÷nd, uint32_t &divisor) { if (divisor > (UINT32_MAX - c) / 10) { warning(WARNING_LARGE_CONSTANT, "Precision of fixed-point constant is too large"); // Discard any additional digits - skipChars([](int d) { return isDigit(d) || d == '_'; }); - return false; + for (int d = peek(); isDigit(d) || d == '_'; d = nextChar()) { + c = d; + } + return c == '_'; } dividend = dividend * 10 + c; divisor *= 10; continue; - } else if (c == 'q' || c == 'Q') { - // '_' is allowed before 'q'/'Q' - return true; } else { - checkDigitsEnding(false, nullptr, prevWasSeparator, "fixed-point"); - return false; + return prevWasSeparator; } } } @@ -1012,10 +1010,13 @@ static uint32_t finishReadingFixedPoint(uint32_t integer) { uint32_t dividend = 0, divisor = 1; uint8_t precision = options.fixPrecision; - bool hasPrecisionSuffix = readFractionDigits(dividend, divisor); - if (hasPrecisionSuffix) { - shiftChar(); // Skip the 'q'/'Q' + bool prevWasSeparator = readFractionDigits(dividend, divisor); + if (int c = peek(); c == 'q' || c == 'Q') { + // '_' is allowed before 'q'/'Q', so do not call `checkDigitsEnding` + shiftChar(); precision = readPrecisionSuffix(); + } else { + checkDigitsEnding(false, nullptr, prevWasSeparator, "fixed-point"); } if (precision < 1 || precision > 31) { diff --git a/test/asm/fixed-point-syntax.asm b/test/asm/fixed-point-syntax.asm index e0eeb172b..7aa640782 100644 --- a/test/asm/fixed-point-syntax.asm +++ b/test/asm/fixed-point-syntax.asm @@ -13,3 +13,5 @@ println 12.34q1_5 println 1_.2 println 1._2 println 1.2q +println 1.999_999_999_999_999 +println 1.999_999_999_999_999q.16 diff --git a/test/asm/fixed-point-syntax.err b/test/asm/fixed-point-syntax.err index 82d15d4b9..11fb7dcaf 100644 --- a/test/asm/fixed-point-syntax.err +++ b/test/asm/fixed-point-syntax.err @@ -12,4 +12,8 @@ error: Invalid fixed-point constant, '_' after '.' at fixed-point-syntax.asm(14) error: Invalid fixed-point constant, no digits after 'q' at fixed-point-syntax.asm(15) +warning: Precision of fixed-point constant is too large [-Wlarge-constant] + at fixed-point-syntax.asm(16) +warning: Precision of fixed-point constant is too large [-Wlarge-constant] + at fixed-point-syntax.asm(17) Assembly aborted with 7 errors diff --git a/test/asm/fixed-point-syntax.out b/test/asm/fixed-point-syntax.out index cde3a1136..f182065bf 100644 --- a/test/asm/fixed-point-syntax.out +++ b/test/asm/fixed-point-syntax.out @@ -8,3 +8,5 @@ $C570A $13333 $13333 $13333 +$10000 +$10000 From 214a054f24095a1eeba30e801a80e8b7046061d2 Mon Sep 17 00:00:00 2001 From: Rangi Date: Mon, 6 Apr 2026 21:03:39 -0400 Subject: [PATCH 6/7] More review edits --- src/asm/lexer.cpp | 18 +++++++++--------- test/asm/fixed-point-syntax.asm | 6 ++++-- test/asm/fixed-point-syntax.err | 12 +++++++++--- test/asm/fixed-point-syntax.out | 2 ++ 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/asm/lexer.cpp b/src/asm/lexer.cpp index acd965535..9a26aff24 100644 --- a/src/asm/lexer.cpp +++ b/src/asm/lexer.cpp @@ -952,19 +952,18 @@ static void } static bool readFractionDigits(uint32_t ÷nd, uint32_t &divisor) { - bool seenDigit = false; bool prevWasSeparator = false; - for (int c = peek();; c = nextChar()) { + int c = peek(); + if (c == '_') { + error("Invalid fixed-point constant, '_' after '.'"); + } + + for (;; c = nextChar()) { if (c == '_') { - if (!seenDigit) { - error("Invalid fixed-point constant, '_' after '.'"); - } checkDigitSeparator(prevWasSeparator, "fixed-point"); prevWasSeparator = true; - continue; } else if (isDigit(c)) { - seenDigit = true; prevWasSeparator = false; c -= '0'; if (divisor > (UINT32_MAX - c) / 10) { @@ -977,11 +976,12 @@ static bool readFractionDigits(uint32_t ÷nd, uint32_t &divisor) { } dividend = dividend * 10 + c; divisor *= 10; - continue; } else { - return prevWasSeparator; + break; } } + + return prevWasSeparator; } static uint8_t readPrecisionSuffix() { diff --git a/test/asm/fixed-point-syntax.asm b/test/asm/fixed-point-syntax.asm index 7aa640782..6a0d7e4a0 100644 --- a/test/asm/fixed-point-syntax.asm +++ b/test/asm/fixed-point-syntax.asm @@ -8,10 +8,12 @@ println 1.q2 ; bad println 12.34q0 -println 12.34q_15 -println 12.34q1_5 +println 12.34q_15 ; lexes as `12.34q` (invalid) then symbol `_15` +println 12.34q1_5 ; lexes as `12.34q1` (valid) then symbol `_5` println 1_.2 println 1._2 +println 1.__2 println 1.2q println 1.999_999_999_999_999 +println 1.999_999_999_999_999q16 println 1.999_999_999_999_999q.16 diff --git a/test/asm/fixed-point-syntax.err b/test/asm/fixed-point-syntax.err index 11fb7dcaf..4c0e0f0ba 100644 --- a/test/asm/fixed-point-syntax.err +++ b/test/asm/fixed-point-syntax.err @@ -10,10 +10,16 @@ error: Invalid integer constant, trailing '_' at fixed-point-syntax.asm(13) error: Invalid fixed-point constant, '_' after '.' at fixed-point-syntax.asm(14) -error: Invalid fixed-point constant, no digits after 'q' +error: Invalid fixed-point constant, '_' after '.' at fixed-point-syntax.asm(15) -warning: Precision of fixed-point constant is too large [-Wlarge-constant] +error: Invalid fixed-point constant, '_' after another '_' + at fixed-point-syntax.asm(15) +error: Invalid fixed-point constant, no digits after 'q' at fixed-point-syntax.asm(16) warning: Precision of fixed-point constant is too large [-Wlarge-constant] at fixed-point-syntax.asm(17) -Assembly aborted with 7 errors +warning: Precision of fixed-point constant is too large [-Wlarge-constant] + at fixed-point-syntax.asm(18) +warning: Precision of fixed-point constant is too large [-Wlarge-constant] + at fixed-point-syntax.asm(19) +Assembly aborted with 9 errors diff --git a/test/asm/fixed-point-syntax.out b/test/asm/fixed-point-syntax.out index f182065bf..cac04af4b 100644 --- a/test/asm/fixed-point-syntax.out +++ b/test/asm/fixed-point-syntax.out @@ -8,5 +8,7 @@ $C570A $13333 $13333 $13333 +$13333 +$10000 $10000 $10000 From 2f3ad42977e2e90a3054ba71594108dbe62b0a89 Mon Sep 17 00:00:00 2001 From: Rangi Date: Mon, 6 Apr 2026 21:20:57 -0400 Subject: [PATCH 7/7] Avoid 5-deep indent --- src/asm/lexer.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/asm/lexer.cpp b/src/asm/lexer.cpp index 9a26aff24..2f5696470 100644 --- a/src/asm/lexer.cpp +++ b/src/asm/lexer.cpp @@ -969,9 +969,7 @@ static bool readFractionDigits(uint32_t ÷nd, uint32_t &divisor) { if (divisor > (UINT32_MAX - c) / 10) { warning(WARNING_LARGE_CONSTANT, "Precision of fixed-point constant is too large"); // Discard any additional digits - for (int d = peek(); isDigit(d) || d == '_'; d = nextChar()) { - c = d; - } + for (int d = peek(); isDigit(d) || d == '_'; c = d, d = nextChar()) {} return c == '_'; } dividend = dividend * 10 + c;