diff --git a/.kiro/specs/calculator/design.md b/.kiro/specs/calculator/design.md index d38946a..6109bff 100644 --- a/.kiro/specs/calculator/design.md +++ b/.kiro/specs/calculator/design.md @@ -67,7 +67,7 @@ build.zig (workspace root) | `number.zig` | The exact/inexact numeric model (section 2.7), including how a value renders itself at a budget the caller supplies. Lowercase: `Number` is a tagged union, which a file-as-struct cannot express | | `tokenizer.zig` | Lexer, and `Base` for literals | | `ast.zig` | AST node definitions | -| `parser.zig` | Pratt parser -> AST | +| `Parser.zig` | Pratt parser -> AST (file-as-struct) | | `evaluator.zig` | Walk AST, produce results | | `bitwise.zig` | The fixed-width operators (`& \| xor ~ << >> >>> rol ror`), shared by both modes | | `programmer.zig` | Programmer-mode evaluation, its `Config`, wrapping arithmetic | @@ -89,10 +89,13 @@ what needed splitting was the type inside it. Each piece has gone to the module owns it. `struct_layout.zig` is still unimplemented (Phase 4). A file is named TitleCase when the file *is* the type, with its fields at container -level: `Integer.zig` and `Rational.zig`. `number.zig` stays lowercase because `Number` -is a tagged union and a Zig file is always a struct, so the file cannot be that type -without wrapping the union in a field, which would add a hop at every use of the tag -that is the type's whole identity. +level: `Integer.zig`, `Rational.zig` and `Parser.zig`. `number.zig` stays lowercase +because `Number` is a tagged union and a Zig file is always a struct, so the file +cannot be that type without wrapping the union in a field, which would add a hop at +every use of the tag that is the type's whole identity. `tokenizer.zig` stays +lowercase for a different reason: `Base`, `TokenKind` and `Token` are the lexer's +vocabulary rather than parts of `Tokenizer`, and `Base` in particular is used by the +AST and both evaluators without a tokenizer anywhere in sight. ### 2.2 Core Data Types @@ -185,7 +188,7 @@ assumes it (see `ast.zig` for why that distinction matters). the tree, because every walk over it recurses. A flat chain like `1+1+1+...` builds a left-deep tree whose depth is the operator count, and 4000 terms segfaulted `evalExact` before the limit existed. Exceeding either reports -`InvalidExpression`. +`ExpressionTooLarge` or `NestingTooDeep`, so the message says which. ### 2.4 Parser Design @@ -212,12 +215,22 @@ keyword forms: `&`/`and`, `|`/`or`, `~`/`not`. XOR is keyword-only (`xor`) since `^` is reserved for power. Every operator means the same thing in standard and programmer modes. +**The table above is one declaration in the code.** `parser.infix_table` lists each +infix operator once as `{ trigger, op, prec, assoc }`, where the trigger is either a +token kind or a keyword's text. It replaced four switches that had to agree pairwise +plus an inline special case for right-associativity; see Task 5.19. Both spellings of +power associate to the right (`2^3^2` is `2^(3^2)`), everything else to the left. + +**Assignment is a statement, not an operator.** `name = expr` is recognised at the top +of an input, not in prefix position, so it cannot appear where an operand can. It has +a precedence level in the table above for documentation only: nothing returns it. + **One implementation of the fixed-width operators.** `bitwise.zig` implements the eight fixed-width binary operators plus `~` and two's complement negation, over a `Domain` of width plus signedness. Standard mode passes `Domain.standard`, which is 64-bit signed and not configurable; programmer mode passes its configured width and signedness. Values are `u128` bit patterns masked to the width, the representation -`types.Integer` already used. Both callers dispatch through an `inline else` prong +`Integer` already used. Both callers dispatch through an `inline else` prong that resolves the operator at comptime, so an operator added to `BinaryOp` that belongs in this tier fails to compile rather than falling through to the rational tier. FR-2.13 covers the distance rules: shifts run to completion, rotations are @@ -1997,7 +2010,7 @@ Each module declares what it can fail with, and the sets compose: // parser.zig pub const Error = error{ UnexpectedToken, UnmatchedParen, UnexpectedEnd, - InvalidExpression, InvalidNumber, OutOfMemory, + ExpressionTooLarge, NestingTooDeep, InvalidNumber, UnterminatedString, OutOfMemory, }; // Rational.zig, inherited by number.zig diff --git a/.kiro/specs/calculator/tasks.md b/.kiro/specs/calculator/tasks.md index 75f6723..d88b569 100644 --- a/.kiro/specs/calculator/tasks.md +++ b/.kiro/specs/calculator/tasks.md @@ -914,6 +914,55 @@ plans a `--raw` flag, but today it is exercised only by tests. 100% line coverage, engine 99.44%. CLI output byte-identical across all five rows, both byte orders, ASCII packing and the multi-base standard-mode view. +### Task 5.19: One operator table; assignment is a statement; Parser.zig + +`parser.zig` became `Parser.zig`, file-as-struct, since everything in it serves the +parser and its external surface is exactly `Parser.init`, `parse`, `Error` and +`freeExpr`. The importers now read `Parser.Error` and `Parser.freeExpr` rather than +going through a `parser_mod` alias, and `engine.zig`'s re-export follows the name. +The fields sit just above the functions that use them rather than at the very top, +because the precedence table below reads as the language's binding order and belongs +in front of the code that consumes it. + +Four switches encoded one operator table: token kind to operator, token kind to +precedence, keyword to operator, keyword to precedence, plus `if (op == .pow)` inline +for right-associativity. Each pair had to agree and nothing enforced it. A kind with +a precedence but no operator advanced past the token and then reported "unexpected +token"; a keyword added to one switch and not the other bound at the wrong level. + +`infix_table` now lists each operator once as `{ trigger, op, prec, assoc }`, where +the trigger is a token kind or a keyword's text. `parseExpr` asks `infixOp()` for the +entry, so `parseInfix` receives an operator it already has and has no failure case +left. `keywordBinaryOp`, `keywordPrec`, `infixPrecedence` and `tokenToBinaryOp` are +gone, and with them the `_ = self` in the last one and the redundant `kind` parameter +in the third (it took a kind while reading `self.current` for the keyword arm, so the +two could disagree). A test walks the table and parses one expression per entry. + +**Assignment moved to statement level.** It was recognised in `parsePrefix`, so it +could appear wherever an operand could: + +``` +$ tally '1 + x = 2' +3 <- assigned 2 to x, then added +$ tally 'f(x = 1)' <- an argument list with a side effect +``` + +`parse` now handles `name = expr` itself, with a one-token lookahead (a copy of the +tokenizer, which is just a position in a string). Both cases above are parse errors, +and open item 10's first half is closed. Chained assignment (`x = y = 3`) goes with +it; nothing in the requirements asks for it. + +**An unterminated string literal is reported.** `readStringLiteral` returned the span +without its closing quote, and the parser stripped the first and last byte to get the +content, so `'AB` parsed as the literal `'A'` and `'` alone reported "invalid number". +The scan knows whether it found the quote, so it emits `unterminated_string` and the +parser reports `UnterminatedString`. The `len < 2` guard is now an assert. + +**`InvalidExpression` split into `ExpressionTooLarge` and `NestingTooDeep`.** Both +limits reported the same error, so "invalid expression" was all either could say. The +messages are now "the expression has too many terms" and "the expression is nested too +deeply". + ### Task 5.18: A literal is its text; each reader parses it at its own precision The tokenizer computed two lossy representations of every numeric literal up front, @@ -1008,7 +1057,9 @@ of bare `u128`, and `Config` holds width and signedness flat. `Integer.zig` and `Rational.zig` are named TitleCase and the file *is* the type: the fields sit at container level, `@This()` names it, and the auxiliary declarations (`IntType`, `BitWidth`, `Signedness`; `Error`, `DecimalResult`) are nested inside. -`git mv` kept the history. +`git mv` kept the history. `Parser.zig` joined them in Task 5.19: everything else in +that file (`Error`, the precedence table, `freeExpr`) exists to serve the parser, and +its whole external surface is `Parser.init`, `parse`, `Error` and `freeExpr`. `number.zig` stays lowercase. `Number` is a tagged union and a Zig file is always a struct container, so the file can only be that type by wrapping the union in a field. @@ -1250,8 +1301,8 @@ STILL OPEN, in the order I would take them: (Ctrl-L frees into an arena). 9. ~~Money formatting degrades to `?` and still exits 0 at large magnitudes.~~ Fixed by the de-duplication pass above. -10. Assignment parses in prefix position (`1 + x = 2` mutates `x`), and assignment - to a constant name is silently discarded. +10. ~~Assignment parses in prefix position (`1 + x = 2` mutates `x`)~~, fixed by Task + 5.19; assignment to a constant name is still silently discarded. 11. ~~Literals longer than 128 characters are rejected by a fixed tokenizer buffer, and non-decimal literals are capped at 64 bits, both below what the exact tier supports.~~ Fixed by Task 5.18. diff --git a/engine/src/parser.zig b/engine/src/Parser.zig similarity index 53% rename from engine/src/parser.zig rename to engine/src/Parser.zig index dc34343..cdd3649 100644 --- a/engine/src/parser.zig +++ b/engine/src/Parser.zig @@ -20,6 +20,11 @@ const Tokenizer = tokenizer_mod.Tokenizer; const TokenKind = tokenizer_mod.TokenKind; const Token = tokenizer_mod.Token; +/// The file is the parser: the state below is its fields, and the functions below +/// take it as `self`. Same shape as `Integer.zig` and `Rational.zig`; everything +/// else here (`Error`, the precedence table, `freeExpr`) exists to serve it. +const Parser = @This(); + /// What parsing can fail with. /// /// Declared here rather than shared: the parser used to return a single engine-wide @@ -30,8 +35,14 @@ pub const Error = error{ UnexpectedToken, UnmatchedParen, UnexpectedEnd, - InvalidExpression, + /// More nodes than `Parser.max_nodes`. Was `InvalidExpression`, which the depth + /// limit also returned, so "invalid expression" was the only thing either said. + ExpressionTooLarge, + /// Deeper nesting than `Parser.max_nest_depth`. + NestingTooDeep, InvalidNumber, + /// An opening quote with no closing one. + UnterminatedString, OutOfMemory, }; @@ -56,325 +67,323 @@ const Prec = enum(u8) { call = 10, // function calls }; -pub const Parser = struct { - source: []const u8, - tokenizer: Tokenizer, - current: Token, - allocator: Allocator, - /// Nodes built so far, checked against `max_nodes`. - node_count: usize, - /// Current parseExpr/parsePrefix nesting, checked against `max_nest_depth`. - nest_depth: usize, +/// One infix operator: what introduces it, what it means, how tightly it binds and +/// which way it associates. +const InfixOp = struct { + trigger: union(enum) { + /// Punctuation, recognised by token kind. + kind: TokenKind, + /// A keyword operator, recognised by an identifier's text. + keyword: []const u8, + }, + op: BinaryOp, + prec: Prec, + assoc: enum { left, right } = .left, +}; - /// Ceiling on tree size. - /// - /// This is a stack-safety limit, not a style preference. Everything that walks - /// a tree recurses on it (`evalExact`, `freeExpr`, `hasNonDecimalLiteral`), and - /// a flat chain like `1+1+1+...` builds a left-deep tree whose depth equals the - /// number of operators. Measured on this build: 3000 terms evaluated fine and - /// 4000 terms segfaulted in `evalExact`, from about 8 KB of input. A thousand - /// nodes is far more than any hand-written expression and leaves a wide margin. - pub const max_nodes: usize = 1000; +/// The infix operators, in one table. +/// +/// This was four switches that had to agree pairwise: two over token kinds (kind to +/// operator, kind to precedence) and two over keyword names, plus an inline +/// `if (op == .pow)` for right-associativity. A token kind with a precedence but no +/// operator would advance past the token and then report "unexpected token", and a +/// keyword added to one switch but not the other would silently bind at the wrong +/// level. Written once, none of that is expressible. +const infix_table = [_]InfixOp{ + .{ .trigger = .{ .kind = .pipe }, .op = .bit_or, .prec = .bit_or }, + .{ .trigger = .{ .keyword = "or" }, .op = .bit_or, .prec = .bit_or }, + .{ .trigger = .{ .keyword = "xor" }, .op = .bit_xor, .prec = .bit_xor }, + .{ .trigger = .{ .kind = .ampersand }, .op = .bit_and, .prec = .bit_and }, + .{ .trigger = .{ .keyword = "and" }, .op = .bit_and, .prec = .bit_and }, + .{ .trigger = .{ .kind = .shift_left }, .op = .shift_left, .prec = .shift }, + .{ .trigger = .{ .kind = .shift_right }, .op = .shift_right, .prec = .shift }, + .{ .trigger = .{ .kind = .shift_right_logical }, .op = .shift_right_logical, .prec = .shift }, + .{ .trigger = .{ .keyword = "rol" }, .op = .rotate_left, .prec = .shift }, + .{ .trigger = .{ .keyword = "ror" }, .op = .rotate_right, .prec = .shift }, + .{ .trigger = .{ .kind = .plus }, .op = .add, .prec = .additive }, + .{ .trigger = .{ .kind = .minus }, .op = .sub, .prec = .additive }, + .{ .trigger = .{ .kind = .star }, .op = .mul, .prec = .multiplicative }, + .{ .trigger = .{ .kind = .slash }, .op = .div, .prec = .multiplicative }, + .{ .trigger = .{ .kind = .percent }, .op = .mod, .prec = .multiplicative }, + // `^` is exponentiation in both modes (FR-2.12), and both spellings of it + // associate to the right: 2^3^2 is 2^(3^2). + .{ .trigger = .{ .kind = .caret }, .op = .pow, .prec = .power, .assoc = .right }, + .{ .trigger = .{ .kind = .star_star }, .op = .pow, .prec = .power, .assoc = .right }, +}; - /// Ceiling on nesting, which bounds recursion inside the parser itself. - /// Reached long before `max_nodes` by input like `((((...1...))))`. - pub const max_nest_depth: usize = 128; +source: []const u8, +tokenizer: Tokenizer, +current: Token, +allocator: Allocator, +/// Nodes built so far, checked against `max_nodes`. +node_count: usize, +/// Current parseExpr/parsePrefix nesting, checked against `max_nest_depth`. +nest_depth: usize, - /// The grammar does not depend on the mode: the same source parses to the same - /// tree in standard and programmer mode, and only evaluation differs. `init` - /// used to take a `Mode` and store it, along with `previous`, `had_error` and - /// `error_pos`; nothing ever read any of them. Errors are reported by returning - /// them, not by leaving a flag behind. - pub fn init(allocator: Allocator, source: []const u8) Parser { - var tok = Tokenizer.init(source); - const first = tok.next(); - return .{ - .source = source, - .tokenizer = tok, - .current = first, - .allocator = allocator, - .node_count = 0, - .nest_depth = 0, - }; +/// Ceiling on tree size. +/// +/// This is a stack-safety limit, not a style preference. Everything that walks +/// a tree recurses on it (`evalExact`, `freeExpr`, `hasNonDecimalLiteral`), and +/// a flat chain like `1+1+1+...` builds a left-deep tree whose depth equals the +/// number of operators. Measured on this build: 3000 terms evaluated fine and +/// 4000 terms segfaulted in `evalExact`, from about 8 KB of input. A thousand +/// nodes is far more than any hand-written expression and leaves a wide margin. +pub const max_nodes: usize = 1000; + +/// Ceiling on nesting, which bounds recursion inside the parser itself. +/// Reached long before `max_nodes` by input like `((((...1...))))`. +pub const max_nest_depth: usize = 128; + +/// The grammar does not depend on the mode: the same source parses to the same +/// tree in standard and programmer mode, and only evaluation differs. `init` +/// used to take a `Mode` and store it, along with `previous`, `had_error` and +/// `error_pos`; nothing ever read any of them. Errors are reported by returning +/// them, not by leaving a flag behind. +pub fn init(allocator: Allocator, source: []const u8) Parser { + var tok = Tokenizer.init(source); + const first = tok.next(); + return .{ + .source = source, + .tokenizer = tok, + .current = first, + .allocator = allocator, + .node_count = 0, + .nest_depth = 0, + }; +} + +/// Parse a complete expression. Returns error if parsing fails. +/// +/// On success the caller owns the tree and must release it with `freeExpr`. +/// On failure nothing is returned and nothing is left allocated: every error +/// path below frees what it built. Without that, a single typo in an +/// interactive session leaks the partial tree, which is exactly what the TUI +/// does on every keystroke-completed expression. +pub fn parse(self: *Parser) Error!*Expr { + const expr = try self.parseStatement(); + if (self.current.kind != .eof) { + // Trailing tokens: the tree parsed so far is unreachable. + freeExpr(self.allocator, expr); + return Error.UnexpectedToken; + } + return expr; +} + +/// A whole input: `name = expr`, or an expression. +/// +/// Assignment lives here rather than in `parsePrefix` because it is a statement, +/// not an operand. Parsed as a prefix expression it could appear wherever an +/// operand could, so `1 + x = 2` assigned 2 to `x` and evaluated to 3, and +/// `f(x = 1)` was an argument list with a side effect. +fn parseStatement(self: *Parser) Error!*Expr { + if (self.current.kind == .identifier and self.peekKind() == .equals) { + const name = self.current.text(self.source); + self.advance(); // the name + self.advance(); // the '=' + const value = try self.parseExpr(.none); + errdefer freeExpr(self.allocator, value); + return self.makeNode(.{ .assignment = .{ .name = name, .value = value } }); + } + return self.parseExpr(.none); +} + +/// The kind of the token after `current`, without consuming anything. The +/// tokenizer is a position in a string, so a copy of it is a lookahead. +fn peekKind(self: Parser) TokenKind { + var probe = self.tokenizer; + return probe.next().kind; +} + +/// Parse an expression with the given minimum precedence. +fn parseExpr(self: *Parser, min_prec: Prec) Error!*Expr { + if (self.nest_depth >= max_nest_depth) return Error.NestingTooDeep; + self.nest_depth += 1; + defer self.nest_depth -= 1; + + var left = try self.parsePrefix(); + // Each successful parseInfix returns a node that has adopted `left`, so + // this errdefer always covers the whole tree built so far. + errdefer freeExpr(self.allocator, left); + + while (self.infixOp()) |entry| { + if (@intFromEnum(entry.prec) <= @intFromEnum(min_prec)) break; + left = try self.parseInfix(left, entry); } - /// Parse a complete expression. Returns error if parsing fails. - /// - /// On success the caller owns the tree and must release it with `freeExpr`. - /// On failure nothing is returned and nothing is left allocated: every error - /// path below frees what it built. Without that, a single typo in an - /// interactive session leaks the partial tree, which is exactly what the TUI - /// does on every keystroke-completed expression. - pub fn parse(self: *Parser) Error!*Expr { - const expr = try self.parseExpr(.none); - if (self.current.kind != .eof) { - // Trailing tokens: the tree parsed so far is unreachable. - freeExpr(self.allocator, expr); - return Error.UnexpectedToken; + return left; +} + +/// The infix operator the current token introduces, if it introduces one. +fn infixOp(self: Parser) ?InfixOp { + for (infix_table) |entry| { + switch (entry.trigger) { + .kind => |kind| if (kind == self.current.kind) return entry, + .keyword => |name| if (self.current.kind == .identifier and + std.mem.eql(u8, name, self.current.text(self.source))) return entry, } - return expr; } + return null; +} - /// Parse an expression with the given minimum precedence. - fn parseExpr(self: *Parser, min_prec: Prec) Error!*Expr { - if (self.nest_depth >= max_nest_depth) return Error.InvalidExpression; - self.nest_depth += 1; - defer self.nest_depth -= 1; +/// Parse a prefix expression (number, identifier, unary op, parenthesized). +fn parsePrefix(self: *Parser) Error!*Expr { + const tok = self.current; + switch (tok.kind) { + .number => { + self.advance(); + return self.makeNode(.{ .number = .{ + .base = tok.base, + .text = tok.text(self.source), + } }); + }, + .invalid_number => { + return Error.InvalidNumber; + }, + .string_literal => { + self.advance(); + const text = tok.text(self.source); + // The tokenizer only produces this kind with both quotes present, + // so the content is everything between them. + std.debug.assert(text.len >= 2); + return self.makeNode(.{ .string_literal = text[1 .. text.len - 1] }); + }, + .unterminated_string => { + return Error.UnterminatedString; + }, + .identifier => { + self.advance(); + const name = tok.text(self.source); - var left = try self.parsePrefix(); - // Each successful parseInfix returns a node that has adopted `left`, so - // this errdefer always covers the whole tree built so far. - errdefer freeExpr(self.allocator, left); - - while (true) { - const prec = self.infixPrecedence(self.current.kind); - if (@intFromEnum(prec) <= @intFromEnum(min_prec)) break; - - left = try self.parseInfix(left, prec); - } - - return left; - } - - /// Parse a prefix expression (number, identifier, unary op, parenthesized). - fn parsePrefix(self: *Parser) Error!*Expr { - const tok = self.current; - switch (tok.kind) { - .number => { - self.advance(); - return self.makeNode(.{ .number = .{ - .base = tok.base, - .text = tok.text(self.source), - } }); - }, - .invalid_number => { - return Error.InvalidNumber; - }, - .string_literal => { - self.advance(); - const text = tok.text(self.source); - // Strip quotes: 'abc' -> abc - if (text.len < 2) return Error.InvalidNumber; - const content = text[1 .. text.len - 1]; - return self.makeNode(.{ .string_literal = content }); - }, - .identifier => { - self.advance(); - const name = tok.text(self.source); - - // "not" prefix keyword = bitwise NOT - if (std.mem.eql(u8, name, "not")) { - const operand = try self.parseExpr(.unary); - errdefer freeExpr(self.allocator, operand); - return self.makeNode(.{ .unary = .{ - .op = .bitwise_not, - .operand = operand, - } }); - } - - // Check for assignment: identifier = expr - if (self.current.kind == .equals) { - self.advance(); - const value = try self.parseExpr(.none); - errdefer freeExpr(self.allocator, value); - return self.makeNode(.{ .assignment = .{ - .name = name, - .value = value, - } }); - } - - // Check for function call: identifier(args) - if (self.current.kind == .left_paren) { - self.advance(); // consume ( - var args = std.ArrayList(*Expr).empty; - defer args.deinit(self.allocator); - // Arguments parsed before the failure still own their trees. - errdefer for (args.items) |arg| freeExpr(self.allocator, arg); - - if (self.current.kind != .right_paren) { - const first_arg = try self.parseExpr(.none); - args.append(self.allocator, first_arg) catch { - freeExpr(self.allocator, first_arg); - return Error.OutOfMemory; - }; - - while (self.current.kind == .comma) { - self.advance(); // consume , - const arg = try self.parseExpr(.none); - args.append(self.allocator, arg) catch { - freeExpr(self.allocator, arg); - return Error.OutOfMemory; - }; - } - } - - if (self.current.kind != .right_paren) { - return Error.UnmatchedParen; - } - self.advance(); // consume ) - - const args_slice = self.allocator.dupe(*Expr, args.items) catch - return Error.OutOfMemory; - errdefer self.allocator.free(args_slice); - - return self.makeNode(.{ .call = .{ - .name = name, - .args = args_slice, - } }); - } - - // Check for keyword operators (rol, ror) - these are identifiers - // that act as infix operators, handled in parseInfix via infixPrecedence - // Only reach here if it's a plain variable reference. - return self.makeNode(.{ .variable = name }); - }, - .left_paren => { - self.advance(); // consume ( - const inner = try self.parseExpr(.none); - if (self.current.kind != .right_paren) { - freeExpr(self.allocator, inner); - return Error.UnmatchedParen; - } - self.advance(); // consume ) - return inner; - }, - .minus => { - self.advance(); - const operand = try self.parseExpr(.unary); - errdefer freeExpr(self.allocator, operand); - return self.makeNode(.{ .unary = .{ - .op = .negate, - .operand = operand, - } }); - }, - .tilde => { - self.advance(); + // "not" prefix keyword = bitwise NOT + if (std.mem.eql(u8, name, "not")) { const operand = try self.parseExpr(.unary); errdefer freeExpr(self.allocator, operand); return self.makeNode(.{ .unary = .{ .op = .bitwise_not, .operand = operand, } }); - }, - .eof => { - return Error.UnexpectedEnd; - }, - else => { - return Error.UnexpectedToken; - }, - } - } + } - /// Parse an infix expression given the left-hand side and precedence. - fn parseInfix(self: *Parser, left: *Expr, prec: Prec) Error!*Expr { - const tok = self.current; + // Check for function call: identifier(args) + if (self.current.kind == .left_paren) { + self.advance(); // consume ( + var args = std.ArrayList(*Expr).empty; + defer args.deinit(self.allocator); + // Arguments parsed before the failure still own their trees. + errdefer for (args.items) |arg| freeExpr(self.allocator, arg); - // Handle keyword operators (rol, ror, and, or, xor) - if (tok.kind == .identifier) { - const name = tok.text(self.source); - if (keywordBinaryOp(name)) |op| { - self.advance(); - const right = try self.parseExpr(prec); - // `left` belongs to the caller's errdefer until makeNode adopts - // it, so only the right operand is released here. - errdefer freeExpr(self.allocator, right); - return self.makeNode(.{ .binary = .{ - .op = op, - .left = left, - .right = right, + if (self.current.kind != .right_paren) { + const first_arg = try self.parseExpr(.none); + args.append(self.allocator, first_arg) catch { + freeExpr(self.allocator, first_arg); + return Error.OutOfMemory; + }; + + while (self.current.kind == .comma) { + self.advance(); // consume , + const arg = try self.parseExpr(.none); + args.append(self.allocator, arg) catch { + freeExpr(self.allocator, arg); + return Error.OutOfMemory; + }; + } + } + + if (self.current.kind != .right_paren) { + return Error.UnmatchedParen; + } + self.advance(); // consume ) + + const args_slice = self.allocator.dupe(*Expr, args.items) catch + return Error.OutOfMemory; + errdefer self.allocator.free(args_slice); + + return self.makeNode(.{ .call = .{ + .name = name, + .args = args_slice, } }); } - } - self.advance(); - const op = self.tokenToBinaryOp(tok.kind) orelse { + // Check for keyword operators (rol, ror) - these are identifiers + // that act as infix operators, handled in parseInfix via infixPrecedence + // Only reach here if it's a plain variable reference. + return self.makeNode(.{ .variable = name }); + }, + .left_paren => { + self.advance(); // consume ( + const inner = try self.parseExpr(.none); + if (self.current.kind != .right_paren) { + freeExpr(self.allocator, inner); + return Error.UnmatchedParen; + } + self.advance(); // consume ) + return inner; + }, + .minus => { + self.advance(); + const operand = try self.parseExpr(.unary); + errdefer freeExpr(self.allocator, operand); + return self.makeNode(.{ .unary = .{ + .op = .negate, + .operand = operand, + } }); + }, + .tilde => { + self.advance(); + const operand = try self.parseExpr(.unary); + errdefer freeExpr(self.allocator, operand); + return self.makeNode(.{ .unary = .{ + .op = .bitwise_not, + .operand = operand, + } }); + }, + .eof => { + return Error.UnexpectedEnd; + }, + else => { return Error.UnexpectedToken; - }; - - // Right-associative for power - const next_prec: Prec = if (op == .pow) - @enumFromInt(@intFromEnum(prec) - 1) - else - prec; - - const right = try self.parseExpr(next_prec); - errdefer freeExpr(self.allocator, right); - return self.makeNode(.{ .binary = .{ - .op = op, - .left = left, - .right = right, - } }); + }, } +} - /// Map a keyword identifier to a binary operator, if it is one. - fn keywordBinaryOp(name: []const u8) ?BinaryOp { - if (std.mem.eql(u8, name, "rol")) return .rotate_left; - if (std.mem.eql(u8, name, "ror")) return .rotate_right; - if (std.mem.eql(u8, name, "and")) return .bit_and; - if (std.mem.eql(u8, name, "or")) return .bit_or; - if (std.mem.eql(u8, name, "xor")) return .bit_xor; - return null; - } +/// Parse an infix expression, given the left-hand side and the operator the +/// current token introduces. `parseExpr` only calls this with an operator it has +/// already found in the table, so there is no "not an operator" case here. +fn parseInfix(self: *Parser, left: *Expr, entry: InfixOp) Error!*Expr { + self.advance(); - /// Precedence of a keyword infix operator, if the name is one. - fn keywordPrec(name: []const u8) ?Prec { - if (std.mem.eql(u8, name, "rol") or std.mem.eql(u8, name, "ror")) return .shift; - if (std.mem.eql(u8, name, "and")) return .bit_and; - if (std.mem.eql(u8, name, "or")) return .bit_or; - if (std.mem.eql(u8, name, "xor")) return .bit_xor; - return null; - } + // A right-associative operator parses its right side at one level below its + // own, so an operator of equal precedence binds there instead of here. + const next_prec: Prec = switch (entry.assoc) { + .left => entry.prec, + .right => @enumFromInt(@intFromEnum(entry.prec) - 1), + }; - /// Get the infix precedence of a token kind. - fn infixPrecedence(self: *Parser, kind: TokenKind) Prec { - return switch (kind) { - .pipe => .bit_or, - .caret => .power, // always exponentiation - .ampersand => .bit_and, - .shift_left, .shift_right, .shift_right_logical => .shift, - .plus, .minus => .additive, - .star, .slash, .percent => .multiplicative, - .star_star => .power, - .identifier => keywordPrec(self.current.text(self.source)) orelse .none, - else => .none, - }; - } + const right = try self.parseExpr(next_prec); + // `left` belongs to the caller's errdefer until makeNode adopts it, so only + // the right operand is released here. + errdefer freeExpr(self.allocator, right); + return self.makeNode(.{ .binary = .{ + .op = entry.op, + .left = left, + .right = right, + } }); +} - /// Map a token kind to a binary operator. - fn tokenToBinaryOp(self: *Parser, kind: TokenKind) ?BinaryOp { - _ = self; - return switch (kind) { - .plus => .add, - .minus => .sub, - .star => .mul, - .slash => .div, - .percent => .mod, - .caret => .pow, // always exponentiation - .star_star => .pow, - .ampersand => .bit_and, - .pipe => .bit_or, - .shift_left => .shift_left, - .shift_right => .shift_right, - .shift_right_logical => .shift_right_logical, - else => null, - }; - } +fn advance(self: *Parser) void { + self.current = self.tokenizer.next(); +} - fn advance(self: *Parser) void { - self.current = self.tokenizer.next(); +fn makeNode(self: *Parser, expr: Expr) Error!*Expr { + // Budget checked here so every construction site is covered by one test. + if (self.node_count >= max_nodes) { + return Error.ExpressionTooLarge; } - - fn makeNode(self: *Parser, expr: Expr) Error!*Expr { - // Budget checked here so every construction site is covered by one test. - if (self.node_count >= max_nodes) { - return Error.InvalidExpression; - } - self.node_count += 1; - const node = self.allocator.create(Expr) catch return Error.OutOfMemory; - node.* = expr; - return node; - } -}; + self.node_count += 1; + const node = self.allocator.create(Expr) catch return Error.OutOfMemory; + node.* = expr; + return node; +} // -- Tests -- @@ -775,7 +784,7 @@ test "a tree larger than the node budget is rejected, not built" { } var parser = Parser.init(testing.allocator, over.items); - try testing.expectError(Error.InvalidExpression, parser.parse()); + try testing.expectError(Error.ExpressionTooLarge, parser.parse()); // Nothing is left allocated: testing.allocator would report a leak otherwise. } @@ -801,7 +810,7 @@ test "nesting deeper than the depth limit is rejected" { for (0..Parser.max_nest_depth + 10) |_| try deep.append(testing.allocator, ')'); var parser = Parser.init(testing.allocator, deep.items); - try testing.expectError(Error.InvalidExpression, parser.parse()); + try testing.expectError(Error.NestingTooDeep, parser.parse()); } test "nesting within the depth limit parses" { @@ -835,7 +844,7 @@ test "the depth limit also covers nested calls and unary operators" { for (0..Parser.max_nest_depth + 10) |_| try deep.append(testing.allocator, ')'); var parser = Parser.init(testing.allocator, deep.items); - try testing.expectError(Error.InvalidExpression, parser.parse()); + try testing.expectError(Error.NestingTooDeep, parser.parse()); var unary = std.ArrayList(u8).empty; defer unary.deinit(testing.allocator); @@ -843,7 +852,7 @@ test "the depth limit also covers nested calls and unary operators" { try unary.append(testing.allocator, '1'); var unary_parser = Parser.init(testing.allocator, unary.items); - try testing.expectError(Error.InvalidExpression, unary_parser.parse()); + try testing.expectError(Error.NestingTooDeep, unary_parser.parse()); } test "a numeric span that is not a number is an invalid number, not an unexpected token" { @@ -856,3 +865,126 @@ test "a numeric span that is not a number is an invalid number, not an unexpecte var stray = Parser.init(testing.allocator, "1 $ 2"); try testing.expectError(Error.UnexpectedToken, stray.parse()); } + +// -- Assignment is a statement, not an operand -- +// +// It used to be recognised in prefix position, so it could appear wherever an +// operand could: `1 + x = 2` assigned 2 to `x` and evaluated to 3, and `f(x = 1)` +// was an argument list with a side effect. Both are the same bug, and both are now +// parse errors (tasks.md open item 10). + +test "assignment at the top of the input still parses" { + const expr = try testParse("X = 42"); + defer freeExpr(testing.allocator, expr); + try testing.expectEqualStrings("X", expr.assignment.name); + try expectLiteral("42", expr.assignment.value); +} + +test "an assignment inside an expression is an error, not a side effect" { + for ([_][]const u8{ + "1 + x = 2", // the reported case + "f(x = 1)", // an argument that assigns + "(x = 5)", // parenthesised, so still an operand position + "-x = 1", // under a unary + "2 * y = 3", + }) |source| { + var parser = Parser.init(testing.allocator, source); + if (parser.parse()) |expr| { + freeExpr(testing.allocator, expr); + std.debug.print("expected a parse error for \"{s}\"\n", .{source}); + return error.TestUnexpectedResult; + } else |_| {} + } +} + +test "an identifier followed by something other than = is not an assignment" { + // The lookahead must not consume: `x` alone is a variable, and `x + 1` is a + // binary expression whose left side is one. + const variable = try testParse("x"); + defer freeExpr(testing.allocator, variable); + try testing.expectEqualStrings("x", variable.variable); + + const sum = try testParse("x + 1"); + defer freeExpr(testing.allocator, sum); + try testing.expectEqualStrings("x", sum.binary.left.variable); + + const call = try testParse("sin(1)"); + defer freeExpr(testing.allocator, call); + try testing.expectEqualStrings("sin", call.call.name); +} + +test "an unterminated string literal is reported, not silently shortened" { + // `'AB` used to parse as the literal 'A': the tokenizer returned the span + // without its closing quote and the parser stripped the last byte as if it were + // one. + for ([_][]const u8{ "'AB", "'", "1 + 'AB" }) |source| { + var parser = Parser.init(testing.allocator, source); + try testing.expectError(Error.UnterminatedString, parser.parse()); + } + + const terminated = try testParse("'AB'"); + defer freeExpr(testing.allocator, terminated); + try testing.expectEqualStrings("AB", terminated.string_literal); +} + +// -- The infix operator table -- + +test "every table entry parses, with the operator the table names" { + // The four switches this replaced had to agree pairwise. Walking the table + // proves each entry is reachable and lands on its own operator, so an entry + // added with the wrong operator or a missing precedence fails here. + for (infix_table) |entry| { + var source = std.ArrayList(u8).empty; + defer source.deinit(testing.allocator); + try source.appendSlice(testing.allocator, "12 "); + switch (entry.trigger) { + .kind => |kind| try source.appendSlice(testing.allocator, switch (kind) { + .pipe => "|", + .ampersand => "&", + .shift_left => "<<", + .shift_right => ">>", + .shift_right_logical => ">>>", + .plus => "+", + .minus => "-", + .star => "*", + .slash => "/", + .percent => "%", + .caret => "^", + .star_star => "**", + else => unreachable, + }), + .keyword => |name| try source.appendSlice(testing.allocator, name), + } + try source.appendSlice(testing.allocator, " 3"); + + const expr = try testParse(source.items); + defer freeExpr(testing.allocator, expr); + try testing.expectEqual(entry.op, expr.binary.op); + try expectLiteral("12", expr.binary.left); + try expectLiteral("3", expr.binary.right); + } +} + +test "associativity comes from the table" { + // Right for both spellings of power, left for everything else. + const power = try testParse("2^3^2"); + defer freeExpr(testing.allocator, power); + try testing.expectEqual(BinaryOp.pow, power.binary.right.binary.op); + try expectLiteral("2", power.binary.left); + + const stars = try testParse("2**3**2"); + defer freeExpr(testing.allocator, stars); + try testing.expectEqual(BinaryOp.pow, stars.binary.right.binary.op); + + const subtraction = try testParse("9 - 4 - 3"); + defer freeExpr(testing.allocator, subtraction); + try testing.expectEqual(BinaryOp.sub, subtraction.binary.left.binary.op); + try expectLiteral("3", subtraction.binary.right); +} + +test "an identifier that is not a keyword operator does not bind as one" { + // `infixOp` returns null for it, so the precedence climb stops and the trailing + // token is reported rather than absorbed. + var parser = Parser.init(testing.allocator, "1 foo 2"); + try testing.expectError(Error.UnexpectedToken, parser.parse()); +} diff --git a/engine/src/engine.zig b/engine/src/engine.zig index 7f16c0b..57d9678 100644 --- a/engine/src/engine.zig +++ b/engine/src/engine.zig @@ -14,7 +14,7 @@ pub const number = @import("number.zig"); // Language layer. pub const tokenizer = @import("tokenizer.zig"); pub const ast = @import("ast.zig"); -pub const parser = @import("parser.zig"); +pub const Parser = @import("Parser.zig"); pub const evaluator = @import("evaluator.zig"); // Fixed-width integer operations, shared by both modes. pub const bitwise = @import("bitwise.zig"); @@ -73,7 +73,9 @@ pub fn phrase(err: Error) []const u8 { error.UnexpectedToken => "unexpected token", error.UnmatchedParen => "unmatched parenthesis", error.UnexpectedEnd => "unexpected end of expression", - error.InvalidExpression => "invalid expression", + error.ExpressionTooLarge => "the expression has too many terms", + error.NestingTooDeep => "the expression is nested too deeply", + error.UnterminatedString => "unterminated string literal", error.InvalidNumber => "invalid number", // Names @@ -150,7 +152,7 @@ test "the error set is derived from the modules, not enumerated here" { const promoted: Error = @field(Error, field.name); try testing.expect(phrase(promoted).len > 0); } - inline for (@typeInfo(parser.Error).error_set.?) |field| { + inline for (@typeInfo(Parser.Error).error_set.?) |field| { const promoted: Error = @field(Error, field.name); try testing.expect(phrase(promoted).len > 0); } @@ -173,7 +175,7 @@ test "phrase is usable at comptime, which is how frontends decorate it" { } test "the cases the TUI table used to lose" { - try testing.expectEqualStrings("invalid expression", phrase(error.InvalidExpression)); + try testing.expectEqualStrings("the expression has too many terms", phrase(error.ExpressionTooLarge)); try testing.expectEqualStrings("no solution found", phrase(error.ConvergenceFailure)); try testing.expectEqualStrings( "these values do not determine an answer", diff --git a/engine/src/evaluator.zig b/engine/src/evaluator.zig index eac1c12..66d9400 100644 --- a/engine/src/evaluator.zig +++ b/engine/src/evaluator.zig @@ -23,9 +23,8 @@ pub const Error = error{ UnknownVariable, DomainError, Overflow, -} || parser_mod.Error || number_mod.Error || bitwise.Error || financial.Error; -const parser_mod = @import("parser.zig"); -const Parser = parser_mod.Parser; +} || Parser.Error || number_mod.Error || bitwise.Error || financial.Error; +const Parser = @import("Parser.zig"); const number_mod = @import("number.zig"); const Number = number_mod.Number; const bitwise = @import("bitwise.zig"); @@ -529,7 +528,7 @@ pub fn evalStringInfo(env: *Environment, allocator: Allocator, source: []const u // scratch arena), so it can be released as soon as evaluation is done. // Without this every evaluated expression leaked its whole AST, which only // went unnoticed because the CLI hands in an arena. - defer parser_mod.freeExpr(allocator, expr); + defer Parser.freeExpr(allocator, expr); // Intermediates live in a scratch arena; only the final value is copied out // into the caller's allocator. diff --git a/engine/src/programmer.zig b/engine/src/programmer.zig index 66e937f..c895cfc 100644 --- a/engine/src/programmer.zig +++ b/engine/src/programmer.zig @@ -13,8 +13,7 @@ const BinaryOp = ast.BinaryOp; const Integer = @import("Integer.zig"); const BitWidth = Integer.BitWidth; const Signedness = Integer.Signedness; -const parser_mod = @import("parser.zig"); -const Parser = parser_mod.Parser; +const Parser = @import("Parser.zig"); const bitwise = @import("bitwise.zig"); const number_mod = @import("number.zig"); const Number = number_mod.Number; @@ -30,7 +29,7 @@ pub const Error = error{ Overflow, UnknownFunction, UnknownVariable, -} || parser_mod.Error || bitwise.Error || number_mod.Error; +} || Parser.Error || bitwise.Error || number_mod.Error; /// What programmer mode needs to know: the integer type to compute in, and one /// display preference that does not affect arithmetic. @@ -175,7 +174,7 @@ pub fn evalProgrammerString(allocator: Allocator, source: []const u8, config: Co const expr = try p.parse(); // Same ownership rule as evalStringInfo: the tree is ours to release, and the // returned Integer does not borrow from it. - defer parser_mod.freeExpr(allocator, expr); + defer Parser.freeExpr(allocator, expr); return evalProgrammer(allocator, config, expr); } diff --git a/engine/src/tokenizer.zig b/engine/src/tokenizer.zig index b247784..103ecc6 100644 --- a/engine/src/tokenizer.zig +++ b/engine/src/tokenizer.zig @@ -104,6 +104,9 @@ pub const TokenKind = enum { invalid_number, // String literal (single-quoted, for ASCII byte packing in programmer mode) string_literal, + /// An opening quote with no closing one. Same reasoning as `invalid_number`: + /// the scan knows, and nothing downstream can tell without re-lexing. + unterminated_string, }; pub const Token = struct { @@ -392,9 +395,14 @@ pub const Tokenizer = struct { while (self.pos < self.source.len and self.source[self.pos] != '\'') { self.pos += 1; } - if (self.pos < self.source.len) { - self.pos += 1; // consume closing quote + if (self.pos == self.source.len) { + // No closing quote. Reported here because this is where it is known: + // the parser strips the first and last byte to get the content, so a + // span returned as a literal without its terminator lost a real + // character instead ("'AB" evaluated as 'A'). + return .{ .kind = .unterminated_string, .start = start, .len = self.pos - start }; } + self.pos += 1; // consume closing quote return .{ .kind = .string_literal, .start = start, .len = self.pos - start }; } diff --git a/src/tui.zig b/src/tui.zig index 0d4b2cc..eb6d38c 100644 --- a/src/tui.zig +++ b/src/tui.zig @@ -1436,7 +1436,7 @@ pub fn drawHistory(items: []const App.HistoryEntry, surface: *vxfw.Surface, star /// /// The phrases live once, in `engine.phrase`; the prefix is added at /// comptime. This switch used to be a second full copy of the CLI's, and had fallen -/// behind: `InsufficientParameters`, `ConvergenceFailure` and `InvalidExpression` +/// behind: `InsufficientParameters`, `ConvergenceFailure` and the parser's limit errors /// all came out as "evaluation error". fn errorStr(err: engine.Error) []const u8 { return switch (err) {