human review Parser.zig

.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`
+level: `Integer.zig`, `Rational.zig` and `Parser.zig`. `number.zig` stays lowercase
-is a tagged union and a Zig file is always a struct, so the file cannot be that type
+because `Number` is a tagged union and a Zig file is always a struct, so the file
-without wrapping the union in a field, which would add a hop at every use of the tag
+cannot be that type without wrapping the union in a field, which would add a hop at
-that is the type's whole identity.
+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

.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
+10. ~~Assignment parses in prefix position (`1 + x = 2` mutates `x`)~~, fixed by Task
-    to a constant name is silently discarded.
+    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.

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,36 +67,79 @@ const Prec = enum(u8) {
     call = 10, // function calls
 };
 
-pub const Parser = struct {
+/// One infix operator: what introduces it, what it means, how tightly it binds and
-    source: []const u8,
+/// which way it associates.
-    tokenizer: Tokenizer,
+const InfixOp = struct {
-    current: Token,
+    trigger: union(enum) {
-    allocator: Allocator,
+        /// Punctuation, recognised by token kind.
-    /// Nodes built so far, checked against `max_nodes`.
+        kind: TokenKind,
-    node_count: usize,
+        /// A keyword operator, recognised by an identifier's text.
-    /// Current parseExpr/parsePrefix nesting, checked against `max_nest_depth`.
+        keyword: []const u8,
-    nest_depth: usize,
+    },
+    op: BinaryOp,
+    prec: Prec,
+    assoc: enum { left, right } = .left,
+};
 
-    /// Ceiling on tree size.
+/// The infix operators, in one table.
-    ///
+///
-    /// This is a stack-safety limit, not a style preference. Everything that walks
+/// This was four switches that had to agree pairwise: two over token kinds (kind to
-    /// a tree recurses on it (`evalExact`, `freeExpr`, `hasNonDecimalLiteral`), and
+/// operator, kind to precedence) and two over keyword names, plus an inline
-    /// a flat chain like `1+1+1+...` builds a left-deep tree whose depth equals the
+/// `if (op == .pow)` for right-associativity. A token kind with a precedence but no
-    /// number of operators. Measured on this build: 3000 terms evaluated fine and
+/// operator would advance past the token and then report "unexpected token", and a
-    /// 4000 terms segfaulted in `evalExact`, from about 8 KB of input. A thousand
+/// keyword added to one switch but not the other would silently bind at the wrong
-    /// nodes is far more than any hand-written expression and leaves a wide margin.
+/// level. Written once, none of that is expressible.
-    pub const max_nodes: usize = 1000;
+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.
+source: []const u8,
-    /// Reached long before `max_nodes` by input like `((((...1...))))`.
+tokenizer: Tokenizer,
-    pub const max_nest_depth: usize = 128;
+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
+/// Ceiling on tree size.
-    /// 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
+/// This is a stack-safety limit, not a style preference. Everything that walks
-    /// `error_pos`; nothing ever read any of them. Errors are reported by returning
+/// a tree recurses on it (`evalExact`, `freeExpr`, `hasNonDecimalLiteral`), and
-    /// them, not by leaving a flag behind.
+/// a flat chain like `1+1+1+...` builds a left-deep tree whose depth equals the
-    pub fn init(allocator: Allocator, source: []const u8) Parser {
+/// 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 .{
@@ -96,28 +150,53 @@ pub const Parser = struct {
         .node_count = 0,
         .nest_depth = 0,
     };
-    }
+}
 
-    /// Parse a complete expression. Returns error if parsing fails.
+/// Parse a complete expression. Returns error if parsing fails.
-    ///
+///
-    /// On success the caller owns the tree and must release it with `freeExpr`.
+/// 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
+/// 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
+/// 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
+/// interactive session leaks the partial tree, which is exactly what the TUI
-    /// does on every keystroke-completed expression.
+/// does on every keystroke-completed expression.
-    pub fn parse(self: *Parser) Error!*Expr {
+pub fn parse(self: *Parser) Error!*Expr {
-        const expr = try self.parseExpr(.none);
+    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;
-    }
+}
 
-    /// Parse an expression with the given minimum precedence.
+/// A whole input: `name = expr`, or an expression.
-    fn parseExpr(self: *Parser, min_prec: Prec) Error!*Expr {
+///
-        if (self.nest_depth >= max_nest_depth) return Error.InvalidExpression;
+/// 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;
 
@@ -126,18 +205,28 @@ pub const Parser = struct {
     // this errdefer always covers the whole tree built so far.
     errdefer freeExpr(self.allocator, left);
 
-        while (true) {
+    while (self.infixOp()) |entry| {
-            const prec = self.infixPrecedence(self.current.kind);
+        if (@intFromEnum(entry.prec) <= @intFromEnum(min_prec)) break;
-            if (@intFromEnum(prec) <= @intFromEnum(min_prec)) break;
+        left = try self.parseInfix(left, entry);
-
-            left = try self.parseInfix(left, prec);
     }
 
     return left;
-    }
+}
 
-    /// Parse a prefix expression (number, identifier, unary op, parenthesized).
+/// The infix operator the current token introduces, if it introduces one.
-    fn parsePrefix(self: *Parser) Error!*Expr {
+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 null;
+}
+
+/// Parse a prefix expression (number, identifier, unary op, parenthesized).
+fn parsePrefix(self: *Parser) Error!*Expr {
     const tok = self.current;
     switch (tok.kind) {
         .number => {
@@ -153,10 +242,13 @@ pub const Parser = struct {
         .string_literal => {
             self.advance();
             const text = tok.text(self.source);
-                // Strip quotes: 'abc' -> abc
+            // The tokenizer only produces this kind with both quotes present,
-                if (text.len < 2) return Error.InvalidNumber;
+            // so the content is everything between them.
-                const content = text[1 .. text.len - 1];
+            std.debug.assert(text.len >= 2);
-                return self.makeNode(.{ .string_literal = content });
+            return self.makeNode(.{ .string_literal = text[1 .. text.len - 1] });
+        },
+        .unterminated_string => {
+            return Error.UnterminatedString;
         },
         .identifier => {
             self.advance();
@@ -172,17 +264,6 @@ pub const Parser = struct {
                 } });
             }
 
-                // 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 (
@@ -263,118 +344,46 @@ pub const Parser = struct {
             return Error.UnexpectedToken;
         },
     }
-    }
+}
 
-    /// Parse an infix expression given the left-hand side and precedence.
+/// Parse an infix expression, given the left-hand side and the operator the
-    fn parseInfix(self: *Parser, left: *Expr, prec: Prec) Error!*Expr {
+/// current token introduces. `parseExpr` only calls this with an operator it has
-        const tok = self.current;
+/// already found in the table, so there is no "not an operator" case here.
-
+fn parseInfix(self: *Parser, left: *Expr, entry: InfixOp) Error!*Expr {
-        // 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,
-                } });
-            }
-        }
 
-        self.advance();
+    // A right-associative operator parses its right side at one level below its
-        const op = self.tokenToBinaryOp(tok.kind) orelse {
+    // own, so an operator of equal precedence binds there instead of here.
-            return Error.UnexpectedToken;
+    const next_prec: Prec = switch (entry.assoc) {
+        .left => entry.prec,
+        .right => @enumFromInt(@intFromEnum(entry.prec) - 1),
     };
 
-        // Right-associative for power
-        const next_prec: Prec = if (op == .pow)
-            @enumFromInt(@intFromEnum(prec) - 1)
-        else
-            prec;
-
     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 = op,
+        .op = entry.op,
         .left = left,
         .right = right,
     } });
-    }
+}
 
-    /// Map a keyword identifier to a binary operator, if it is one.
+fn advance(self: *Parser) void {
-    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;
-    }
-
-    /// 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;
-    }
-
-    /// 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,
-        };
-    }
-
-    /// 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 makeNode(self: *Parser, expr: Expr) Error!*Expr {
+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;
+        return Error.ExpressionTooLarge;
     }
     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());
+}

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",

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;
+} || Parser.Error || number_mod.Error || bitwise.Error || financial.Error;
-const parser_mod = @import("parser.zig");
+const Parser = @import("Parser.zig");
-const Parser = parser_mod.Parser;
 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.

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 = @import("Parser.zig");
-const Parser = parser_mod.Parser;
 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);
 }
 

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) {
+        if (self.pos == self.source.len) {
-            self.pos += 1; // consume closing quote
+            // 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 };
     }
 

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) {