diff --git a/openspec/changes/archive/2026-05-28-add-builtin-constants-to-v2-calculator/.openspec.yaml b/openspec/changes/archive/2026-05-28-add-builtin-constants-to-v2-calculator/.openspec.yaml new file mode 100644 index 0000000..352690f --- /dev/null +++ b/openspec/changes/archive/2026-05-28-add-builtin-constants-to-v2-calculator/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-05-28 diff --git a/openspec/changes/archive/2026-05-28-add-builtin-constants-to-v2-calculator/design.md b/openspec/changes/archive/2026-05-28-add-builtin-constants-to-v2-calculator/design.md new file mode 100644 index 0000000..761bcaf --- /dev/null +++ b/openspec/changes/archive/2026-05-28-add-builtin-constants-to-v2-calculator/design.md @@ -0,0 +1,105 @@ +## Context + +The v2-calculator's `LOAD_VAR` opcode resolves names **lazily** against the runtime `Environment` (`src/v2-calculator/vm/run.ts:23-28`). Identifiers are scanned by the `ident` grammar rule (`src/v2-calculator/parser/grammar.ohm:31`) and emitted by the compiler as `LOAD_VAR { index: nameIndex(name) }` with no awareness of which names are "built-in". That late-bound architecture is the controlling fact for every decision below: it gives us a free seam to inject defaults into the runtime layer without touching any other layer. + +The `Environment` class itself (`src/v2-calculator/vm/Environment.ts`) is a single-scope `Map` with `get` (throws `UndefinedVariable` on miss) and `set` (create-or-overwrite). It was deliberately minimal in the variables change — the smallest learnable subset that demonstrates the idea once. Built-in constants extend that minimum the same way the user's own assignments do: through the existing `set` path, just at construction time instead of at user-code time. + +## Goals / Non-Goals + +**Goals:** + +- `pi`, `e`, and `tau` resolve to the mathematically correct double-precision values without any user setup, in every fresh `Environment`. +- Built-ins are reassignable — `pi = 5` silently shadows, matching the calculator's existing "no `let`, implicit reassignment" semantics for user variables. +- Implementation touches **only** the runtime layer (`vm/`). Grammar, parser, semantics, AST, compiler, bytecode, public entry point, and REPL are all unchanged. +- The built-in registry is a **single discoverable module** — `vm/builtins.ts` is the answer to "what names does the language pre-bind?" today, and is positioned to absorb future additions. +- The design does not foreclose any reasonable mechanism for built-in **functions** (the next todo item) — it leaves that decision to the change that adds them. + +**Non-Goals:** + +- Built-in functions (`sin`, `cos`, `tan`, …) — separate todo, separate change. The `Environment`'s value type (`number`) is not widened here. +- IEEE-754 named constants (`inf`, `nan`, `epsilon`) — separate todo, captured during brainstorming. +- A dedicated opcode for built-in access (Ball's `OpGetBuiltin` pattern from _Writing A Compiler In Go_). +- Compile-time detection that an identifier resolves to a built-in. +- Protecting built-in slots from reassignment. +- A constructor flag for "vanilla" (empty) `Environment` for tests — YAGNI; tests can `set` over the defaults if they truly need to. + +## Decisions + +### Decision 1: Built-ins are pre-populated bindings in `Environment`, not a separate opcode + +The `Environment` constructor pre-fills its existing `Map` from a frozen `BUILTIN_CONSTANTS` table. The runtime path is the existing `LOAD_VAR → env.get(name)`. Built-ins are indistinguishable from user variables once they're in the Map. + +```ts +// src/v2-calculator/vm/builtins.ts +export const BUILTIN_CONSTANTS: Readonly> = Object.freeze({ + pi: Math.PI, + e: Math.E, + tau: 2 * Math.PI, +}); + +// src/v2-calculator/vm/Environment.ts +constructor() { + for (const [name, value] of Object.entries(BUILTIN_CONSTANTS)) { + this.records.set(name, value); + } +} +``` + +Consequences: + +- `pi` already matches the `ident` grammar rule, so the parser and compiler need no changes. +- A fresh `new Environment()` returns `Math.PI` from `get("pi")` immediately. +- `env.set("pi", 5)` overwrites the built-in. Reassignment semantics fall out for free — this is the existing `set` behavior, no special case. +- The compiler still records `["pi"]` in `bytecode.names` and emits `LOAD_VAR 0` for a read of `pi`, exactly as it would for a user variable. Disassembled bytecode remains readable. + +**Alternatives considered:** + +- _Ball's `OpGetBuiltin` opcode pattern_ (_Writing A Compiler In Go_, ch. on Built-in Functions) — **rejected**. Ball uses a dedicated opcode because his built-ins are _functions_ whose dispatch shape differs from variable reads (they need arity and native dispatch). For numeric constants, the dispatch is _identical_ to user-variable reads, so a separate opcode would be unmotivated ceremony. When the next change introduces built-in functions, `OpGetBuiltin` can be revisited on its merits then — it does not need to be foreshadowed here, and prematurely adopting it would couple constants to a function-dispatch story they don't need. +- _Compile-time inlining: emit `LOAD_CONST Math.PI` whenever the compiler sees `pi`_ — **rejected**. (a) Loses symbolic visibility in disassembly (no `LOAD_VAR` shows `pi`). (b) Requires special-casing in the compiler that would have to be undone when built-in _functions_ arrive (functions cannot be inlined as `LOAD_CONST`). (c) Interacts awkwardly with the chosen "reassignable" semantics — what does the compiler emit for `pi` after a `pi = 5` assignment in the same program? Either the compiler tracks "shadowed" state across the program (complex), or it always emits `LOAD_VAR pi` after seeing any `Assign` to `pi` (special-case), or it always emits `LOAD_VAR pi` and the inlining is only "fast-path" (now we have two paths to maintain). The conceptual model becomes harder than the implementation it tried to optimize. +- _Falling back inside `Environment.get` to a separate built-ins map when the user-map misses_ — **rejected**. Saves the constructor loop but at the cost of two-stage lookup logic on every read, and pushes "knowledge of built-ins" into `get`'s control flow rather than into construction-time data. The constructor loop is cheaper and conceptually simpler ("the env is one Map; built-ins are there from the start"). + +### Decision 2: `BUILTIN_CONSTANTS` lives in its own module, frozen + +The constants table is exported from `src/v2-calculator/vm/builtins.ts` rather than declared as a private static field inside `Environment`. The object is `Object.freeze`d at definition time. + +Why a separate module: + +- **Discoverability.** A future contributor (or future-you) asking "what does the calculator pre-bind?" greps `BUILTIN_CONSTANTS` and finds _the_ answer in one file. Adding `phi` later is a one-line change in one place. +- **Independence of mechanism from policy.** `Environment` is _mechanism_ (slot storage with get/set semantics). `builtins.ts` is _policy_ (which names exist and what their values are). The two will drift independently over the calculator's lifetime; keeping them in separate files lets each evolve without colliding. +- **Test imports.** `Environment.test.ts` can import `BUILTIN_CONSTANTS` and iterate it to confirm every entry resolves, rather than enumerating names twice (once in the constants file, once in the test). +- **Forward shape.** When `sin`, `cos`, … arrive in a later change, `builtins.ts` is the natural home for a second export (whether that's `BUILTIN_FUNCTIONS`, a widened-value union, or whatever the function change decides). The discovery point is already in place. + +Why `Object.freeze`: + +- `BUILTIN_CONSTANTS` is intended as read-only data; freezing turns a comment into a hard invariant. +- Cheap insurance against accidental mutation in tests or in some future "let's just hot-patch the constants for this experiment" code path. (Mutating the table would corrupt every subsequent `Environment` constructed in that process.) + +**Alternatives considered:** + +- _Declare the constants inline in `Environment.ts`_ — **rejected**. Couples the storage class to the policy of which names are pre-bound. Makes the next iteration push more of "what's pre-bound" into `Environment.ts`, which would grow into a god-file. +- _Pass the constants map into `Environment`'s constructor as a parameter_ — **rejected**. Every caller (REPL, `index.ts`, tests) would then need to know about the constants registry, leaking policy across the codebase. The point of pre-population is precisely that callers _don't_ need to think about it. If a caller ever does need a vanilla env, that's a one-line override worth designing then. + +### Decision 3: Reassignment overwrites silently — no protected slots, no warnings + +Per the brainstorming decision, `pi = 5` is legal and silently overwrites the built-in for the lifetime of that `Environment`. No new error, no warning channel, no compile-time check. + +This matches the calculator's existing semantics for _all_ names: no `let` keyword, no `const` keyword, no notion of "protected" identifiers anywhere else. Treating built-ins differently would be the surprise; treating them as ordinary default bindings is the consistency. + +Reassignment in one `Environment` does NOT affect any other `Environment` — each instance pre-populates independently in its constructor, so `pi` is restored to `Math.PI` in every freshly-constructed env. This is a natural consequence of pre-populating in the constructor rather than (for example) sharing a single mutable defaults object across instances. + +**Alternatives considered:** + +- _Reject `pi = 5` at compile time_ — **rejected**. Requires the compiler to consult `BUILTIN_CONSTANTS`, which couples a layer that currently doesn't know about it. Also forces a hard binary choice that's inconsistent with how every other name behaves. +- _Reject `pi = 5` at runtime in `Environment.set`_ — **rejected**. Same problem in a different layer, and worse: now the calculator has _two_ runtime errors for assignment (one for built-ins, the existing-but-absent "you cannot assign that"). The cost-to-benefit of protecting three names is wrong. +- _Warn on shadowing_ — **rejected**. The REPL has no warning channel; `console.warn` mid-evaluation would interleave with output and confuse the prompt. Worth revisiting if/when a richer diagnostics infrastructure arrives. + +## Risks / Trade-offs + +- **Every `new Environment()` runs a small constructor-time loop over the constants table.** → Loop is O(n) where n is fixed at 3 (and almost certainly <20 even after future additions). Pre-population is one-shot per `Environment` instance; the REPL constructs exactly one for the entire session. Cost is negligible and never on a hot path. +- **The existing scenario titled "A new Environment starts empty" becomes slightly misleading.** → The scenario's body tests resolution of `"x"`, which is _not_ a built-in, so the behavior the scenario asserts (`UndefinedVariable` on unbound name) is unchanged. The scenario remains valid. The spec delta leaves it in place rather than renaming, because renaming would require a MODIFIED block whose only delta is a title change — disproportionate. +- **A future test that imports `BUILTIN_CONSTANTS` and iterates it could become brittle if `Object.freeze` is removed.** → We document the frozen invariant in the spec; if a future change removes the freeze, the spec change captures the new contract. +- **Built-in functions (next todo) cannot be expressed by widening `BUILTIN_CONSTANTS` alone.** → Acknowledged and explicitly out of scope. Decision 1 deliberately preserves that optionality — the next change will add either a sibling registry (`BUILTIN_FUNCTIONS`) or a different opcode (Ball's `OpGetBuiltin`), and either is consistent with this design. + +## Open Questions + +None at this scope. The follow-ups — IEEE-754 names (`inf`, `nan`) and built-in functions (`sin`, `cos`, …) — are each scoped to their own todo items and own changes. diff --git a/openspec/changes/archive/2026-05-28-add-builtin-constants-to-v2-calculator/proposal.md b/openspec/changes/archive/2026-05-28-add-builtin-constants-to-v2-calculator/proposal.md new file mode 100644 index 0000000..fbcaa62 --- /dev/null +++ b/openspec/changes/archive/2026-05-28-add-builtin-constants-to-v2-calculator/proposal.md @@ -0,0 +1,42 @@ +## Why + +The v2-calculator's `pi` is currently an undefined variable. A user typing `pi`, `e`, or `tau` at the REPL gets `UndefinedVariable: pi` — a hostile experience for a _math_ calculator. Pre-binding a small set of well-known mathematical constants turns the calculator into something you can use without first defining your own constants every session. + +Now that variables exist, every name lookup already goes through `Environment` (`LOAD_VAR` resolves names lazily at runtime — `src/v2-calculator/vm/run.ts:23-28`). That late-bound lookup gives us a free seam to inject built-ins: pre-populating the `Environment` requires no opcode, no grammar, no compiler, and no AST changes. The smallest possible change is also the right change. + +## What Changes + +- **New module** `src/v2-calculator/vm/builtins.ts` exports a frozen `BUILTIN_CONSTANTS` map of `{ pi: Math.PI, e: Math.E, tau: 2 * Math.PI }`. Single source of truth for "what names does the language pre-bind?", positioned to absorb future entries (`phi`, `inf`, `nan`, …) and eventually built-in functions. +- **`Environment` constructor** pre-populates from `BUILTIN_CONSTANTS`. A fresh `new Environment()` already contains `pi`, `e`, `tau` — no caller change needed in `index.ts`, `cli.ts`, or `run.ts`. +- **Reassignment semantics** — built-ins are ordinary default bindings, not protected slots. `pi = 5` overwrites the binding silently, exactly as user-defined variables do. No new error, no warning. +- **Zero changes** to `grammar.ohm`, `ast.ts`, `semantics.ts`, `parser.ts`, `compiler.ts`, `bytecode.ts`, `run.ts`, `index.ts`, or `cli.ts`. The existing `LOAD_VAR` / `STORE_VAR` paths handle these reads and writes correctly. +- **Tests** in `vm/Environment.test.ts` (fresh-env lookups, shadowing, no regression on undefined-name throw) and `calculator.test.ts` (end-to-end `run("pi")`, `run("2 * pi")`, `run("tau / 2")`, shadowing through the full pipeline). +- **`todo.md`** — tick item 4 (`Add built-in constants, like pi`) and add a follow-up `Add IEEE-754 named constants: inf, nan` adjacent to it. + +## Capabilities + +### New Capabilities + + + +### Modified Capabilities + +- `v2-calculator`: `Environment` gains pre-populated bindings for the built-in mathematical constants `pi`, `e`, `tau`. A new requirement covers the existence, identity, and shadowing semantics of these built-ins. + +## Out of Scope (Deferred) + +- **`inf`, `nan`, `epsilon`.** Captured as a separate todo item. Surfacing IEEE-754 sentinels as user-visible names deserves its own small design discussion (naming, whether `nan == nan`, error-vs-value role) which would clutter this change. +- **Built-in functions (`sin`, `cos`, `tan`, …).** Already a separate todo item. The `Environment` value type is currently `number`; functions will need either a widened value type or Ball's `OpGetBuiltin` pattern (see _Writing A Compiler In Go_, ch. on Built-in Functions). This change deliberately does not prejudge that decision. +- **Compile-time shadowing detection or warnings.** The chosen semantics are "reassignable, no warning", matching the calculator's existing implicit-reassignment stance for user variables. +- **Marking built-ins as `const` / preventing reassignment at runtime.** Same rationale — and the calculator has no `const`-vs-mutable concept anywhere else. +- **Separate `Environment` constructor variant for a "vanilla" empty env.** Tests that need an empty environment can use `set` to overwrite, or the design can grow a flag later. YAGNI for now. + +## Impact + +- `src/v2-calculator/vm/builtins.ts` (NEW): frozen `Readonly>` named `BUILTIN_CONSTANTS` with the three initial entries. +- `src/v2-calculator/vm/Environment.ts`: constructor pre-populates the internal `Map` from `BUILTIN_CONSTANTS` via `Object.entries`. No method signatures change. +- `src/v2-calculator/vm/Environment.test.ts`: new cases for fresh-env built-in resolution, user shadowing, and a confirmation that `UndefinedVariable` still throws for non-built-in names. +- `src/v2-calculator/calculator.test.ts`: new end-to-end cases covering `pi`, `e`, `tau` evaluation through the full parse → compile → run pipeline, plus REPL-like shadowing across two `run` calls sharing one `Environment`. +- `src/v2-calculator/todo.md`: item 4 ticked; new follow-up item added beneath it. +- `src/v2-calculator/parser/*`, `src/v2-calculator/compiler/*`, `src/v2-calculator/vm/run.ts`, `src/v2-calculator/index.ts`, `src/v2-calculator/cli.ts`: **unchanged**. This is the design's main virtue — the change rides entirely on existing infrastructure. +- After approval + merge: sync delta into `openspec/specs/v2-calculator/spec.md` via the `openspec-sync-specs` skill. diff --git a/openspec/changes/archive/2026-05-28-add-builtin-constants-to-v2-calculator/specs/v2-calculator/spec.md b/openspec/changes/archive/2026-05-28-add-builtin-constants-to-v2-calculator/specs/v2-calculator/spec.md new file mode 100644 index 0000000..c9fcb40 --- /dev/null +++ b/openspec/changes/archive/2026-05-28-add-builtin-constants-to-v2-calculator/specs/v2-calculator/spec.md @@ -0,0 +1,88 @@ +## ADDED Requirements + +### Requirement: Mathematical built-in constants are pre-bound in every Environment + +The file `src/v2-calculator/vm/builtins.ts` SHALL export a frozen `BUILTIN_CONSTANTS` constant of type `Readonly>` containing exactly three entries: + +```ts +export const BUILTIN_CONSTANTS: Readonly> = + Object.freeze({ + pi: Math.PI, + e: Math.E, + tau: 2 * Math.PI, + }); +``` + +The `BUILTIN_CONSTANTS` object SHALL be frozen via `Object.freeze` so that mutation attempts are rejected (silently in non-strict mode, with a `TypeError` in strict mode). The keys SHALL be the complete, ordered set of pre-bound names; no other module SHALL inject additional defaults into a fresh `Environment`. + +Every newly-constructed `Environment` (`src/v2-calculator/vm/Environment.ts`) SHALL pre-populate its internal storage from `BUILTIN_CONSTANTS` during construction so that, immediately after construction, `env.get("pi")`, `env.get("e")`, and `env.get("tau")` return `Math.PI`, `Math.E`, and `2 * Math.PI` respectively without throwing. + +Built-in constants SHALL be ordinary bindings — they participate in `Environment.set` exactly like user variables. A user assignment such as `pi = 5` SHALL overwrite the built-in binding for the lifetime of that `Environment` instance. A second `Environment` SHALL NOT be affected by reassignments performed in the first; each instance pre-populates independently in its own constructor. + +The parser, compiler, AST, and bytecode layers SHALL NOT be aware that any identifier is a built-in. Resolution SHALL happen exclusively at runtime through the existing `LOAD_VAR` opcode and `Environment.get` lookup path. A read of a built-in identifier SHALL still be compiled as `LOAD_VAR { index }` against the `bytecode.names` pool, indistinguishable from a read of a user-defined name. + +#### Scenario: A fresh Environment resolves pi to Math.PI + +- **WHEN** a new `Environment` is constructed +- **AND** `env.get("pi")` is called +- **THEN** the returned value is exactly `Math.PI` + +#### Scenario: A fresh Environment resolves e to Math.E + +- **WHEN** a new `Environment` is constructed +- **AND** `env.get("e")` is called +- **THEN** the returned value is exactly `Math.E` + +#### Scenario: A fresh Environment resolves tau to 2 \* Math.PI + +- **WHEN** a new `Environment` is constructed +- **AND** `env.get("tau")` is called +- **THEN** the returned value is exactly `2 * Math.PI` + +#### Scenario: Built-ins evaluate end-to-end through the public API + +- **WHEN** `run("pi")` is called with no shared `Environment` +- **THEN** the result is exactly `Math.PI` + +#### Scenario: Built-ins compose with arithmetic + +- **WHEN** `run("2 * pi")` is called with no shared `Environment` +- **THEN** the result is exactly `2 * Math.PI` + +#### Scenario: Tau equals two pi end-to-end + +- **WHEN** `run("tau / 2")` is called with no shared `Environment` +- **THEN** the result is exactly `Math.PI` + +#### Scenario: User assignment shadows a built-in within the same Environment + +- **WHEN** a fresh `Environment` is constructed +- **AND** `env.set("pi", 5)` is called +- **AND** `env.get("pi")` is then called +- **THEN** the returned value is `5` + +#### Scenario: Shadowing is local to its Environment instance + +- **WHEN** an `Environment` `envA` has `pi` reassigned to `5` via `set` +- **AND** a second fresh `Environment` `envB` is then constructed +- **AND** `envB.get("pi")` is called +- **THEN** the returned value is exactly `Math.PI` + +#### Scenario: Shadowing roundtrips through the full pipeline + +- **WHEN** an `Environment` is constructed +- **AND** `run("pi = 5\npi", env)` is called with that `env` +- **THEN** the result is `5` + +#### Scenario: Names outside the built-in set still throw UndefinedVariable + +- **WHEN** a fresh `Environment` is constructed +- **AND** `env.get("phi")` is called +- **THEN** an `UndefinedVariable` error is thrown +- **AND** the thrown error's `name` property is `"phi"` + +#### Scenario: BUILTIN_CONSTANTS is frozen at runtime + +- **WHEN** code attempts to mutate `BUILTIN_CONSTANTS` (e.g. `(BUILTIN_CONSTANTS as Record).foo = 1`) +- **THEN** the mutation does not take effect +- **AND** `BUILTIN_CONSTANTS.foo` is `undefined` after the attempted write diff --git a/openspec/changes/archive/2026-05-28-add-builtin-constants-to-v2-calculator/tasks.md b/openspec/changes/archive/2026-05-28-add-builtin-constants-to-v2-calculator/tasks.md new file mode 100644 index 0000000..137cf13 --- /dev/null +++ b/openspec/changes/archive/2026-05-28-add-builtin-constants-to-v2-calculator/tasks.md @@ -0,0 +1,45 @@ +## 1. Built-ins Module + +- [x] 1.1 Create `src/v2-calculator/vm/builtins.ts` exporting `BUILTIN_CONSTANTS` typed as `Readonly>` and wrapped in `Object.freeze`. Initial entries: `pi: Math.PI`, `e: Math.E`, `tau: 2 * Math.PI`. No other exports. +- [x] 1.2 Add a one-line TS doc comment on the export explaining that this is the canonical registry of names pre-bound in every fresh `Environment` (see `design.md` Decision 2). + +## 2. Environment Pre-population + +- [x] 2.1 In `src/v2-calculator/vm/Environment.ts`: import `BUILTIN_CONSTANTS` from `./builtins`. +- [x] 2.2 Add a constructor to `Environment` that iterates `Object.entries(BUILTIN_CONSTANTS)` and calls `this.records.set(name, value)` for each entry. The constructor takes no arguments. +- [x] 2.3 Confirm no method signatures change — `get` and `set` are untouched. +- [x] 2.4 Confirm no other module's import of `Environment` needs to change (`run.ts`, `cli.ts`, `index.ts`, tests). + +## 3. Environment Unit Tests + +- [x] 3.1 In `src/v2-calculator/vm/Environment.test.ts`: add a test that a fresh `Environment` resolves `pi` to `Math.PI`, `e` to `Math.E`, and `tau` to `2 * Math.PI`. Use `toBe` for exact value equality. +- [x] 3.2 Add a test that iterates `Object.entries(BUILTIN_CONSTANTS)` and asserts every entry resolves via `env.get(name)` to its declared value. This catches drift if the constants file gains entries without the env loop being updated. +- [x] 3.3 Add a shadowing test: `env.set("pi", 5)` then `env.get("pi")` returns `5`. +- [x] 3.4 Add an isolation test: shadow `pi` in one `Environment`, construct a second fresh `Environment`, confirm `env.get("pi")` in the second returns `Math.PI`. +- [x] 3.5 Add a negative test: `env.get("phi")` (a non-built-in) on a fresh `Environment` still throws `UndefinedVariable` with `name === "phi"`. This confirms no broad fallback was introduced. +- [x] 3.6 Add a freeze test: a runtime attempt to mutate `BUILTIN_CONSTANTS` (cast to a mutable shape) does not modify the object — `BUILTIN_CONSTANTS.foo` remains `undefined` after the attempt. + +## 4. End-to-end Tests + +- [x] 4.1 In `src/v2-calculator/calculator.test.ts`: add a test that `run("pi")` returns exactly `Math.PI`. +- [x] 4.2 Add a test that `run("e")` returns exactly `Math.E`. +- [x] 4.3 Add a test that `run("tau")` returns exactly `2 * Math.PI`. +- [x] 4.4 Add a test that `run("2 * pi")` returns exactly `2 * Math.PI` (composition with arithmetic). +- [x] 4.5 Add a test that `run("tau / 2")` returns exactly `Math.PI` (composition with arithmetic). +- [x] 4.6 Add a shadowing-through-pipeline test: construct an `Environment`, call `run("pi = 5\npi", env)`, confirm the result is `5`. +- [x] 4.7 Add a test that `run("phi")` (a name NOT in `BUILTIN_CONSTANTS`) still throws `UndefinedVariable` with `name === "phi"`. + +## 5. Documentation + +- [x] 5.1 In `src/v2-calculator/todo.md`: tick item 4 (`Add built-in constants, like pi`) by changing `- [ ]` to `- [x]`. +- [x] 5.2 In `src/v2-calculator/todo.md`: add a new item immediately after the ticked one: `- [ ] Add IEEE-754 named constants: \`inf\`, \`nan\`` so the follow-up captured during brainstorming isn't lost. + +## 6. Quality Gate + +- [x] 6.1 Run `yarn lint` and fix any violations. +- [x] 6.2 Run `yarn typecheck` and fix any errors. +- [x] 6.3 Run `yarn test` and confirm the entire suite is green (not just the v2-calculator slice). + +## 7. Post-Merge + +- [x] 7.1 After approval and merge, sync the delta spec into `openspec/specs/v2-calculator/spec.md` using the `openspec-sync-specs` skill. diff --git a/openspec/changes/archive/2026-05-28-add-v2-calculator/.openspec.yaml b/openspec/changes/archive/2026-05-28-add-v2-calculator/.openspec.yaml new file mode 100644 index 0000000..352690f --- /dev/null +++ b/openspec/changes/archive/2026-05-28-add-v2-calculator/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-05-28 diff --git a/openspec/changes/archive/2026-05-28-add-v2-calculator/design.md b/openspec/changes/archive/2026-05-28-add-v2-calculator/design.md new file mode 100644 index 0000000..f762859 --- /dev/null +++ b/openspec/changes/archive/2026-05-28-add-v2-calculator/design.md @@ -0,0 +1,173 @@ +## Context + +Lucky Script v2 (`src/v2/`) implements the current language stack: an Ohm-generated parser, a tree-walking compiler that emits bytecode, and a stack VM. The VM's `Value` union spans nine kinds (`number`, `closure`, `array`, `string`, `range`, `none`, `boolean`, `native`, `bound_native`, `iterator`), and the bytecode supports closures (`MAKE_CLOSURE`, `CALL`, `RETURN`), control flow (`JMP`, `JMP_IF_FALSY`), iteration (`MAKE_ITER`, `ITER_NEXT_OR_END`), arrays (`MAKE_ARRAY`, `INDEX_GET`, `INDEX_SET`), ranges (`MAKE_RANGE`), and per-kind property dispatch (`GET_PROPERTY`). The pipeline is correct and well-tested, but as a teaching artifact its surface is dense: a reader of `src/v2/vm/run.ts` confronts ~600 LOC of opcode handlers before the underlying fetch/dispatch shape becomes visible. + +A calculator-sized cousin under `src/v2-calculator/` solves this by stripping every concern except the parse-compile-execute pipeline. With one value kind (`number`), six opcodes, and one expression grammar, the entire VM fits in ~25 LOC and the entire compiler in ~30. The structural shape — directory layout (`parser/`, `compiler/`, `vm/`), type names (`Literal` / `Arithmetic` / `Unary`), parser API (`parse` / `tryParse` with the `incomplete: true` discriminant), `run(source) → result` entry point — mirrors v2 exactly. Flipping between the two codebases is a one-to-one mental operation: each calculator file is a strict subset of its v2 counterpart. + +**Stakeholders.** The author is the only user. Lucky has no external consumers (root `AGENTS.md` Philosophy section). Breaking changes are explicitly fine. + +## Goals / Non-Goals + +**Goals:** + +- Ship `src/v2-calculator/` as a self-contained subset of v2: parser, compiler, VM. +- Subset the Ohm grammar to arithmetic only — `+ - * /`, parens, unary `+`/`-`, decimal numbers. +- Mirror v2's directory naming and module surface (`parse`/`tryParse`, `compile`, `run`). +- Reach the result through real bytecode — not tree-walking — so the pipeline reads as "v2 minus features." +- Provide a single-line REPL via `yarn lucky-calculator`. +- Cover behaviour with integration-first tests through `run(source)`. + +**Non-Goals:** + +- Variables, scoping, environments — there is no `Environment.ts` and no symbol resolution. +- Closures, function definitions, `MAKE_CLOSURE` — there is no `FunctionProto` wrapper. +- Comparison, logical, range, string, boolean, `none`, or array operations — only numbers exist. +- Source spans on AST nodes — Ohm's own `m.message` covers error reporting at calculator scale. +- A `runtime/` layer with built-ins or method registries — no host overrides, no per-kind dispatch. +- Multi-line REPL continuation — each line is a complete expression or a parse error. +- Custom error classes — plain `Error` throws everywhere. +- Backwards compatibility with v2 — v2 stays unchanged; the calculator is a new artifact, not a refactor. + +## Decisions + +### Decision 1: Flat bytecode — no `FunctionProto`, no per-function constant pools + +**Decision.** `Bytecode` is a record of two arrays: + +```ts +export interface Bytecode { + readonly code: readonly Instruction[]; + readonly constants: readonly number[]; +} +``` + +`code` is the instruction sequence; `constants` is a flat pool of numeric literals indexed by `LOAD_CONST`. There is no `main: FunctionProto`, no `functions: FunctionProto[]`, no `name`, no `params`. The VM accepts `Bytecode` directly and dispatches its instructions in order against a `number[]` operand stack. + +**Why.** v2's `FunctionProto` wrapper exists for two purposes: (1) `MAKE_CLOSURE { fnIndex }` references a flat function table by integer, and (2) each function carries its own constant pool so dedup is local. The calculator has no closures and no functions, so both purposes evaporate. The wrapper would carry a vestigial `name: "__main"`, `params: []`, and an unused `functions: []` outer field, obscuring rather than clarifying what the bytecode actually contains. + +**Alternatives considered:** + +- **Faithful v2 wrapping** — keep `{ main: FunctionProto; functions: FunctionProto[] }` with `functions` always `[]`. Pro: future extensions (let-bindings → DEFINE → environment → function definitions) would have a head start because the wrapper is already in place. Con: the extra layer reads as "why is this here?" for anyone studying the calculator alone, and the calculator's stated purpose is to be readable in isolation. Rejected. +- **No bytecode at all — a tree-walking evaluator labeled "vm"** — smallest LOC by far. Rejected because it defeats the goal of mirroring v2's parse → compile → execute pipeline; the `vm/` directory would be a misnomer. + +### Decision 2: One value kind on the operand stack — raw `number` + +**Decision.** The operand stack is `number[]`. There is no `Value` discriminated union, no `vm/value.ts`, no factory functions. `LOAD_CONST` pushes `bytecode.constants[index]` (a `number`); arithmetic opcodes pop two numbers and push their result; `NEG` pops one number and pushes its negation; `HALT` pops the top of the stack and returns it. + +```ts +// vm/run.ts (sketch) +case "ADD": { const r = stack.pop()!; const l = stack.pop()!; stack.push(l + r); break; } +case "DIV": { const r = stack.pop()!; const l = stack.pop()!; stack.push(l / r); break; } +case "NEG": stack.push(-stack.pop()!); break; +case "HALT": return stack.pop()!; +``` + +**Why.** v2's `Value` union exists to discriminate kinds at runtime — `ADD` peels two arms and throws `TypeMismatch` if either operand is non-numeric; `CALL` peels four arms. The calculator's grammar produces exactly one value kind, so every opcode operates on `number` and no discrimination is needed. Stripping the wrapper removes the `vm/value.ts` factory-and-formatter file and makes the VM loop direct. + +**Alternatives considered:** + +- **A single-arm `Value = { kind: "number"; value: number }` union** — would mirror v2's stack shape exactly but at zero information density (every dereference is `.value`). Rejected as ceremony with no payoff; if a second value kind is ever introduced the wrapper comes with it as part of that change. + +### Decision 3: Unary `+` emits no opcode + +**Decision.** The semantics layer produces a `Unary { op: "+", expr }` AST node (mirroring v2's shape). The compiler's `Unary` case emits the operand's bytecode and then emits `NEG` only if `op === "-"`. `+5` compiles to the same instructions as `5`. + +```ts +// compiler/compiler.ts (sketch) +case "Unary": + emit(expr.expr); + if (expr.op === "-") code.push({ opcode: "NEG" }); + return; // "+" is a no-op +``` + +**Why.** Unary `+` is the identity operation on numbers; emitting a `POS` opcode that pops and re-pushes the same value would consume runtime cycles for nothing. v2 takes the same approach in `compileUnary` — the branch is `expr.op === "not" ? "NOT" : "NEG"`, with `+` falling through silently. + +**Alternatives considered:** + +- **Drop `+e` to `e` during semantics** — collapse the node at parse time. Pro: smaller AST. Con: diverges from v2's AST shape (v2 keeps the `Unary` node so source-span / error paths can refer to the position of the `+`). The calculator doesn't carry spans, but matching the AST shape preserves the "calculator = v2-minus-features" property. Rejected. + +### Decision 4: No constant-pool dedup + +**Decision.** The compiler's `Literal` case appends each literal to `constants` unconditionally — no lookup of existing entries. `1+1+1` produces `[1, 1, 1]` in the pool, with three corresponding `LOAD_CONST` instructions referencing indices `0`, `1`, `2`. + +**Why.** v2's base `addConstant` (in `CompilerContext.ts`) also pushes unconditionally; dedup is performed only by the dedicated `bytecode-constant-pool` capability in v2. Mirroring v2's pre-dedup state is the simplest baseline behaviour and avoids adding a `Map` lookup that the calculator's tiny programs never benefit from. If dedup becomes interesting later, it is a five-line patch (add a `Map`, check-or-insert, return the existing index). + +**Alternatives considered:** + +- **Dedup via `constants.indexOf`** — one line of code, but `O(n)` per literal. Rejected: no functional benefit at this scale, and asymmetric with how v2's `addConstant` works. + +### Decision 5: REPL is single-line — multi-line continuation is intentionally absent + +**Decision.** Every readline `line` event is parsed standalone. If `tryParse` succeeds, run and `console.log` the result; if it fails (for any reason, including incomplete-at-EOF), `console.error` the message and re-prompt. The `tryParse.incomplete` flag is still set on `TryParseResult` to match v2's parser API, but the REPL ignores it. + +```ts +// cli.ts (sketch) +rl.on("line", (line) => { + const trimmed = line.trim(); + if (trimmed === "") { + rl.prompt(); + return; + } + if (trimmed === "exit" || trimmed === "quit") { + rl.close(); + return; + } + + const parsed = tryParse(line); + if (!parsed.ok) { + console.error(parsed.message); + rl.prompt(); + return; + } + + try { + console.log(runBytecode(compile(parsed.program))); + } catch (err) { + console.error(err instanceof Error ? err.message : String(err)); + } + rl.prompt(); +}); +``` + +**Why.** v2's REPL uses the `incomplete: true` signal so multi-statement blocks (`if … end`) can be typed across lines. The calculator has no block constructs — `(1 +\n 2)` is the only multi-line case, and forcing the user to put it on one line is acceptable for a single-line arithmetic REPL. Dropping the buffer state simplifies `cli.ts` from ~40 LOC to ~25 and removes a continuation-prompt mode. + +**Alternatives considered:** + +- **Mirror v2's `processReplLine` exactly** — accumulate a buffer until `incomplete` clears. Pro: closer parity with v2. Con: adds buffer state, a continuation prompt, and a separate helper module for zero value at calculator scale. +- **Drop `tryParse` entirely; export only `parse(code): Program`** — the REPL would wrap it in try/catch. Rejected because matching v2's parser API surface was an explicit goal; keeping `tryParse` costs ten lines and zero additional runtime behaviour. + +### Decision 6: Division-by-zero passes through to native JS + +**Decision.** The `DIV` opcode is `stack.push(left / right)`. No special-case for `right === 0`. `1/0` evaluates to `Infinity`, `0/0` to `NaN`. These values flow through subsequent arithmetic per IEEE 754. + +**Why.** v2's `DIV` does the same — see `src/v2/vm/run.test.ts` `"DIV by zero produces Infinity"` and `"DIV of zero by zero produces NaN"`. The behaviour matches what users of a JS-hosted calculator expect, and special-casing would require a custom error type, a `try/catch` in the REPL formatter, and special-printing logic — all of which contradicts the "plain `Error` throws everywhere" decision. + +**Alternatives considered:** + +- **Throw on divide-by-zero with a friendly message** — `throw new Error("division by zero")`. Rejected for the divergence from v2 and the extra REPL plumbing for a behaviour every desktop-calculator user already understands. + +### Decision 7: Public surface is exactly `run(source) → number` + +**Decision.** `src/v2-calculator/index.ts` exports exactly one function: + +```ts +export function run(source: string): number { + return runBytecode(compile(parse(source))); +} +``` + +Integration tests import this. The REPL inlines the same composition (so it can branch on `tryParse` for error-reporting before reaching the compiler). `parse`, `compile`, `tryParse`, the `Bytecode` type, and the `Instruction` type are reachable only via their respective layer modules; nothing else is re-exported from the root. + +**Why.** Matches v2's `src/v2/index.ts` (`export { run, type RunOptions } from "./runtime"`) — same shape, fewer args (no `RunOptions` because there are no built-ins to override and no resource limits to wire). Integration tests can `expect(run("1+2")).toBe(3)`; that's the only API needed. + +**Alternatives considered:** + +- **Expose every layer** (`parse`, `compile`, `tryParse`, types) — appropriate if the calculator were intended as a library others would link against. It isn't. Re-export can be added later if a use-case appears. + +## Risks / Trade-offs + +- **Risk: divergence drift between v2 and the calculator over time.** As v2 evolves, the calculator may stop being a clean subset (e.g., if v2 renames `Arithmetic` to `BinaryOp`, the calculator's identical name becomes a near-miss rather than a literal subset). **Mitigation:** the calculator is small enough (~350 LOC total) that mechanical realignment is cheap; periodic sync is acceptable. + +- **Risk: float passthrough surfaces JS-isms (`0.1 + 0.2 === 0.30000000000000004`) that confuse casual users.** **Mitigation:** explicitly documented in the spec under the REPL's "raw `console.log` output" requirement. The behaviour matches v2, so users learning Lucky see the same arithmetic everywhere. + +- **Trade-off: by mirroring v2's `tryParse` API but not using the `incomplete` flag, the calculator carries one piece of dead branching at the REPL boundary.** Cost is ~5 LOC. Kept because reusing v2's parser shape was an explicit goal, and removing it later would create a non-obvious diff between the two parsers' surfaces. diff --git a/openspec/changes/archive/2026-05-28-add-v2-calculator/proposal.md b/openspec/changes/archive/2026-05-28-add-v2-calculator/proposal.md new file mode 100644 index 0000000..9bdcb26 --- /dev/null +++ b/openspec/changes/archive/2026-05-28-add-v2-calculator/proposal.md @@ -0,0 +1,75 @@ +## Why + +Lucky v2 (`src/v2/`) is the current language stack — Ohm grammar, compiler, bytecode VM — and a rich teaching artifact. Its richness is also a barrier: a newcomer reading `src/v2/vm/run.ts` has to ignore eight value kinds and a couple dozen opcodes before the bytecode loop itself becomes visible. A calculator-sized cousin under `src/v2-calculator/` strips the language to "arithmetic expressions only," compresses the whole pipeline to ~350 LOC, and keeps the directory layout structurally identical to v2 so the larger codebase reads as "calculator + features." Useful for the author as a study object and a stable scaffold to compare future v2 changes against. + +## What Changes + +**New directory `src/v2-calculator/`** holding a self-contained parse → compile → execute pipeline: + +- **Parser (Ohm)**: subset of `src/v2/parser/grammar.ohm`. Top rule `Program = AddExp`. Precedence chain `AddExp → MulExp → UnaryExp → PriExp`. Atoms are `( AddExp )` and `number`. Newlines are ordinary whitespace (no `nl` override, no `space` override). +- **AST** (`parser/ast.ts`): three node kinds, matching v2 names — `Literal`, `Arithmetic` (op ∈ `{+, -, *, /}`), `Unary` (op ∈ `{+, -}`). No source spans. +- **Parser surface** (`parser/parser.ts`): `parse(code) → Program` and `tryParse(code) → TryParseResult` with the same `incomplete`-at-EOF discriminant as v2. +- **Compiler** (`compiler/`): tree-walking emitter producing flat bytecode. `Bytecode = { code: Instruction[]; constants: number[] }` — no `FunctionProto` wrapper, no `functions[]` table. Six opcodes plus `HALT`: `LOAD_CONST`, `ADD`, `SUB`, `MUL`, `DIV`, `NEG`, `HALT`. Unary `+` emits no opcode. Constants are not deduplicated. +- **VM** (`vm/run.ts`): single fetch/dispatch loop over a `number[]` operand stack. Division-by-zero passes through to native JS (`1/0 → Infinity`, `0/0 → NaN`), matching v2's `DIV` opcode. +- **Entry point** (`index.ts`): `export function run(source: string): number = runBytecode(compile(parse(source)))`. +- **REPL** (`cli.ts`): prompt `> `, single-line evaluation, exits via Ctrl-D / `exit` / `quit`. Parse errors print to stderr and re-prompt. Multi-line continuation is intentionally absent. +- **`yarn lucky-calculator`** script in `package.json`. +- **Tests** (integration-first): one AST-shape file under `parser/` plus one integration file at the calculator root that drives source strings through `run()`. + +**Explicitly out of scope** (deliberately excluded from the calculator surface): + +- Variables, `let` bindings, assignment +- Comparison operators, logical operators, `not` +- Ranges, strings, booleans, `none`, arrays +- Function definitions, anonymous functions, closures +- Control flow (`if` / `while` / `for`) +- Built-in functions, method dispatch, per-kind method registry +- Source spans on AST nodes +- Custom error classes (a `vm/errors.ts`) — every failure is a plain `Error` +- A `runtime/` layer (no built-ins to seed, no method registry, no persistent REPL session state) +- An `examples/` directory (one integration-test file at the calculator root suffices) +- Multi-line REPL continuation +- Per-function constant pools (the pool is flat) + +This change does **not** alter any v2 behaviour. `src/v2/` and the existing `yarn lucky` REPL are untouched. + +## Capabilities + +### New Capabilities + +- `v2-calculator`: The full calculator pipeline — grammar shape, AST node kinds, parser surface, bytecode layout, opcode semantics, the `run(source)` entry point, and the REPL contract. The capability holds the entire calculator behaviour; it is not extended by any other capability. + +### Modified Capabilities + +None. The calculator is a standalone artifact under `src/v2-calculator/` and does not change behaviour of any existing v2 capability. + +## Impact + +**New code** under `src/v2-calculator/`: + +``` +parser/ + grammar.ohm + grammar.ohm-bundle.js (generated by yarn ohm) + grammar.ohm-bundle.d.ts (generated by yarn ohm) + parser.ts + ast.ts + semantics.ts + parser.test.ts +compiler/ + compiler.ts + bytecode.ts +vm/ + run.ts +cli.ts +index.ts +calculator.test.ts +``` + +**Modified files**: `package.json` (one `scripts` entry added). + +**Dependencies**: none added. Reuses `ohm-js`, `vitest`, `ts-node` already in the repo. + +**Tooling**: `yarn ohm` already globs `src/**/*.ohm`, so the new grammar bundle regenerates without script changes. The `PostToolUse` hook noted in v2's AGENTS.md fires on `*.ohm` saves and covers the calculator's grammar too. + +**Quality gates**: `yarn lint && yarn typecheck && yarn test` after implementation. No CI changes needed. diff --git a/openspec/changes/archive/2026-05-28-add-v2-calculator/specs/v2-calculator/spec.md b/openspec/changes/archive/2026-05-28-add-v2-calculator/specs/v2-calculator/spec.md new file mode 100644 index 0000000..efb7ff9 --- /dev/null +++ b/openspec/changes/archive/2026-05-28-add-v2-calculator/specs/v2-calculator/spec.md @@ -0,0 +1,334 @@ +## ADDED Requirements + +### Requirement: Calculator grammar accepts a single arithmetic expression per program + +The file `src/v2-calculator/parser/grammar.ohm` SHALL define an Ohm grammar named `Calculator` whose top rule is `Program = AddExp`. The grammar SHALL accept exactly one arithmetic expression per program — there are no statement separators, no block constructs, no top-level lists. + +Operator precedence SHALL be encoded by the production chain `AddExp → MulExp → UnaryExp → PriExp`. The operator set SHALL be exactly: binary `+`, `-`, `*`, `/` (left-associative); unary prefix `+` and `-`; grouping with `(` and `)`. The atomic productions of `PriExp` SHALL be exactly `( AddExp )` and `number`. + +The `number` rule SHALL be `digit+ ("." digit+)?` — non-negative integer or decimal literal. The leading sign on a literal SHALL be parsed as unary prefix `Unary`, not as part of `number`. + +The grammar SHALL NOT include any of: identifier, keyword, string literal, boolean literal, `none`, array literal, range expression, comparison operator (`<`, `>`, `<=`, `>=`, `==`, `!=`), logical operator (`and`, `or`, `not`), assignment, postfix call (`(...)`), postfix index (`[...]`), postfix property access (`.ident`), statement, function definition, let binding, or any `space`/`nl` override (newlines SHALL be ordinary whitespace under Ohm's default `space` rule). + +#### Scenario: A bare integer literal parses as a Literal + +- **WHEN** the source `42` is parsed +- **THEN** the resulting AST is `{ kind: "Literal", value: 42 }` + +#### Scenario: A decimal literal parses as a Literal + +- **WHEN** the source `3.14` is parsed +- **THEN** the resulting AST is `{ kind: "Literal", value: 3.14 }` + +#### Scenario: Subtraction is left-associative + +- **WHEN** the source `1 - 2 - 3` is parsed +- **THEN** the resulting AST is `Arithmetic("-", Arithmetic("-", Literal(1), Literal(2)), Literal(3))` + +#### Scenario: Multiplication binds tighter than addition + +- **WHEN** the source `1 + 2 * 3` is parsed +- **THEN** the resulting AST is `Arithmetic("+", Literal(1), Arithmetic("*", Literal(2), Literal(3)))` + +#### Scenario: Parentheses override default precedence + +- **WHEN** the source `(1 + 2) * 3` is parsed +- **THEN** the resulting AST is `Arithmetic("*", Arithmetic("+", Literal(1), Literal(2)), Literal(3))` + +#### Scenario: Unary minus stacks + +- **WHEN** the source `--5` is parsed +- **THEN** the resulting AST is `Unary("-", Unary("-", Literal(5)))` + +#### Scenario: Newlines are ordinary whitespace + +- **WHEN** the source is `1 +\n 2` (newline between `+` and `2`) +- **THEN** parsing succeeds +- **AND** the resulting AST equals the AST of `1 + 2` + +#### Scenario: An identifier is not a legal expression + +- **WHEN** the source `x` is parsed +- **THEN** parsing fails + +#### Scenario: A boolean literal is not a legal expression + +- **WHEN** the source `true` is parsed +- **THEN** parsing fails + +#### Scenario: A trailing operator is not a legal expression + +- **WHEN** the source `1 +` is parsed +- **THEN** parsing fails + +### Requirement: The AST has exactly three node kinds, named to match v2 + +The file `src/v2-calculator/parser/ast.ts` SHALL export the discriminated union: + +```ts +export type Expr = + | { kind: "Literal"; value: number } + | { kind: "Arithmetic"; op: "+" | "-" | "*" | "/"; left: Expr; right: Expr } + | { kind: "Unary"; op: "+" | "-"; expr: Expr }; + +export type Program = Expr; +``` + +The discriminant names `Literal`, `Arithmetic`, and `Unary` SHALL match the corresponding node kinds in `src/v2/parser/ast/`. AST nodes SHALL NOT carry source spans, parent pointers, or any field beyond those listed in the type definition above. + +#### Scenario: The three AST kinds are distinct + +- **WHEN** the sources `5`, `1+2`, and `-5` are parsed +- **THEN** the resulting `kind` values are exactly `"Literal"`, `"Arithmetic"`, and `"Unary"` respectively + +#### Scenario: AST nodes have no span field + +- **WHEN** any source is parsed +- **THEN** no AST node in the result contains a key named `span` + +### Requirement: Parser surface mirrors v2's API exactly + +The file `src/v2-calculator/parser/parser.ts` SHALL export `parse(code: string): Program` and `tryParse(code: string): TryParseResult` with the same `TryParseResult` discriminated union as v2: + +```ts +export type TryParseResult = + | { ok: true; program: Program } + | { ok: false; incomplete: true; message: string } + | { ok: false; incomplete: false; message: string }; +``` + +`tryParse` SHALL return `{ ok: false, incomplete: true, … }` when the Ohm match fails at end-of-input (i.e., `matchResult.getRightmostFailurePosition() === code.length`) and `{ ok: false, incomplete: false, … }` otherwise. `parse` SHALL throw `new Error(message)` whenever `tryParse` returns `ok: false`, regardless of the `incomplete` flag. + +The semantics layer (`src/v2-calculator/parser/semantics.ts`) SHALL register a single `toAst` operation on the grammar's semantics that builds the AST defined above. + +#### Scenario: tryParse flags trailing-operator failure as incomplete + +- **WHEN** `tryParse("1 +")` is called +- **THEN** the result is `{ ok: false, incomplete: true, message: }` + +#### Scenario: tryParse flags mid-expression failure as not incomplete + +- **WHEN** `tryParse("1 1")` is called +- **THEN** the result is `{ ok: false, incomplete: false, message: }` + +#### Scenario: tryParse flags unclosed-paren as incomplete + +- **WHEN** `tryParse("(1 + 2")` is called +- **THEN** the result is `{ ok: false, incomplete: true, message: }` + +#### Scenario: parse throws on any failure + +- **WHEN** `parse("1 +")` is called +- **THEN** an `Error` is thrown whose message comes from the Ohm matcher + +#### Scenario: parse returns the AST on success + +- **WHEN** `parse("1 + 2")` is called +- **THEN** the return value is `Arithmetic("+", Literal(1), Literal(2))` + +### Requirement: Bytecode is a flat record of instructions plus a numeric constant pool + +The file `src/v2-calculator/compiler/bytecode.ts` SHALL export: + +```ts +export type Instruction = + | { opcode: "LOAD_CONST"; index: number } + | { opcode: "ADD" } + | { opcode: "SUB" } + | { opcode: "MUL" } + | { opcode: "DIV" } + | { opcode: "NEG" } + | { opcode: "HALT" }; + +export interface Bytecode { + readonly code: readonly Instruction[]; + readonly constants: readonly number[]; +} +``` + +The `Bytecode` type SHALL NOT include a `FunctionProto` wrapper, a `functions` table, a `main` sub-record, a `name` field, or a `params` field. There SHALL NOT be additional opcodes beyond the seven listed; in particular, no immediate-number opcode (`PUSH_NUMBER`), no identity opcode for unary plus (`POS`), no `POP`, no `DUP`, no jumps, no `MAKE_*` constructors, no `CALL`/`RETURN`. + +The constant pool `constants` SHALL hold raw JavaScript `number` values, not wrapped `NumberValue` objects. The compiler SHALL NOT deduplicate equal entries in the pool. + +#### Scenario: A single literal compiles to LOAD_CONST + HALT + +- **WHEN** the source `42` is compiled +- **THEN** `bytecode.code` is `[{ opcode: "LOAD_CONST", index: 0 }, { opcode: "HALT" }]` +- **AND** `bytecode.constants` is `[42]` + +#### Scenario: A binary expression compiles to operand instructions plus the operator + +- **WHEN** the source `1 + 2` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, LOAD_CONST 1, ADD, HALT]` +- **AND** `bytecode.constants` is `[1, 2]` + +#### Scenario: Operator precedence reflects in bytecode order + +- **WHEN** the source `1 + 2 * 3` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, LOAD_CONST 1, LOAD_CONST 2, MUL, ADD, HALT]` +- **AND** `bytecode.constants` is `[1, 2, 3]` + +#### Scenario: Parentheses change emission order + +- **WHEN** the source `(1 + 2) * 3` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, LOAD_CONST 1, ADD, LOAD_CONST 2, MUL, HALT]` + +#### Scenario: Repeated literals occupy separate pool slots + +- **WHEN** the source `1 + 1 + 1` is compiled +- **THEN** `bytecode.constants` has length `3` +- **AND** each entry equals `1` + +#### Scenario: Unary plus emits no opcode + +- **WHEN** the source `+5` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, HALT]` +- **AND** `bytecode.constants` is `[5]` + +#### Scenario: Unary minus emits NEG + +- **WHEN** the source `-5` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, NEG, HALT]` +- **AND** `bytecode.constants` is `[5]` + +#### Scenario: Stacked unary minus emits stacked NEG + +- **WHEN** the source `--5` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, NEG, NEG, HALT]` + +### Requirement: The VM executes the six opcodes against a `number[]` operand stack + +The file `src/v2-calculator/vm/run.ts` SHALL export `run(bytecode: Bytecode): number`. The operand stack SHALL be typed `number[]`. There SHALL NOT be a `Value` discriminated union, a `vm/value.ts` file, or any wrapper type around stack entries. + +The opcode handlers SHALL behave as follows: + +- `LOAD_CONST { index }` pushes `bytecode.constants[index]` onto the stack. +- `ADD` pops `right`, then `left`, pushes `left + right`. +- `SUB` pops `right`, then `left`, pushes `left - right`. +- `MUL` pops `right`, then `left`, pushes `left * right`. +- `DIV` pops `right`, then `left`, pushes `left / right` — the native JavaScript `/` operator. `1/0` SHALL yield `Infinity`, `0/0` SHALL yield `NaN`; no special-casing. +- `NEG` pops one number, pushes its arithmetic negation. +- `HALT` pops the top of the stack and returns it as the function result. + +The VM SHALL NOT bounds-check the operand stack, the constant pool, or the instruction sequence. The grammar guarantees one value on the stack at `HALT`; deviations from that invariant are compiler bugs, not user-program errors. + +#### Scenario: Sum of two literals + +- **WHEN** `run(compile(parse("1 + 2")))` is called +- **THEN** the result is `3` + +#### Scenario: Subtraction is left-associative at runtime + +- **WHEN** `run(compile(parse("10 - 3 - 2")))` is called +- **THEN** the result is `5` + +#### Scenario: Multiplication binds tighter than addition at runtime + +- **WHEN** `run(compile(parse("1 + 2 * 3")))` is called +- **THEN** the result is `7` + +#### Scenario: Parentheses override precedence at runtime + +- **WHEN** `run(compile(parse("(1 + 2) * 3")))` is called +- **THEN** the result is `9` + +#### Scenario: Division produces a decimal + +- **WHEN** `run(compile(parse("10 / 4")))` is called +- **THEN** the result is `2.5` + +#### Scenario: Unary negation produces a negative + +- **WHEN** `run(compile(parse("-(1 + 2)")))` is called +- **THEN** the result is `-3` + +#### Scenario: Double unary minus is identity + +- **WHEN** `run(compile(parse("--5")))` is called +- **THEN** the result is `5` + +#### Scenario: Unary plus is identity + +- **WHEN** `run(compile(parse("+5")))` is called +- **THEN** the result is `5` + +#### Scenario: Decimal literals add precisely when representable + +- **WHEN** `run(compile(parse("1.5 + 0.5")))` is called +- **THEN** the result is `2` + +#### Scenario: Division-by-zero produces Infinity + +- **WHEN** `run(compile(parse("1 / 0")))` is called +- **THEN** the result is `Infinity` (`Number.POSITIVE_INFINITY`) + +#### Scenario: Zero-divided-by-zero produces NaN + +- **WHEN** `run(compile(parse("0 / 0")))` is called +- **THEN** the result satisfies `Number.isNaN(result)` + +### Requirement: The public entry point composes parse, compile, and run + +The file `src/v2-calculator/index.ts` SHALL export a function with signature `run(source: string): number` that composes `parse`, `compile`, and the VM's `run`: + +```ts +export function run(source: string): number { + return runBytecode(compile(parse(source))); +} +``` + +The function SHALL be the only public surface re-exported from `index.ts`. `parse`, `compile`, `tryParse`, `Bytecode`, `Instruction`, and other layer types SHALL remain accessible only through their respective layer modules (`./parser`, `./compiler/...`, `./vm/run`). The function SHALL propagate parse-time and compile-time errors as thrown `Error` objects with no wrapping, swallowing, or transformation. + +#### Scenario: Numeric result is returned as a JavaScript number + +- **WHEN** `run("1.5 + 0.5")` is called +- **THEN** the return value is `2` + +#### Scenario: Parse error propagates as a thrown Error + +- **WHEN** `run("1 +")` is called +- **THEN** the call throws `Error` with the Ohm parse-error message + +### Requirement: `yarn lucky-calculator` launches a single-line REPL + +The file `package.json` SHALL contain the script `"lucky-calculator": "ts-node src/v2-calculator/cli.ts"`. Running `yarn lucky-calculator` SHALL start an interactive REPL with the following contract: + +- The prompt SHALL be `> ` for every input. There SHALL NOT be a continuation prompt; the prompt SHALL NOT change shape across lines. +- The REPL SHALL parse each line standalone; multi-line continuation SHALL NOT be implemented. The `tryParse.incomplete` flag SHALL be ignored at the REPL level — a parse failure for any reason is reported and the prompt re-displayed. +- The bare typed words `exit` or `quit` (after `trim()`) SHALL exit the REPL with code `0`. +- Ctrl-D (the readline `close` event) SHALL exit the REPL with code `0`. +- A successful evaluation SHALL print the result with `console.log(result)`. No formatting SHALL be applied; the JavaScript `String(number)` representation passes through unchanged. +- A parse error SHALL be printed to `console.error` (the parse-error message string) and the prompt SHALL be re-displayed. +- A runtime-thrown `Error` SHALL be printed to `console.error` (the error's `message`) and the prompt SHALL be re-displayed. +- An empty or whitespace-only line SHALL be treated as a no-op — the prompt is re-displayed without evaluation. + +The REPL SHALL NOT carry session state across lines — there SHALL NOT be a `_` rebinding, persistent variables, or a multi-line buffer. + +#### Scenario: A simple expression evaluates and prints + +- **WHEN** the user types `1 + 2` at the prompt +- **THEN** the REPL prints `3` to stdout +- **AND** re-displays the `> ` prompt + +#### Scenario: A parse error is reported and the REPL continues + +- **WHEN** the user types `1 +` at the prompt +- **THEN** the REPL prints the Ohm parse-error message to stderr +- **AND** re-displays the `> ` prompt +- **AND** the process does NOT exit + +#### Scenario: `exit` closes the REPL + +- **WHEN** the user types `exit` at the prompt +- **THEN** the REPL exits with code `0` + +#### Scenario: `quit` closes the REPL + +- **WHEN** the user types `quit` at the prompt +- **THEN** the REPL exits with code `0` + +#### Scenario: An empty line re-prompts without output + +- **WHEN** the user presses Enter on an empty prompt +- **THEN** the REPL re-displays `> ` +- **AND** nothing is printed to stdout or stderr diff --git a/openspec/changes/archive/2026-05-28-add-v2-calculator/tasks.md b/openspec/changes/archive/2026-05-28-add-v2-calculator/tasks.md new file mode 100644 index 0000000..531494c --- /dev/null +++ b/openspec/changes/archive/2026-05-28-add-v2-calculator/tasks.md @@ -0,0 +1,66 @@ +## 1. Grammar + +- [x] 1.1 Create `src/v2-calculator/parser/grammar.ohm` with the `Calculator` grammar: top rule `Program = AddExp`, the `AddExp → MulExp → UnaryExp → PriExp` precedence chain (labelled alternatives `_plus`, `_minus`, `_times`, `_divide`, `_pos`, `_neg`, `_paren`), and the `number = digit+ ("." digit+)?` rule. Do NOT override `space` or define an `nl` rule — newlines are ordinary whitespace. +- [x] 1.2 Save the grammar; the project's PostToolUse hook should regenerate `src/v2-calculator/parser/grammar.ohm-bundle.js` and `grammar.ohm-bundle.d.ts`. If running manually, `yarn ohm`. Verify both files appear under `src/v2-calculator/parser/`. +- [x] 1.3 Run `yarn typecheck` and confirm the generated bundle's types compile against the rest of the calculator directory (which is still empty at this point — typecheck must pass on the bundle alone). + +## 2. Parser (AST + semantics + entry) + +- [x] 2.1 Create `src/v2-calculator/parser/ast.ts` exporting the `Expr` discriminated union with three members (`Literal`, `Arithmetic`, `Unary`) plus `Program = Expr`. No `span` field; no helper factories; no re-exports from elsewhere. +- [x] 2.2 Create `src/v2-calculator/parser/semantics.ts`. Import the generated grammar, call `createSemantics()`, register a single `toAst` operation with handlers for `Program`, `AddExp_plus`, `AddExp_minus`, `MulExp_times`, `MulExp_divide`, `UnaryExp_pos`, `UnaryExp_neg`, `PriExp_paren`, and `number`. The `number` handler builds `{ kind: "Literal", value: parseFloat(this.sourceString) }`. Export the configured `semantics` object. +- [x] 2.3 Create `src/v2-calculator/parser/parser.ts` exporting `tryParse(code) → TryParseResult` and `parse(code) → Program`. Mirror v2's API exactly, including the `incomplete: true` branch when `matchResult.getRightmostFailurePosition() === code.length`. +- [x] 2.4 Create `src/v2-calculator/parser/parser.test.ts`. Cover AST shape and parse-error behaviour: + - bare integer literal (`"42"` → `Literal(42)`) + - bare decimal literal (`"3.14"` → `Literal(3.14)`) + - subtraction is left-associative (`"1 - 2 - 3"` shape) + - precedence (`"1 + 2 * 3"` shape) + - parens override precedence (`"(1+2)*3"` shape) + - unary negation (`"-5"` shape) + - stacked unary minus (`"--5"` shape) + - unary plus produces a `Unary` node, not a collapsed `Literal` + - newlines are whitespace (`"1 +\n 2"` parses with the same AST as `"1 + 2"`) + - rejected: identifier (`"x"`), boolean keyword (`"true"`), juxtaposed numbers (`"1 1"`) + - `tryParse("1 +")` returns `{ ok: false, incomplete: true, … }` + - `tryParse("1 1")` returns `{ ok: false, incomplete: false, … }` + - `tryParse("(1 + 2")` returns `{ ok: false, incomplete: true, … }` + - `parse("1 +")` throws an `Error` + +## 3. Compiler + +- [x] 3.1 Create `src/v2-calculator/compiler/bytecode.ts` exporting the seven-arm `Instruction` union (`LOAD_CONST`, `ADD`, `SUB`, `MUL`, `DIV`, `NEG`, `HALT`) and the `Bytecode = { code, constants }` interface. No `FunctionProto`, no `functions[]`. +- [x] 3.2 Create `src/v2-calculator/compiler/compiler.ts` exporting `compile(program: Program): Bytecode`. Walk the AST recursively in post-order: + - `Literal`: push the value into `constants`, emit `LOAD_CONST { index: constants.length - 1 }`. + - `Arithmetic`: recurse into `left`, then `right`, then emit `ADD`/`SUB`/`MUL`/`DIV` for the operator. + - `Unary`: recurse into `expr`; emit `NEG` only if `op === "-"`. `op === "+"` emits nothing. + - After the root expression, append `HALT`. + No constant-pool dedup. + +## 4. VM + +- [x] 4.1 Create `src/v2-calculator/vm/run.ts` exporting `run(bytecode: Bytecode): number`. Use a `number[]` operand stack and a `let ip = 0` instruction pointer. Implement the six opcodes plus `HALT` via a `switch (instr.opcode)`. Use native JS arithmetic for `DIV` (no divide-by-zero special-case). The handler for `HALT` returns `stack.pop()!` (the grammar guarantees one value on the stack). + +## 5. Entry point + REPL + +- [x] 5.1 Create `src/v2-calculator/index.ts` exporting `run(source: string): number` composed as `runBytecode(compile(parse(source)))`. Import `runBytecode` as a named alias for `vm/run.ts`'s `run`. +- [x] 5.2 Create `src/v2-calculator/cli.ts` with a `startRepl()` function using Node's `readline`. Prompt always `> `; empty/whitespace lines re-prompt without evaluating; bare `exit`/`quit` close the readline; otherwise call `tryParse` — on `ok`, `console.log(runBytecode(compile(parsed.program)))`; on failure (any kind), `console.error(parsed.message)`; in either case re-prompt. Wrap the compile-and-run step in a try/catch and `console.error` any thrown runtime error. Add an `if (require.main === module) startRepl()` entry guard. +- [x] 5.3 Edit `package.json` and add `"lucky-calculator": "ts-node src/v2-calculator/cli.ts"` to the `scripts` block. Do not reorder existing entries. + +## 6. End-to-end tests + +- [x] 6.1 Create `src/v2-calculator/calculator.test.ts` driving source strings through `run()` from `index.ts`. Cover: + - basic ops: `"1+2"` → `3`, `"5-3"` → `2`, `"2*3"` → `6`, `"10/4"` → `2.5` + - precedence: `"1+2*3"` → `7`, `"(1+2)*3"` → `9`, `"8/2/2"` → `2` (left-associative division) + - unary: `"-5"` → `-5`, `"-(1+2)"` → `-3`, `"--5"` → `5`, `"+5"` → `5` + - decimals: `"1.5+0.5"` → `2` + - whitespace tolerance: `" 1 + 2 "` → `3` + - native float passthrough: `run("1/0") === Infinity`, `Number.isNaN(run("0/0"))` + - parse error: `() => run("1 +")` throws an `Error` + +## 7. Quality gate + +- [x] 7.1 Run `yarn lint && yarn typecheck && yarn test`. Fix every failure before declaring the change complete. The calculator's test files should appear in Vitest's discovery output and pass. +- [x] 7.2 Manually verify the REPL: `yarn lucky-calculator`, type `1 + 2`, confirm `3` prints. Type `(1 + 2) * 3`, confirm `9`. Type `1 +`, confirm a parse error appears and the REPL stays open. Type `exit`, confirm process exits cleanly. + +## 8. Spec sync (after approval and merge) + +- [x] 8.1 After this change is approved and merged into `master`, sync the delta spec into `openspec/specs/v2-calculator/spec.md` using the `openspec-sync-specs` skill. The synced spec's `## Purpose` should be replaced with a brief one-paragraph description of the calculator's role (the placeholder `TBD - created by syncing change add-v2-calculator. Update after archive.` is set by the sync; replace it post-archive). diff --git a/openspec/changes/archive/2026-05-28-add-variables-to-v2-calculator/design.md b/openspec/changes/archive/2026-05-28-add-variables-to-v2-calculator/design.md new file mode 100644 index 0000000..ad86069 --- /dev/null +++ b/openspec/changes/archive/2026-05-28-add-variables-to-v2-calculator/design.md @@ -0,0 +1,205 @@ +## Context + +The v2-calculator is a deliberately small sibling of `src/v2/`: Ohm grammar → AST → compiler → bytecode VM, but without functions, closures, control flow, or compound values. Today every program is a single arithmetic expression, the VM stack is `number[]`, and `run` returns whatever's left on top of the stack at `HALT`. + +Adding variables is the first feature that breaks the "program = one expression" assumption and the first that requires _persistent state_ outside the operand stack. Every design choice below is dominated by the principle that the calculator should remain the smallest learnable subset that demonstrates each idea once. + +Reference shape — `src/v2/vm/Environment.ts` is a chained, two-API (`define` vs `assign`) lexical Environment built for closures and `let`. The calculator only needs the single-scope core of that, with one method per direction. + +## Goals / Non-Goals + +**Goals:** + +- A program may contain a sequence of expressions separated by newlines; only the last expression's value is the program's result. +- Identifiers can be assigned (`x = e`) and read (`x`) in the same scope. No `let` keyword. +- Assignment is a right-associative expression yielding the assigned value. Chained assignment (`x = y = 1`) works. +- Variables persist across REPL lines. +- Undefined-variable reads raise a runtime error. +- Keep the single-scope, no-chain Environment trivial enough to be a learning artifact. + +**Non-Goals:** + +- Nested scopes, blocks, `let` shadowing. +- Compile-time undefined-variable detection. +- Multi-line expression continuation. +- `_` binding to last result (separate change). +- Built-in constants or functions (separate change). +- Anything user-visible from the existing `src/v2/`'s `Environment.ts` beyond the `get`/`set` shape. + +## Decisions + +### Decision 1: Assignment is an expression with lowest precedence; LHS is grammar-restricted to `ident` + +Assignment lives at the **top** of the expression hierarchy (above `AddExp`), and `AssignExp` is right-recursive on its own RHS for chaining. Crucially, the LHS slot of `AssignExp_assign` is `ident`, not `Expr` or `AddExp` — the grammar refuses to parse anything that's not a bare identifier on the left, so the compiler never needs an "is this an lvalue?" pass. + +```ohm +Expr = AssignExp +AssignExp = ident "=" AssignExp -- assign + | AddExp +``` + +Consequences: + +- `x = y = 123` parses as `x = (y = 123)`. Both bound to 123. Value: 123. +- `1 + x = 2` is a **parse error**. `AssignExp` tries `_assign` first; the leading `1` is not an `ident`, so it falls to `AddExp`, which parses `1 + x`. The leftover `= 2` has no rule to consume it. +- `(x = 1) + 2` is legal. `PriExp_paren` wraps an `Expr` (which is an `AssignExp`), so parenthesised assignment is a normal expression that participates in arithmetic. + +**Alternatives considered:** + +- _Assignment at `PriExp` (tight binding)_ — rejected. Would make `1 + x = 2 → 3`, which is unconventional and a usability trap. The standard "lowest precedence" convention wins on familiarity. +- _Assignment is a statement, not an expression_ — rejected. Loses chained assignment and conflicts with the language's "everything is an expression" stance. + +### Decision 2: Newlines are syntactically significant; statements separate on newline only + +Ohm's default `space` rule swallows newlines. We override it to allow only horizontal whitespace, then make `newline` a first-class separator in the program rule: + +```ohm +Program = NonemptyListOf +newline = "\n"+ +space := " " | "\t" +``` + +`newline = "\n"+` collapses runs of blank lines into a single separator, so `x = 1\n\n\ny = 2` is legal. + +This is a **breaking change** versus the current grammar — today `1 +\n 2` parses (newline is whitespace) and after this change it does not. Acceptable because the calculator already declares itself a learning artifact with no backcompat guarantees (`AGENTS.md`). + +**Alternatives considered:** + +- _Allow `;` as an alternative separator_ — rejected. User's example used only newlines and didn't ask for `;`. Adding it later is a one-line change. +- _Allow continuation lines after a trailing operator_ — rejected for scope. Would require either a lexical-mode tweak (e.g. `newline` after `"+"` is whitespace) or an implicit-join pass. Multi-line expressions are not in the goal set. + +### Decision 3: `STORE_VAR` does not pop; the compiler emits `POP` between top-level expressions + +For assignment to be an expression whose value is the assigned value, the `STORE_VAR` opcode must **read** the top of stack and write the environment **without consuming** the stack value. The value naturally remains available for further arithmetic. + +``` + x = 5 (one-expression program) + ───────────── + LOAD_CONST 0 ; stack: [5] + STORE_VAR 0 ; stack: [5] (env now has x=5; value kept) + HALT ; returns 5 + + x = y = 5 (chained) + ───────────── + LOAD_CONST 0 ; stack: [5] + STORE_VAR 1 (y) ; stack: [5] + STORE_VAR 0 (x) ; stack: [5] + HALT ; returns 5 + + x = 1 (two-expression program) + x + 2 + ───────────── + LOAD_CONST 0 ; stack: [1] + STORE_VAR 0 ; stack: [1] + POP ; stack: [] ← discard intermediate result + LOAD_VAR 0 ; stack: [1] + LOAD_CONST 1 ; stack: [1, 2] + ADD ; stack: [3] + HALT ; returns 3 +``` + +The rule for the compiler is simple: emit each top-level expression, emit `POP` after all but the last. `run()` still returns exactly the top of stack at `HALT`, so `index.ts`'s public signature stays `(source) => number`. + +This matches Lox / clox's pattern exactly. _Crafting Interpreters_ ch.21.4 (globals) and ch.22.4 (locals) both implement the SET opcodes with `peek(0)`, not `pop()`, and Nystrom states the rule directly: "setting a variable doesn't pop the value off the stack. Remember, assignment is an expression, so it needs to leave that value there in case the assignment is nested inside some larger expression" (ch.21.4). Lox's complementary half is that expression statements compile to `; OP_POP` (ch.21.2) — the calculator's "emit POP between top-level expressions" rule is that exact convention adapted to a language with no separate statement form. + +**Alternatives considered:** + +- _Pop in STORE_VAR (Monkey / `Writing a Compiler in Go`, ch.7)_ — rejected. Ball can pop because Monkey's `let` is a syntactic statement, not an expression — no surrounding context ever wants the stack value back. Our language design says assignment **is** an expression (`x = y = 123`, `(x = 1) + 2`), which is the same constraint that drove Nystrom to peek instead of pop. Adopting Ball's opcode shape would be incompatible with the language we agreed on. +- _Have `STORE_VAR` pop, and emit a `DUP` beforehand when the value is needed_ — rejected. Pushes the same logic into the compiler at every assignment site, obscures the "assignment is an expression" intent, and diverges from the cited CI pattern without any compensating benefit. +- _Track per-statement stack effect and elide `POP` when statically zero_ — rejected. Every top-level expression in the calculator produces exactly one value; a uniform `POP`-between rule has no false positives to elide. + +### Decision 4: Environment is owned by the caller; `run` takes an optional env parameter + +The REPL persists variables across lines; a batch caller of `run(source)` does not need to. Pushing the `Environment` out of `run()` into the caller's hands keeps the public API a single function and lets the REPL be a thin loop: + +```ts +// src/v2-calculator/index.ts +export function run( + source: string, + env: Environment = new Environment(), +): number { + return runBytecode(compile(parse(source)), env); +} + +// src/v2-calculator/cli.ts +const env = new Environment(); +rl.on("line", (line) => { + // ... + console.log(run(line, env)); +}); +``` + +The default-argument pattern preserves the existing one-arg call sites (`run("1 + 2")` still works) while making persistence opt-in. + +**Alternatives considered:** + +- _Introduce a `ReplSession` class that owns the env and exposes `evaluate(line)`_ — rejected for scope. That pattern fits `src/v2/` because the REPL there has more state (last-result binding, multi-line buffer, history). The calculator's REPL is six lines of `readline` plumbing; an extra class would be ceremony. +- _Keep the env inside `run()` and expose a separate `runWithEnv(source, env)` overload_ — rejected. Same outcome with two function names instead of one optional parameter. + +### Decision 5: Undefined-variable detection is runtime-only via a names pool + +The bytecode carries a `names: readonly string[]` pool, parallel to `constants`. `LOAD_VAR { index }` and `STORE_VAR { index }` carry an index into this pool; the VM resolves `names[index]` against the `Environment`: + +```ts +// vm/run.ts (excerpt) +case "LOAD_VAR": { + const name = bytecode.names[instr.index]!; + stack.push(env.get(name)); // throws UndefinedVariable if missing + break; +} +case "STORE_VAR": { + const name = bytecode.names[instr.index]!; + env.set(name, stack[stack.length - 1]!); // peek, do not pop + break; +} +case "POP": + stack.pop(); + break; +``` + +The compiler adds a name to the pool the first time it appears (read or write), and reuses the same slot for subsequent occurrences: + +```ts +function nameIndex(name: string): number { + const existing = names.indexOf(name); + if (existing !== -1) return existing; + names.push(name); + return names.length - 1; +} +``` + +Runtime-only detection means the REPL behaves consistently with batch execution: `x` defined on line 1 is visible on line 2 because the same `Environment` is threaded through; the compiler never needs to know what was defined in previous calls. + +**Alternatives considered:** + +- _Compile-time undefined detection inside a single batch_ — rejected. Forces the REPL to either share compiler state across calls or accept divergence between REPL and batch semantics. The calculator's simplicity loses more here than the early-error catches. +- _String-operand opcodes (`LOAD_VAR { name: string }`)_ — rejected. Mirroring the existing `LOAD_CONST { index }` pattern with `constants[]` keeps the bytecode shape uniform; the names pool is the natural sibling to the constants pool. + +### Decision 6: `Environment` is single-scope with one read and one write method + +```ts +// src/v2-calculator/vm/Environment.ts +import { UndefinedVariable } from "./errors"; + +export class Environment { + private readonly records = new Map(); + + get(name: string): number { + if (!this.records.has(name)) throw new UndefinedVariable(name); + return this.records.get(name)!; + } + + set(name: string, value: number): void { + this.records.set(name, value); + } +} +``` + +No `enclosing` chain (the calculator has one global scope). No `define`-vs-`assign` split (no `let` keyword; reassignment is implicit). One `Map` directly because the operand stack is already `number[]` and the calculator has no `Value` union. + +If the calculator ever grows a `let` form or nested scopes, the API can absorb a chained version by adding a constructor parameter and an `enclosing`-walking inner loop — mirroring `src/v2/vm/Environment.ts` exactly. Until then, the minimum surface is the lesson. + +**Alternatives considered:** + +- _Reuse `src/v2/vm/Environment.ts` directly_ — rejected. Cross-tree import couples the calculator to v2's `Value` union (the v2 environment stores `Value`, not `number`). Calculator stays self-contained. +- _Plain `Map` with no class_ — rejected. Encapsulating the `UndefinedVariable` throw inside `get` makes the call sites in `run.ts` exactly one line each, and keeps the door open for the chained variant later without renaming call sites. diff --git a/openspec/changes/archive/2026-05-28-add-variables-to-v2-calculator/proposal.md b/openspec/changes/archive/2026-05-28-add-variables-to-v2-calculator/proposal.md new file mode 100644 index 0000000..76eb0bd --- /dev/null +++ b/openspec/changes/archive/2026-05-28-add-variables-to-v2-calculator/proposal.md @@ -0,0 +1,52 @@ +## Why + +The v2-calculator today accepts exactly one arithmetic expression per program. That ceiling is the first thing every user hits as soon as they want to name an intermediate result — `(1 + 2) * (1 + 2) * (1 + 2)` has no way to factor out the common sub-expression. Variables are the smallest, most fundamental language extension that turns the calculator from a one-shot evaluator into a programmable surface, and they unlock the rest of the calculator's roadmap (`_` binding, built-in constants like `pi`, eventually functions). + +The calculator is deliberately a slimmed-down sibling of `src/v2/`; adding variables here mirrors how `src/v2/vm/Environment.ts` works but at a smaller scale (no lexical chain, no `let`-vs-bare-assignment distinction). That parity is itself part of the learning value of the project. + +## What Changes + +- **Grammar** — `Program` becomes a newline-separated sequence of expressions (`NonemptyListOf`). A new `AssignExp = ident "=" AssignExp | AddExp` rule introduces assignment as a right-associative expression with the **lowest** binding precedence (so `1 + x = 2` is a parse error, not `1 + (x = 2)`). `PriExp` gains an `ident` alternative for variable reads. Newlines become syntactically significant by overriding `space := " " | "\t"`. +- **Identifiers** — `ident = (letter | "_") (letter | digit | "_")*`. Leading underscore is legal (`_`, `_foo` are valid names). +- **AST** — `Program` becomes `readonly Expr[]`. Two new `Expr` kinds: `Var { name }` and `Assign { name; expr }`. +- **Bytecode** — Add `LOAD_VAR { index }`, `STORE_VAR { index }`, and `POP` opcodes. Add a `names: readonly string[]` pool that mirrors the existing `constants` pool. `STORE_VAR` reads top-of-stack but does **not** pop, so assignment behaves as an expression yielding the assigned value. +- **Compiler** — Walks each top-level expression in order, emitting `POP` between them so only the last expression's value survives at `HALT`. Maintains the names pool; each unique identifier first seen (whether as read or write) gets a slot. +- **VM** — Gains an `Environment` (simple `Map`, no enclosing chain) and a new `run(bytecode, env?)` signature. `LOAD_VAR` resolves via the env and throws `UndefinedVariable` when the binding is missing. `STORE_VAR` writes the binding without popping. +- **Public API** — `index.ts`'s `run(source: string, env?: Environment): number`. Caller-owned `Environment` keeps "where state lives" out of `run()`. +- **REPL** — `cli.ts` holds a single `Environment` for the lifetime of the session and passes it on every line. Variables persist across REPL lines. + +## Capabilities + +### Modified Capabilities + +- `v2-calculator`: Grammar, AST, bytecode, VM, public entry point, and REPL all gain variable support. + +### New Capabilities + + + +## Out of Scope (Deferred) + +- **`_` binding to last result.** Listed separately on the calculator todo; not bundled. +- **Built-in constants (`pi`) and functions (`sin`, `cos`, ...).** Need a separate decision on built-in registration; out of scope here. +- **Compile-time undefined-variable detection.** REPL would have to share compiler state across lines to make compile-time detection consistent — out of scope. Single-batch programs accept the same runtime-only behaviour. +- **Lexical scoping / nested scopes.** Calculator has one global scope. No `let`, no blocks. +- **Multi-line expression continuation.** Newlines terminate statements unconditionally — `1 +\n2` is a parse error in this change (it parsed before; this is a deliberate break, see Decision D2 in `design.md`). + +## Impact + +- `src/v2-calculator/parser/grammar.ohm`: new `AssignExp` rule, `Program` becomes a `NonemptyListOf`, `space` is overridden, `ident` and `newline` added, `PriExp` gains an `ident` alternative. +- `src/v2-calculator/parser/grammar.ohm-bundle.{js,d.ts}`: regenerated via the `PostToolUse` hook (or `yarn ohm`). +- `src/v2-calculator/parser/ast.ts`: `Expr` union gains `Var` and `Assign`; `Program` becomes `readonly Expr[]`. +- `src/v2-calculator/parser/semantics.ts`: new actions for `Program` (list iteration), `AssignExp_assign`, `PriExp_var`, `ident`. +- `src/v2-calculator/parser/parser.ts`: no signature change; only `Program` type tightens. +- `src/v2-calculator/compiler/bytecode.ts`: extend `Instruction` union (`LOAD_VAR`, `STORE_VAR`, `POP`); extend `Bytecode` interface with `names: readonly string[]`. +- `src/v2-calculator/compiler/compiler.ts`: emit-per-expression with `POP` between, name-pool bookkeeping. +- `src/v2-calculator/vm/Environment.ts` (NEW): single-scope `Map` with `get`/`set`. +- `src/v2-calculator/vm/errors.ts` (NEW): `UndefinedVariable` error class. +- `src/v2-calculator/vm/run.ts`: optional `env` parameter, three new opcode handlers. +- `src/v2-calculator/cli.ts`: persistent `Environment` across REPL lines. +- `src/v2-calculator/index.ts`: optional `env` parameter on the exported `run`. +- `src/v2-calculator/calculator.test.ts`: feature scenarios for assignment, chained assignment, undefined variable, REPL persistence (via repeated `run` calls sharing an env). +- `src/v2-calculator/parser/parser.test.ts`: shape and parse-error tests for the new grammar. +- After approval + merge: sync delta into `openspec/specs/v2-calculator/spec.md` via the `openspec-sync-specs` skill. diff --git a/openspec/changes/archive/2026-05-28-add-variables-to-v2-calculator/specs/v2-calculator/spec.md b/openspec/changes/archive/2026-05-28-add-variables-to-v2-calculator/specs/v2-calculator/spec.md new file mode 100644 index 0000000..8baf52e --- /dev/null +++ b/openspec/changes/archive/2026-05-28-add-variables-to-v2-calculator/specs/v2-calculator/spec.md @@ -0,0 +1,508 @@ +## MODIFIED Requirements + +### Requirement: Calculator grammar accepts a newline-separated sequence of expressions + +The file `src/v2-calculator/parser/grammar.ohm` SHALL define an Ohm grammar named `Calculator` whose top rule is `Program = NonemptyListOf`. Every legal program SHALL be one or more expressions separated by one or more `"\n"` characters; the final newline before EOF is optional. + +The grammar SHALL override Ohm's default whitespace handling with `space := " " | "\t"` so that newlines are syntactically significant. A `newline = "\n"+` rule SHALL collapse runs of blank lines into a single separator. Empty source (zero expressions) SHALL be a parse error. + +The expression hierarchy SHALL be: + +``` +Expr = AssignExp +AssignExp = ident "=" AssignExp -- assign + | AddExp +AddExp = AddExp "+" MulExp -- plus + | AddExp "-" MulExp -- minus + | MulExp +MulExp = MulExp "*" UnaryExp -- times + | MulExp "/" UnaryExp -- divide + | UnaryExp +UnaryExp = "+" UnaryExp -- pos + | "-" UnaryExp -- neg + | PriExp +PriExp = "(" Expr ")" -- paren + | ident -- var + | number +``` + +The `AssignExp_assign` rule's LHS SHALL be `ident` only — `(x) = 1`, `1 = 2`, and any other non-identifier LHS SHALL be rejected by the grammar without reaching the compiler. The `AssignExp_assign` rule SHALL be right-recursive on its own RHS (chained `x = y = 1` parses as `x = (y = 1)`). + +The `ident` rule SHALL be `(letter | "_") (letter | digit | "_")*`. The `number` rule SHALL remain `digit+ ("." digit+)?`. The leading sign on a literal SHALL be parsed as unary prefix `Unary`, not as part of `number`. + +The grammar SHALL NOT include keywords (no `let`, no `var`, no reserved words), string literals, boolean literals, `none`, array literals, range expressions, comparison or logical operators, postfix call/index/property, function definitions, or any block construct. + +#### Scenario: A single expression on a single line parses + +- **WHEN** the source `1 + 2` is parsed +- **THEN** parsing succeeds +- **AND** the AST is a one-element `Program` whose sole element is `Arithmetic("+", Literal(1), Literal(2))` + +#### Scenario: Two expressions separated by a newline parse + +- **WHEN** the source is `x = 1 + 2\n1 + x * 2` +- **THEN** parsing succeeds +- **AND** the resulting `Program` has length `2` +- **AND** the first element is `Assign("x", Arithmetic("+", Literal(1), Literal(2)))` +- **AND** the second element is `Arithmetic("+", Literal(1), Arithmetic("*", Var("x"), Literal(2)))` + +#### Scenario: Blank lines between expressions are legal + +- **WHEN** the source is `x = 1\n\n\ny = 2` +- **THEN** parsing succeeds +- **AND** the resulting `Program` has length `2` + +#### Scenario: Empty source is a parse error + +- **WHEN** the source is the empty string `""` +- **THEN** parsing fails + +#### Scenario: A bare identifier is a legal expression + +- **WHEN** the source `x` is parsed +- **THEN** parsing succeeds +- **AND** the AST is a one-element `Program` whose sole element is `Var("x")` + +#### Scenario: Identifiers may begin with underscore + +- **WHEN** the source `_foo = 1\n_` is parsed +- **THEN** parsing succeeds +- **AND** the first element is `Assign("_foo", Literal(1))` +- **AND** the second element is `Var("_")` + +#### Scenario: Identifiers may contain underscores and digits after the first character + +- **WHEN** the source `foo_bar = 1\nx1 = 2\nfoo123 = 3` is parsed +- **THEN** parsing succeeds +- **AND** each `Assign` node has the expected name + +#### Scenario: Identifiers may not begin with a digit + +- **WHEN** the source `1foo = 1` is parsed +- **THEN** parsing fails + +#### Scenario: Chained assignment parses right-associatively + +- **WHEN** the source `x = y = 123` is parsed +- **THEN** the AST is `Assign("x", Assign("y", Literal(123)))` + +#### Scenario: Assignment binds looser than addition + +- **WHEN** the source `1 + x = 2` is parsed +- **THEN** parsing fails + +#### Scenario: Parenthesised assignment is a valid sub-expression + +- **WHEN** the source `(x = 1) + 2` is parsed +- **THEN** parsing succeeds +- **AND** the AST is `Arithmetic("+", Assign("x", Literal(1)), Literal(2))` + +#### Scenario: Newlines are no longer ordinary whitespace + +- **WHEN** the source is `1 +\n2` (newline between `+` and `2`) +- **THEN** parsing fails + +#### Scenario: Subtraction remains left-associative + +- **WHEN** the source `1 - 2 - 3` is parsed +- **THEN** the resulting AST element is `Arithmetic("-", Arithmetic("-", Literal(1), Literal(2)), Literal(3))` + +#### Scenario: Multiplication binds tighter than addition + +- **WHEN** the source `1 + 2 * 3` is parsed +- **THEN** the resulting AST element is `Arithmetic("+", Literal(1), Arithmetic("*", Literal(2), Literal(3)))` + +#### Scenario: Unary minus stacks + +- **WHEN** the source `--5` is parsed +- **THEN** the resulting AST element is `Unary("-", Unary("-", Literal(5)))` + +#### Scenario: A trailing operator is a parse error + +- **WHEN** the source `1 +` is parsed +- **THEN** parsing fails + +### Requirement: The AST adds Var and Assign nodes; Program becomes a sequence + +The file `src/v2-calculator/parser/ast.ts` SHALL export the discriminated union: + +```ts +export type Expr = + | { kind: "Literal"; value: number } + | { kind: "Arithmetic"; op: "+" | "-" | "*" | "/"; left: Expr; right: Expr } + | { kind: "Unary"; op: "+" | "-"; expr: Expr } + | { kind: "Var"; name: string } + | { kind: "Assign"; name: string; expr: Expr }; + +export type Program = readonly Expr[]; +``` + +The five discriminant names (`Literal`, `Arithmetic`, `Unary`, `Var`, `Assign`) SHALL be the complete set of `Expr` node kinds. AST nodes SHALL NOT carry source spans, parent pointers, or any field beyond those listed. + +The `Assign` node's `name` field SHALL be the raw identifier string captured by the grammar. The compiler — not the AST — is responsible for resolving names to slots. + +#### Scenario: The five AST kinds are distinct + +- **WHEN** the sources `5`, `1+2`, `-5`, `x`, and `x = 1` are each parsed as one-element programs +- **THEN** the sole element's `kind` is exactly `"Literal"`, `"Arithmetic"`, `"Unary"`, `"Var"`, and `"Assign"` respectively + +#### Scenario: AST nodes have no span field + +- **WHEN** any source is parsed +- **THEN** no AST node in the result contains a key named `span` + +#### Scenario: Program is an array + +- **WHEN** the source `1\n2\n3` is parsed +- **THEN** `program.length` is `3` +- **AND** each element is a `Literal` + +### Requirement: Parser surface mirrors v2's API exactly + +The file `src/v2-calculator/parser/parser.ts` SHALL export `parse(code: string): Program` and `tryParse(code: string): TryParseResult` with the same `TryParseResult` discriminated union as v2: + +```ts +export type TryParseResult = + | { ok: true; program: Program } + | { ok: false; incomplete: true; message: string } + | { ok: false; incomplete: false; message: string }; +``` + +`tryParse` SHALL return `{ ok: false, incomplete: true, … }` when the Ohm match fails at end-of-input (i.e., `matchResult.getRightmostFailurePosition() === code.length`) and `{ ok: false, incomplete: false, … }` otherwise. `parse` SHALL throw `new Error(message)` whenever `tryParse` returns `ok: false`, regardless of the `incomplete` flag. + +The semantics layer (`src/v2-calculator/parser/semantics.ts`) SHALL register a single `toAst` operation that builds the `Program` array and the per-node `Expr` AST defined above. The `Program` action SHALL iterate the `NonemptyListOf` children and collect their `toAst()` results. + +#### Scenario: tryParse flags trailing-operator failure as incomplete + +- **WHEN** `tryParse("1 +")` is called +- **THEN** the result is `{ ok: false, incomplete: true, message: }` + +#### Scenario: tryParse flags mid-expression failure as not incomplete + +- **WHEN** `tryParse("1 1")` is called +- **THEN** the result is `{ ok: false, incomplete: false, message: }` + +#### Scenario: tryParse flags unclosed-paren as incomplete + +- **WHEN** `tryParse("(1 + 2")` is called +- **THEN** the result is `{ ok: false, incomplete: true, message: }` + +#### Scenario: tryParse flags a dangling `=` as incomplete + +- **WHEN** `tryParse("x =")` is called +- **THEN** the result is `{ ok: false, incomplete: true, message: }` + +#### Scenario: parse throws on any failure + +- **WHEN** `parse("1 +")` is called +- **THEN** an `Error` is thrown whose message comes from the Ohm matcher + +#### Scenario: parse returns the AST on success + +- **WHEN** `parse("1 + 2")` is called +- **THEN** the return value is a one-element array `[Arithmetic("+", Literal(1), Literal(2))]` + +### Requirement: Bytecode is a flat instruction list plus a numeric constant pool and a names pool + +The file `src/v2-calculator/compiler/bytecode.ts` SHALL export: + +```ts +export type Instruction = + | { opcode: "LOAD_CONST"; index: number } + | { opcode: "LOAD_VAR"; index: number } + | { opcode: "STORE_VAR"; index: number } + | { opcode: "POP" } + | { opcode: "ADD" } + | { opcode: "SUB" } + | { opcode: "MUL" } + | { opcode: "DIV" } + | { opcode: "NEG" } + | { opcode: "HALT" }; + +export interface Bytecode { + readonly code: readonly Instruction[]; + readonly constants: readonly number[]; + readonly names: readonly string[]; +} +``` + +`STORE_VAR { index }` SHALL document (via TS comment) that it reads the top of the operand stack **without popping** and writes the value into the environment under `names[index]`. The value remains on the stack so that assignment behaves as an expression. `LOAD_VAR { index }` SHALL document that it reads `names[index]` from the environment and pushes the value, throwing `UndefinedVariable` when no binding exists. `POP` SHALL pop and discard the top of the operand stack. + +The compiler SHALL maintain `names` as the unique set of identifiers referenced by the program, in first-seen order. Each unique identifier SHALL occupy exactly one slot regardless of how many times it is read or written. The `constants` pool SHALL continue to permit duplicate numeric entries (no deduplication). + +The compiler SHALL emit each top-level `Expr` of the `Program` in source order. After each emitted expression except the last, the compiler SHALL emit a single `POP` instruction. After the last expression, the compiler SHALL emit `HALT`. + +The grammar guarantees that every legal `Program` has at least one expression, so the emitted code SHALL always end with `… HALT` preceded by at least one expression-producing sequence. + +#### Scenario: A single literal compiles to LOAD_CONST + HALT + +- **WHEN** the source `42` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, HALT]` +- **AND** `bytecode.constants` is `[42]` +- **AND** `bytecode.names` is `[]` + +#### Scenario: A bare assignment compiles to LOAD_CONST + STORE_VAR + HALT + +- **WHEN** the source `x = 5` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, STORE_VAR 0, HALT]` +- **AND** `bytecode.constants` is `[5]` +- **AND** `bytecode.names` is `["x"]` + +#### Scenario: Chained assignment emits multiple STORE_VAR over a single value + +- **WHEN** the source `x = y = 5` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, STORE_VAR 1, STORE_VAR 0, HALT]` +- **AND** `bytecode.names` is `["x", "y"]` + +#### Scenario: A two-expression program emits POP between expressions + +- **WHEN** the source `x = 1\n2` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, STORE_VAR 0, POP, LOAD_CONST 1, HALT]` +- **AND** `bytecode.constants` is `[1, 2]` +- **AND** `bytecode.names` is `["x"]` + +#### Scenario: Variable reads reuse the same names slot as their writes + +- **WHEN** the source `x = 1\nx + x` is compiled +- **THEN** `bytecode.names` is `["x"]` (length `1`) +- **AND** both reads emit `LOAD_VAR 0` + +#### Scenario: Reading an identifier before any write still allocates a slot + +- **WHEN** the source `y` is compiled +- **THEN** `bytecode.code` is `[LOAD_VAR 0, HALT]` +- **AND** `bytecode.names` is `["y"]` +- **AND** compilation does NOT throw + +#### Scenario: Multi-statement final-expression result is the program's value + +- **WHEN** the source `x = 1 + 2\n1 + x * 2` is compiled +- **THEN** the final non-`HALT` instruction sequence ends with `… ADD, HALT` +- **AND** there is exactly one `POP` instruction in the emitted code (between the two top-level expressions) + +#### Scenario: Operator precedence reflects in bytecode order (unchanged) + +- **WHEN** the source `1 + 2 * 3` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, LOAD_CONST 1, LOAD_CONST 2, MUL, ADD, HALT]` + +#### Scenario: Unary minus emits NEG (unchanged) + +- **WHEN** the source `-5` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, NEG, HALT]` + +### Requirement: The VM accepts an optional Environment and resolves variables against it + +The file `src/v2-calculator/vm/run.ts` SHALL export `run(bytecode: Bytecode, env?: Environment): number`. When `env` is omitted, the VM SHALL construct a fresh `Environment` for the duration of the call. The operand stack SHALL remain typed `number[]`. + +The opcode handlers SHALL behave as follows: + +- `LOAD_CONST { index }` — pushes `bytecode.constants[index]`. +- `LOAD_VAR { index }` — resolves `bytecode.names[index]` via `env.get(name)` and pushes the result. If no binding exists in `env`, `env.get` SHALL throw `UndefinedVariable`, which propagates out of `run`. +- `STORE_VAR { index }` — reads `stack[stack.length - 1]` (peek, no pop) and calls `env.set(bytecode.names[index], value)`. +- `POP` — discards the top of the operand stack. +- `ADD` / `SUB` / `MUL` / `DIV` — pop `right`, then `left`, push the binary result. `DIV` uses the native JavaScript `/` operator (no special-casing of `1/0` or `0/0`). +- `NEG` — pops one number, pushes its arithmetic negation. +- `HALT` — pops the top of the stack and returns it. + +The VM SHALL NOT bounds-check the operand stack, the constant pool, the names pool, or the instruction sequence. Stack-effect invariants are the compiler's responsibility. + +The Environment passed in SHALL be observable by the caller after `run` returns: any `STORE_VAR` executed during the call mutates the caller's `Environment`. This is the mechanism by which the REPL persists state across lines. + +#### Scenario: A passed Environment is mutated by STORE_VAR + +- **WHEN** `run(compile(parse("x = 7")), env)` is called with a fresh `Environment` +- **THEN** the return value is `7` +- **AND** `env.get("x")` returns `7` after the call + +#### Scenario: A second run sees variables written by the first + +- **WHEN** `run(compile(parse("x = 5")), env)` is called +- **AND** then `run(compile(parse("x + 1")), env)` is called with the same `env` +- **THEN** the second call returns `6` + +#### Scenario: An undefined variable throws UndefinedVariable + +- **WHEN** `run(compile(parse("y")))` is called with a fresh `Environment` +- **THEN** an `UndefinedVariable` error is thrown +- **AND** the error's `name` property is `"y"` + +#### Scenario: A read of a defined variable returns its value + +- **WHEN** `run(compile(parse("x = 3\nx + 4")))` is called +- **THEN** the return value is `7` + +#### Scenario: Sum of two literals (unchanged) + +- **WHEN** `run(compile(parse("1 + 2")))` is called +- **THEN** the result is `3` + +#### Scenario: Division-by-zero produces Infinity (unchanged) + +- **WHEN** `run(compile(parse("1 / 0")))` is called +- **THEN** the result is `Infinity` + +#### Scenario: Zero-divided-by-zero produces NaN (unchanged) + +- **WHEN** `run(compile(parse("0 / 0")))` is called +- **THEN** the result satisfies `Number.isNaN(result)` + +#### Scenario: Reassignment overwrites the prior binding + +- **WHEN** `run(compile(parse("x = 1\nx = 2\nx")))` is called +- **THEN** the result is `2` + +#### Scenario: Assignment is an expression yielding the assigned value + +- **WHEN** `run(compile(parse("1 + (x = 2)")))` is called +- **THEN** the result is `3` +- **AND** after the call, the program's `Environment` has `x = 2` + +### Requirement: The public entry point composes parse, compile, and run with an optional Environment + +The file `src/v2-calculator/index.ts` SHALL export a function with signature `run(source: string, env?: Environment): number` that composes `parse`, `compile`, and the VM's `run`: + +```ts +export function run(source: string, env?: Environment): number { + return runBytecode(compile(parse(source)), env); +} +``` + +When `env` is omitted, each call SHALL operate on a fresh `Environment` — `run("x = 1")` followed by `run("x")` (no shared env) SHALL throw `UndefinedVariable` on the second call. When `env` is supplied, the same `Environment` instance SHALL be reused, and variables persist across calls. + +The function SHALL be the only public surface re-exported from `index.ts`. `parse`, `compile`, `tryParse`, `Bytecode`, `Instruction`, `Environment`, `UndefinedVariable`, and other layer types SHALL remain accessible only through their respective layer modules (`./parser`, `./compiler/...`, `./vm/run`, `./vm/Environment`, `./vm/errors`). The function SHALL propagate parse-time, compile-time, and runtime errors as thrown `Error` objects with no wrapping, swallowing, or transformation. + +#### Scenario: Numeric result is returned as a JavaScript number + +- **WHEN** `run("1.5 + 0.5")` is called +- **THEN** the return value is `2` + +#### Scenario: A shared Environment lets variables persist across calls + +- **WHEN** an `Environment` is created +- **AND** `run("x = 10", env)` is called +- **AND** `run("x * 2", env)` is then called with the same `env` +- **THEN** the second call returns `20` + +#### Scenario: Omitting the Environment isolates calls + +- **WHEN** `run("x = 1")` is called (no env) +- **AND** `run("x")` is then called (no env) +- **THEN** the second call throws `UndefinedVariable` + +#### Scenario: Parse error propagates as a thrown Error + +- **WHEN** `run("1 +")` is called +- **THEN** an `Error` is thrown whose message comes from the Ohm parse-error + +### Requirement: `yarn lucky-calculator` launches a REPL that persists variables across lines + +The file `package.json` SHALL retain the script `"lucky-calculator": "ts-node src/v2-calculator/cli.ts"`. Running `yarn lucky-calculator` SHALL start an interactive REPL with the following contract: + +- The prompt SHALL be `> ` for every input. There SHALL NOT be a continuation prompt; the prompt SHALL NOT change shape across lines. +- The REPL SHALL parse each line standalone; multi-line continuation SHALL NOT be implemented. The `tryParse.incomplete` flag SHALL be ignored at the REPL level — a parse failure for any reason is reported and the prompt re-displayed. +- The bare typed words `exit` or `quit` (after `trim()`) SHALL exit the REPL with code `0`. +- Ctrl-D (the readline `close` event) SHALL exit the REPL with code `0`. +- A successful evaluation SHALL print the result with `console.log(result)`. No formatting SHALL be applied; the JavaScript `String(number)` representation passes through unchanged. +- A parse error SHALL be printed to `console.error` (the parse-error message string) and the prompt SHALL be re-displayed. +- A runtime-thrown `Error` (including `UndefinedVariable`) SHALL be printed to `console.error` (the error's `message`) and the prompt SHALL be re-displayed. +- An empty or whitespace-only line SHALL be treated as a no-op — the prompt is re-displayed without evaluation. + +The REPL SHALL hold a single `Environment` instance for the entire session and pass it to every `run` call. Variables written by a successful evaluation SHALL be visible to subsequent lines. A line that throws (parse or runtime) SHALL NOT roll back partial writes — assignments that completed before the throw remain in the `Environment`; this falls out of the VM operating directly on the live `Environment`. + +The REPL SHALL NOT bind a `_` to the last result; that feature is a separate change. + +#### Scenario: A simple expression evaluates and prints + +- **WHEN** the user types `1 + 2` at the prompt +- **THEN** the REPL prints `3` to stdout +- **AND** re-displays the `> ` prompt + +#### Scenario: A variable defined on one line is visible on the next + +- **WHEN** the user types `x = 5` at the prompt +- **AND** then types `x + 10` at the next prompt +- **THEN** the REPL prints `5` to stdout for the first line +- **AND** prints `15` to stdout for the second line + +#### Scenario: An undefined variable reports an error and the REPL continues + +- **WHEN** the user types `y` at the prompt with no prior assignment to `y` +- **THEN** the REPL prints an `UndefinedVariable` message to stderr +- **AND** re-displays the `> ` prompt +- **AND** the process does NOT exit + +#### Scenario: A parse error is reported and the REPL continues + +- **WHEN** the user types `1 +` at the prompt +- **THEN** the REPL prints the Ohm parse-error message to stderr +- **AND** re-displays the `> ` prompt +- **AND** the process does NOT exit + +#### Scenario: Reassignment is visible to subsequent lines + +- **WHEN** the user types `x = 1`, then `x = 2`, then `x` +- **THEN** the third line prints `2` + +#### Scenario: `exit` closes the REPL + +- **WHEN** the user types `exit` at the prompt +- **THEN** the REPL exits with code `0` + +#### Scenario: `quit` closes the REPL + +- **WHEN** the user types `quit` at the prompt +- **THEN** the REPL exits with code `0` + +#### Scenario: An empty line re-prompts without output + +- **WHEN** the user presses Enter on an empty prompt +- **THEN** the REPL re-displays `> ` +- **AND** nothing is printed to stdout or stderr + +## ADDED Requirements + +### Requirement: Variable storage is a single-scope Environment with explicit undefined-read errors + +The file `src/v2-calculator/vm/Environment.ts` SHALL export a class `Environment` whose state is a single `Map`. There SHALL NOT be an `enclosing` chain, a `define`-vs-`assign` distinction, or any `Value` wrapper around stored numbers. + +The class SHALL expose exactly two methods: + +```ts +export class Environment { + get(name: string): number; // throws UndefinedVariable when name is unbound + set(name: string, value: number): void; // create-or-overwrite +} +``` + +`get(name)` SHALL throw `UndefinedVariable` (a new error class exported from `src/v2-calculator/vm/errors.ts`) when no binding exists for `name`. The thrown error SHALL carry a public `name: string` property equal to the queried identifier and a `message` of the form `undefined variable: `. + +`set(name, value)` SHALL be idempotent on the binding's existence — it creates a new binding when absent and overwrites the existing one when present. There SHALL NOT be a separate "redefine" error; reassignment is the default. + +The `Environment` SHALL be the sole owner of variable state. The compiler SHALL NOT carry state across `compile()` calls; the REPL achieves persistence by sharing an `Environment` instance across `run()` calls, not by sharing compiler context. + +#### Scenario: A new Environment starts empty + +- **WHEN** a new `Environment` is constructed +- **AND** `env.get("x")` is called +- **THEN** `UndefinedVariable` is thrown +- **AND** the thrown error's `name` property is `"x"` + +#### Scenario: set followed by get returns the stored value + +- **WHEN** `env.set("x", 42)` is called +- **AND** `env.get("x")` is then called +- **THEN** the returned value is `42` + +#### Scenario: set overwrites an existing binding + +- **WHEN** `env.set("x", 1)` is called +- **AND** `env.set("x", 2)` is then called +- **AND** `env.get("x")` is then called +- **THEN** the returned value is `2` + +#### Scenario: UndefinedVariable carries the queried name + +- **WHEN** `env.get("not_bound")` is called on a fresh `Environment` +- **THEN** an `UndefinedVariable` is thrown +- **AND** the thrown error's `name` property equals `"not_bound"` +- **AND** the thrown error's `message` equals `"undefined variable: not_bound"` diff --git a/openspec/changes/archive/2026-05-28-add-variables-to-v2-calculator/tasks.md b/openspec/changes/archive/2026-05-28-add-variables-to-v2-calculator/tasks.md new file mode 100644 index 0000000..10ca7ec --- /dev/null +++ b/openspec/changes/archive/2026-05-28-add-variables-to-v2-calculator/tasks.md @@ -0,0 +1,107 @@ +## 1. Grammar + +- [x] 1.1 In `src/v2-calculator/parser/grammar.ohm`: override `space := " " | "\t"` and add `newline = "\n"+` +- [x] 1.2 In `src/v2-calculator/parser/grammar.ohm`: change the top rule to `Program = NonemptyListOf` and introduce `Expr = AssignExp` +- [x] 1.3 In `src/v2-calculator/parser/grammar.ohm`: add `AssignExp = ident "=" AssignExp -- assign | AddExp` (right-recursion on the RHS gives right-associativity; LHS restricted to `ident` so non-identifier targets fail at parse time) +- [x] 1.4 In `src/v2-calculator/parser/grammar.ohm`: add `PriExp_var = ident` alternative ahead of `number` in `PriExp` +- [x] 1.5 In `src/v2-calculator/parser/grammar.ohm`: add `ident = (letter | "_") (letter | digit | "_")*` +- [x] 1.6 Confirm the `PostToolUse` hook regenerated `grammar.ohm-bundle.{js,d.ts}`; if not, run `yarn ohm` manually. Verify `yarn typecheck` passes against the regenerated bundle. + +## 2. AST Types + +- [x] 2.1 In `src/v2-calculator/parser/ast.ts`: extend `Expr` with `{ kind: "Var"; name: string }` and `{ kind: "Assign"; name: string; expr: Expr }` +- [x] 2.2 Change `Program` from `Expr` to `readonly Expr[]` +- [x] 2.3 Confirm no other file imports `Program` expecting a single `Expr` (touches `index.ts`, `parser.ts`, `compiler.ts`, tests — verify each) + +## 3. Ohm Semantics + +- [x] 3.1 In `src/v2-calculator/parser/semantics.ts`: replace the `Program(expr)` action with a `Program(nelist)` action that calls `nelist.asIteration().children.map(c => c.toAst() as Expr)` and returns the array +- [x] 3.2 Add an `AssignExp_assign(name, _eq, rhs)` action returning `{ kind: "Assign", name: name.sourceString, expr: rhs.toAst() as Expr }` +- [x] 3.3 Add a `PriExp_var(idNode)` action returning `{ kind: "Var", name: idNode.sourceString }` +- [x] 3.4 Add an `ident(_first, _rest)` action — even if it just returns `sourceString`, declare it so Ohm's exhaustive-action check stays green +- [x] 3.5 If Ohm requires it, add no-op actions for `newline` and the `NonemptyListOf` iteration node so the semantics dictionary is complete + +## 4. Parser Tests + +- [x] 4.1 In `src/v2-calculator/parser/parser.test.ts`: add shape tests for `x` (one-element `Program` of `Var("x")`), `x = 1` (`Assign("x", Literal(1))`), `x = y = 1` (right-assoc nesting), `(x = 1) + 2`, `_foo = 1`, `foo_bar = 1`, `foo123 = 1` +- [x] 4.2 Add shape test for multi-line program: `x = 1 + 2\n1 + x * 2` (two-element `Program`, second element is the arithmetic tree with `Var("x")`) +- [x] 4.3 Add shape test confirming blank lines collapse: `x = 1\n\n\ny = 2` parses as a two-element `Program` +- [x] 4.4 Add parse-error tests for: `1 + x = 2`, `1foo = 1`, `1 +\n2` (newline inside expression), `""` (empty source), `(x) = 1` +- [x] 4.5 Add `tryParse` tests confirming `tryParse("x =")` returns `{ ok: false, incomplete: true, ... }` + +## 5. Compiler — Bytecode Shape + +- [x] 5.1 In `src/v2-calculator/compiler/bytecode.ts`: extend the `Instruction` union with `{ opcode: "LOAD_VAR"; index: number }`, `{ opcode: "STORE_VAR"; index: number }`, and `{ opcode: "POP" }` +- [x] 5.2 Add a TS doc comment on `STORE_VAR` stating it reads top-of-stack without popping (assignment is an expression — see `design.md` Decision 3 and `Crafting Interpreters` ch.21.4) +- [x] 5.3 Add a TS doc comment on `LOAD_VAR` stating it throws `UndefinedVariable` if the name has no binding +- [x] 5.4 Add `names: readonly string[]` to the `Bytecode` interface alongside `constants` + +## 6. Compiler — Emission + +- [x] 6.1 In `src/v2-calculator/compiler/compiler.ts`: change the entry point to iterate the `Program` array, emit each `Expr`, emit a single `POP` between expressions (not after the last), then emit `HALT` +- [x] 6.2 Add a local `names: string[]` array and a `nameIndex(name)` helper that returns the existing slot or appends a new one +- [x] 6.3 Add an `emit(Var)` case that emits `LOAD_VAR { index: nameIndex(name) }` +- [x] 6.4 Add an `emit(Assign)` case that compiles `expr.expr` first, then emits `STORE_VAR { index: nameIndex(expr.name) }` +- [x] 6.5 Confirm the existing `Literal` / `Arithmetic` / `Unary` cases continue to work unchanged with the new walker shape +- [x] 6.6 Confirm the returned `Bytecode` includes the new `names` field + +## 7. Compiler Tests + +- [x] 7.1 If there is no compiler test file yet, create `src/v2-calculator/compiler/compiler.test.ts`; otherwise extend it +- [x] 7.2 Bytecode test for `x = 5`: `[LOAD_CONST 0, STORE_VAR 0, HALT]`, `constants = [5]`, `names = ["x"]` +- [x] 7.3 Bytecode test for `x = y = 5`: `[LOAD_CONST 0, STORE_VAR 1, STORE_VAR 0, HALT]`, `names = ["x", "y"]` (compile order of slots — outer assign appends `x` first, then inner appends `y` when it compiles; verify against the actual `nameIndex` walk order chosen in 6.2) +- [x] 7.4 Bytecode test for `x = 1\n2`: `[LOAD_CONST 0, STORE_VAR 0, POP, LOAD_CONST 1, HALT]`, `names = ["x"]` +- [x] 7.5 Bytecode test for `x = 1\nx + x`: confirms `names` has length `1` and both reads emit `LOAD_VAR 0` +- [x] 7.6 Bytecode test for `y` (read-before-write): `[LOAD_VAR 0, HALT]`, `names = ["y"]`, compilation does NOT throw + +## 8. VM — Environment + Errors + +- [x] 8.1 Create `src/v2-calculator/vm/errors.ts` exporting `class UndefinedVariable extends Error` with a public `name: string` field and message `"undefined variable: "` +- [x] 8.2 Create `src/v2-calculator/vm/Environment.ts` with a single private `Map`, a `get(name)` that throws `UndefinedVariable` on miss, and a `set(name, value)` create-or-overwrite +- [x] 8.3 Add unit tests `src/v2-calculator/vm/Environment.test.ts` covering: empty-env get throws, set-then-get returns value, set overwrites, error's `name` and `message` fields + +## 9. VM — Run + +- [x] 9.1 In `src/v2-calculator/vm/run.ts`: change the signature to `run(bytecode: Bytecode, env: Environment = new Environment()): number` +- [x] 9.2 Implement `LOAD_VAR { index }`: push `env.get(bytecode.names[index]!)` +- [x] 9.3 Implement `STORE_VAR { index }`: read `stack[stack.length - 1]!` (peek), call `env.set(bytecode.names[index]!, value)`. Do NOT pop. +- [x] 9.4 Implement `POP`: `stack.pop()`, discard +- [x] 9.5 Leave the existing `LOAD_CONST` / `ADD` / `SUB` / `MUL` / `DIV` / `NEG` / `HALT` handlers unchanged +- [x] 9.6 Confirm `UndefinedVariable` propagates out of `run` without wrapping + +## 10. Public Entry Point + +- [x] 10.1 In `src/v2-calculator/index.ts`: change `run(source: string)` to `run(source: string, env?: Environment)`; thread `env` through to `runBytecode` +- [x] 10.2 Confirm no transformation or swallowing of thrown errors + +## 11. REPL + +- [x] 11.1 In `src/v2-calculator/cli.ts`: construct one `Environment` outside the `rl.on("line", ...)` callback so it persists across lines +- [x] 11.2 Pass the persistent `env` into `runBytecode(compile(parsed.program), env)` on every successful parse +- [x] 11.3 Confirm the existing error-handling branches (parse failure, runtime throw) re-display the prompt without exiting — including the new `UndefinedVariable` runtime error path +- [x] 11.4 Smoke-test the REPL manually: `x = 5\nx + 10` prints `5` then `15`; `y` prints an `UndefinedVariable` message and continues; `exit` quits cleanly + +## 12. Integration Tests + +- [x] 12.1 In `src/v2-calculator/calculator.test.ts`: end-to-end test `run("x = 1 + 2\n1 + x * 2") === 7` +- [x] 12.2 End-to-end test for chained assignment: `run("x = y = 5\nx + y") === 10` +- [x] 12.3 End-to-end test for assignment-as-expression: `run("1 + (x = 2)") === 3`, then a second call with the same `env` confirms `x` is `2` +- [x] 12.4 End-to-end test: `run("y")` (no env) throws `UndefinedVariable` whose `name` is `"y"` +- [x] 12.5 End-to-end test for shared-env persistence: build one `Environment`, call `run("x = 10", env)`, then `run("x * 2", env)` returns `20` +- [x] 12.6 End-to-end test for isolation: `run("x = 1")` followed by `run("x")` (neither sharing env) — the second throws `UndefinedVariable` +- [x] 12.7 End-to-end test for reassignment: `run("x = 1\nx = 2\nx") === 2` + +## 13. Documentation + +- [x] 13.1 In `src/v2-calculator/todo.md`: tick the first item (`Add variables, like x = 1 + 2 (no let keyword for simplicity)`) +- [x] 13.2 If `src/v2-calculator/` has a README or notes file, update it to mention variable support; otherwise no action + +## 14. Quality Gate + +- [x] 14.1 Run `yarn lint` and fix all violations +- [x] 14.2 Run `yarn typecheck` and fix all errors +- [x] 14.3 Run `yarn test` and confirm the entire suite is green (not just the v2-calculator slice) + +## 15. Post-Merge + +- [x] 15.1 After approval and merge, sync the delta spec into `openspec/specs/v2-calculator/spec.md` using the `openspec-sync-specs` skill diff --git a/openspec/changes/archive/2026-05-28-refactor-v2-calculator-environment-to-injected-builtins/.openspec.yaml b/openspec/changes/archive/2026-05-28-refactor-v2-calculator-environment-to-injected-builtins/.openspec.yaml new file mode 100644 index 0000000..352690f --- /dev/null +++ b/openspec/changes/archive/2026-05-28-refactor-v2-calculator-environment-to-injected-builtins/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-05-28 diff --git a/openspec/changes/archive/2026-05-28-refactor-v2-calculator-environment-to-injected-builtins/design.md b/openspec/changes/archive/2026-05-28-refactor-v2-calculator-environment-to-injected-builtins/design.md new file mode 100644 index 0000000..46c5f76 --- /dev/null +++ b/openspec/changes/archive/2026-05-28-refactor-v2-calculator-environment-to-injected-builtins/design.md @@ -0,0 +1,158 @@ +## Context + +The previous change, `add-builtin-constants-to-v2-calculator` (archived `2026-05-28`), shipped `pi`/`e`/`tau` by having `Environment.ts` import `BUILTIN_CONSTANTS` at module level and pre-populate its internal `Map` in the constructor. That change explicitly considered and rejected the alternative of injecting `BUILTIN_CONSTANTS` via constructor parameter, on the rationale that "every caller would then need to know about the constants registry, leaking policy across the codebase." + +A follow-up `/grill-me` session walked the design tree again and reopened that decision. The new conclusion: the previous tradeoff understated the value of dependency inversion and overstated the cost. Specifically: + +- `Environment.ts` reaching outward to `builtins.ts` is the kind of implicit coupling that becomes load-bearing the moment a second consumer wants different built-ins (e.g. a "scientific" REPL, an embedded calculator hosted by a parent app, or — more concretely — built-in functions arriving in the next change). +- The "every caller would need to know" cost is real but smaller than it appeared: there are exactly _two_ call sites that need to wire `BUILTIN_CONSTANTS` (`index.ts` and `cli.ts`), and they are precisely the natural composition roots of the calculator. Pushing the wiring up to those roots makes the dependency _visible at the layer where calculator policy lives_. + +This change inverts the dependency. `Environment` becomes policy-free mechanism; `index.ts` and `cli.ts` become explicit composition roots. The previous change's spec delta is rewritten accordingly. + +## Goals / Non-Goals + +**Goals:** + +- `Environment` has no module-level dependency on `BUILTIN_CONSTANTS`. The class is pure storage with optional initial bindings. +- `vm/run.ts` requires `env` — the VM is mechanism, it does not synthesize calculator defaults. +- `index.ts` and `cli.ts` are the SOLE composition roots that wire `BUILTIN_CONSTANTS`. They are the only modules outside `vm/builtins.ts` that import it. +- **Behavior at the public API is unchanged.** `run("pi")` still returns `Math.PI`. `pi = 5` still shadows. The REPL still has `pi`/`e`/`tau` available on startup. +- The 7 end-to-end tests in `calculator.test.ts` pass after refactor with at most one trivial wording adjustment to the shadowing test. The integration assertions remain the truth. + +**Non-Goals:** + +- Built-in functions (`sin`, `cos`, …) — still a separate change. `Environment`'s value type remains `number`. +- A centralized factory like `createCalculatorEnvironment()` — two composition-root calls are clearer than three (the factory, plus the two callers). +- Splitting `Environment` into `Storage` + `CalculatorEnvironment`, or any name change — the existing class absorbs the DI shape fine. +- Compile-time detection of built-in names — still runtime-only. +- Marking built-ins as protected against reassignment — still reassignable. +- Removing the `BUILTIN_CONSTANTS` `Object.freeze` — still frozen. + +## Decisions + +### Decision 1: `Environment`'s constructor takes an optional `initialBindings` parameter defaulting to `{}` + +```ts +// src/v2-calculator/vm/Environment.ts (post-change) +import { UndefinedVariable } from "./errors"; + +export class Environment { + private readonly records = new Map(); + + constructor(initialBindings: Readonly> = {}) { + for (const [name, value] of Object.entries(initialBindings)) { + this.records.set(name, value); + } + } + + get(name: string): number { + if (!this.records.has(name)) throw new UndefinedVariable(name); + return this.records.get(name)!; + } + + set(name: string, value: number): void { + this.records.set(name, value); + } +} +``` + +Consequences: + +- `new Environment()` produces a genuinely empty env. No `pi`, no `e`, no `tau`. +- `new Environment(BUILTIN_CONSTANTS)` produces an env pre-populated with the calculator's built-ins. +- `new Environment({ foo: 1, bar: 2 })` produces an env with arbitrary pre-bindings — useful for tests, useful for any future caller (an embedded calculator library, a "scientific" mode) that wants a different starting set. +- The `BUILTIN_CONSTANTS` import is **removed** from `Environment.ts`. The class no longer knows about calculator policy. + +**Alternatives considered:** + +- _Required parameter (no default)_ — **rejected**. Forces all 17 `new Environment()` call sites (mostly in tests) to pass an empty object. The default `= {}` keeps the storage-mechanism tests one-liner without weakening DI purity — `Environment.ts` still does not import `BUILTIN_CONSTANTS`. +- _Optional parameter defaulting to `BUILTIN_CONSTANTS`_ — **rejected**. This was the "trap option" from the grilling: it preserves the module-level dependency behind a default arg, so it has the same coupling as the previous design while adding the appearance of DI. Worst of both worlds. +- _Take an `Iterable<[string, number]>` instead of `Readonly>`_ — **rejected**. The current `BUILTIN_CONSTANTS` _is_ a `Record`. Generalizing the parameter type for hypothetical future input shapes is YAGNI; the `Record` type matches the data we have. + +### Decision 2: `vm/run.ts` makes `env` a required parameter (no default) + +```ts +// src/v2-calculator/vm/run.ts (post-change) +export function run(bytecode: Bytecode, env: Environment): number { … } +``` + +The VM is mechanism; it interprets bytecode against a passed-in environment. Synthesizing a default `new Environment()` here would either silently produce an empty env (footgun) or require importing `BUILTIN_CONSTANTS` (layering violation — the VM is not the calculator). + +Consequences: + +- Anyone calling `runBytecode(bc)` without an env now gets a TypeScript error at compile time. The fix is `runBytecode(bc, env)` with an explicit env — which is the correct behavior. +- `index.ts`'s public `run` becomes the only place that defaults an env into existence. + +**Alternatives considered:** + +- _Keep the default `new Environment()` (now empty)_ — **rejected**. The default is misleading after the refactor: a caller who relies on the default and writes `runBytecode(bc)` silently gets no built-ins. Making the parameter required forces the caller to think about the env once, and the type system enforces it. +- _Default to `new Environment(BUILTIN_CONSTANTS)`_ — **rejected**. The VM would have to import calculator policy. Layering violation. + +### Decision 3: `index.ts` and `cli.ts` are the SOLE composition roots + +```ts +// src/v2-calculator/index.ts (post-change) +import { compile } from "./compiler/compiler"; +import { parse } from "./parser/parser"; +import { BUILTIN_CONSTANTS } from "./vm/builtins"; +import { Environment } from "./vm/Environment"; +import { run as runBytecode } from "./vm/run"; + +export function run( + source: string, + env: Environment = new Environment(BUILTIN_CONSTANTS), +): number { + return runBytecode(compile(parse(source)), env); +} + +// src/v2-calculator/cli.ts (post-change) +import { BUILTIN_CONSTANTS } from "./vm/builtins"; +import { Environment } from "./vm/Environment"; +// … +const env = new Environment(BUILTIN_CONSTANTS); +``` + +Two composition roots, two imports of `BUILTIN_CONSTANTS`. No other module in the calculator imports `BUILTIN_CONSTANTS`. The wiring is concentrated at the two boundaries where the calculator "begins" — the public function and the REPL. + +Consequences: + +- Behavior at the public API is unchanged. `run("pi")` still returns `Math.PI` because `index.ts` defaults `env` to a built-in-loaded env. +- A caller can opt out by passing `new Environment()` explicitly: `run("pi", new Environment())` throws `UndefinedVariable`. This is a _new_ affordance — previously impossible — and is the testability angle that DI delivers as a side benefit. + +**Alternatives considered:** + +- _A single factory function `createCalculatorEnvironment()` that wraps `new Environment(BUILTIN_CONSTANTS)`_ — **rejected** on YAGNI. Two `new Environment(BUILTIN_CONSTANTS)` calls are clearer than a factory used in two places. If a third or fourth caller arrives later (e.g. a benchmarking script, a test harness), introduce the factory then. +- _Only `index.ts` is a composition root; `cli.ts` calls `index.ts`'s `run` directly_ — **rejected**. The REPL persists an `Environment` across many `run` calls; the env has to live in the REPL loop, not be constructed per call. So `cli.ts` necessarily owns its env, and is necessarily a composition root in its own right. + +### Decision 4: Slim `Environment.test.ts` to mechanism only; keep integration in `calculator.test.ts` + +After the refactor, most of the "built-in constants" tests added to `Environment.test.ts` in the previous change become trivial (`new Environment({pi: 1}).get("pi") === 1` is just testing that the constructor copies its input map into storage). They are dropped. A single replacement test covers the new constructor mechanism: + +```ts +it("copies initialBindings into storage on construction", () => { + const env = new Environment({ foo: 1, bar: 2 }); + expect(env.get("foo")).toBe(1); + expect(env.get("bar")).toBe(2); +}); +``` + +The 7 integration assertions in `calculator.test.ts` (`run("pi")` returns `Math.PI`, etc.) stay where they are; they are the truth about calculator wiring. One scenario — the shadowing-through-pipeline test — is rewritten to use the default `run()` rather than passing its own empty env, because the empty env no longer matches the test's narrative ("user shadows a built-in"; you can only shadow something that's there). + +The `BUILTIN_CONSTANTS` frozen-invariant test is dropped. `Object.freeze` is a language guarantee enforced by the runtime; asserting it in a project test is defensive against a non-failure mode. + +**Alternatives considered:** + +- _Keep the existing 8 tests, rewrite each to pass `BUILTIN_CONSTANTS` explicitly_ — **rejected**. They become tests of "the constructor copies its input", repeated eight times with the same input. Low signal-per-test. +- _Move all built-in tests to a new `builtins.test.ts`_ — **rejected**. The integration tests are already in `calculator.test.ts` and they correctly exercise the composition. A new test file adds ceremony without value. + +## Risks / Trade-offs + +- **Risk**: A direct external caller of `vm/run.ts` (rare, but possible if someone writes a benchmark or a tool) now sees a TypeScript error where they previously didn't. → **Mitigation**: This is a TS error, not a runtime regression — the fix is to construct an env. The error is exactly the awareness DI delivers; treat as feature, not regression. +- **Risk**: Anyone who calls `new Environment()` (with no args) post-refactor gets a _truly_ empty env, which previously had `pi`/`e`/`tau`. → **Mitigation**: Only test code does this. `cli.ts` and `index.ts` (the production callers) are updated to pass `BUILTIN_CONSTANTS`. The change is loud where it matters and inert everywhere else. +- **Risk**: The just-synced main spec gets four MODIFIED requirements; the spec-sync skill has to apply replacements over the previous sync. → **Mitigation**: The MODIFIED blocks include full updated content per the spec rules, and the previous sync's titles are preserved as matching anchors. Sync is straightforward. +- **Risk**: A future reader looks at the git history and sees "add built-ins" → "refactor built-ins" two changes apart and wonders why the refactor didn't happen in the first change. → **Mitigation**: Honest narrative; the design.md of _this_ change explains the reopening. Two small, well-scoped changes are better than one in-flight design revision. +- **Trade-off**: The "right" design surfaced only after implementation. That's not failure — that's the value of grill-me. But it does mean the previous change shipped a design that was superseded within hours. → Accepted as the cost of moving fast on a learning project. + +## Open Questions + +None. The grill-me session resolved the design tree end-to-end. If `BUILTIN_FUNCTIONS` later requires widening `Environment`'s value type, that's a separate change with its own grilling. diff --git a/openspec/changes/archive/2026-05-28-refactor-v2-calculator-environment-to-injected-builtins/proposal.md b/openspec/changes/archive/2026-05-28-refactor-v2-calculator-environment-to-injected-builtins/proposal.md new file mode 100644 index 0000000..c0d255a --- /dev/null +++ b/openspec/changes/archive/2026-05-28-refactor-v2-calculator-environment-to-injected-builtins/proposal.md @@ -0,0 +1,46 @@ +## Why + +The previous change (`add-builtin-constants-to-v2-calculator`) shipped `pi`, `e`, `tau` by having `Environment.ts` import `BUILTIN_CONSTANTS` at module level and pre-populate its internal `Map` from that import. That works, but it ties the storage class — which is otherwise pure mechanism — to a piece of calculator-specific policy (which names are pre-bound). The dependency is implicit: there is no signature that says "this env carries built-ins"; it just does, because `Environment.ts` reaches outward to `builtins.ts`. + +Inverting that dependency removes the implicit module-level coupling and moves the wiring to the two natural composition roots — `index.ts` (the calculator's public `run` function) and `cli.ts` (the REPL). After this change, `Environment` is policy-free: it stores whatever bindings you initialize it with, and _nothing else_. The fact that the calculator wires `pi`/`e`/`tau` into its default env is a fact about the calculator's _composition_, not the storage class. + +This is the path the previous change's design.md explicitly considered and deferred ("Pass the constants map into Environment's constructor as a parameter — rejected"). After implementation and a follow-up `/grill-me` session that walked the trade-offs, we're reopening that decision. The motivation is DI purity, not testability or future-proofing for built-in functions — the latter is its own change. + +## What Changes + +- **`vm/Environment.ts`** — Constructor gains an optional parameter: `constructor(initialBindings: Readonly> = {})`. When the caller passes nothing, the env starts genuinely empty. `Environment.ts` SHALL NO LONGER import `BUILTIN_CONSTANTS`; the dependency is inverted. +- **`vm/run.ts`** — `env` becomes **required** (no default). The VM is mechanism; it does not synthesize defaults. Any call site that wants a usable calculator env must construct one explicitly. +- **`index.ts`** — Becomes a composition root. Imports `BUILTIN_CONSTANTS`. The public `run` signature stays `run(source: string, env?: Environment): number`, but the default is now `new Environment(BUILTIN_CONSTANTS)` rather than `new Environment()`. Behavior at the public API is **unchanged** — `run("pi")` still returns `Math.PI`. +- **`cli.ts`** — Becomes the second composition root. Imports `BUILTIN_CONSTANTS`. Constructs `new Environment(BUILTIN_CONSTANTS)` at REPL startup. +- **Tests** — `vm/Environment.test.ts` sheds the 8 "built-in constants" describe block; replaces it with a single "constructor stores initial bindings" mechanism test. The integration assertions (`run("pi")` returns `Math.PI`, shadowing through the pipeline, …) stay in `calculator.test.ts` where they already are. The `BUILTIN_CONSTANTS` frozen-invariant test is dropped — `Object.freeze` is a language guarantee, not a project invariant worth asserting in a test suite that doesn't test the runtime. +- **`BUILTIN_CONSTANTS` module** — Unchanged. Same three entries, same frozen invariant. +- **Behavior at the public API** — Unchanged. `run("pi")` still returns `Math.PI`. `pi = 5` still shadows. Two `run` calls with separate envs are still isolated. + +## Capabilities + +### New Capabilities + + + +### Modified Capabilities + +- `v2-calculator`: `Environment` constructor signature changes; `vm/run.ts` requires `env`; the wiring of `BUILTIN_CONSTANTS` moves from the storage class to the two composition roots (`index.ts` and `cli.ts`). + +## Out of Scope (Deferred) + +- **Widening `Environment`'s value type to support built-in functions (`sin`, `cos`, …).** Still a separate change, still gated on the built-in-functions todo. This change is purely about _where the constants get wired_, not about _what kinds of values the env can hold_. +- **A factory function like `createCalculatorEnvironment()`** to centralize the wiring further. Discussed during grilling, but rejected on YAGNI grounds — two composition roots calling the constructor twice is clearer than three modules calling a factory. +- **Renaming `Environment` or splitting it into `Storage` + `CalculatorEnvironment`.** Not needed — the DI refactor solves the dependency-inversion concern within the existing class. +- **Marking `BUILTIN_CONSTANTS` entries as protected against reassignment.** Same semantic as before — shadowing is allowed by design. + +## Impact + +- `src/v2-calculator/vm/Environment.ts`: constructor signature changes; `BUILTIN_CONSTANTS` import removed. +- `src/v2-calculator/vm/run.ts`: `env` parameter becomes required (no default arg). +- `src/v2-calculator/index.ts`: imports `BUILTIN_CONSTANTS`; default for `env` becomes `new Environment(BUILTIN_CONSTANTS)`. +- `src/v2-calculator/cli.ts`: imports `BUILTIN_CONSTANTS`; REPL env constructed with built-ins explicit. +- `src/v2-calculator/vm/Environment.test.ts`: deletes the `built-in constants` describe block; adds a single "constructor stores initial bindings" test; drops the `BUILTIN_CONSTANTS` import. +- `src/v2-calculator/calculator.test.ts`: minor adjustment — the existing `pi = 5\npi` shadowing test passes its own `env`; updated to either use the default `run()` (which wires built-ins via `index.ts`) or to pass `new Environment(BUILTIN_CONSTANTS)` explicitly. No other code-level changes; all 7 e2e tests pass against the new wiring. +- `src/v2-calculator/vm/builtins.ts`: **unchanged**. +- `src/v2-calculator/parser/*`, `src/v2-calculator/compiler/*`: **unchanged**. +- After approval + merge: sync delta into `openspec/specs/v2-calculator/spec.md` via the `openspec-sync-specs` skill — note this overwrites four MODIFIED requirements from the previous change's sync. diff --git a/openspec/changes/archive/2026-05-28-refactor-v2-calculator-environment-to-injected-builtins/specs/v2-calculator/spec.md b/openspec/changes/archive/2026-05-28-refactor-v2-calculator-environment-to-injected-builtins/specs/v2-calculator/spec.md new file mode 100644 index 0000000..704376d --- /dev/null +++ b/openspec/changes/archive/2026-05-28-refactor-v2-calculator-environment-to-injected-builtins/specs/v2-calculator/spec.md @@ -0,0 +1,273 @@ +## MODIFIED Requirements + +### Requirement: Variable storage is a single-scope Environment with explicit undefined-read errors + +The file `src/v2-calculator/vm/Environment.ts` SHALL export a class `Environment` whose state is a single `Map`. There SHALL NOT be an `enclosing` chain, a `define`-vs-`assign` distinction, or any `Value` wrapper around stored numbers. + +The class SHALL expose exactly two methods plus a constructor that accepts an optional `initialBindings` parameter: + +```ts +export class Environment { + constructor(initialBindings?: Readonly>); + get(name: string): number; // throws UndefinedVariable when name is unbound + set(name: string, value: number): void; // create-or-overwrite +} +``` + +The `initialBindings` parameter SHALL default to an empty object (`{}`). When supplied, the constructor SHALL iterate `Object.entries(initialBindings)` and write every `(name, value)` pair into the internal map via the same path as `set` — there SHALL NOT be a separate "define" code path that bypasses normal storage. When omitted (or explicitly `{}`), the constructed `Environment` SHALL be genuinely empty — `env.get(name)` SHALL throw `UndefinedVariable` for every `name`. + +The `Environment` SHALL NOT import `BUILTIN_CONSTANTS` (or any other calculator-policy module). The storage class is pure mechanism; what populates the env is the responsibility of whichever module constructs it. + +`get(name)` SHALL throw `UndefinedVariable` (a new error class exported from `src/v2-calculator/vm/errors.ts`) when no binding exists for `name`. The thrown error SHALL carry a public `name: string` property equal to the queried identifier and a `message` of the form `undefined variable: `. + +`set(name, value)` SHALL be idempotent on the binding's existence — it creates a new binding when absent and overwrites the existing one when present. There SHALL NOT be a separate "redefine" error; reassignment is the default. + +The `Environment` SHALL be the sole owner of variable state. The compiler SHALL NOT carry state across `compile()` calls; the REPL achieves persistence by sharing an `Environment` instance across `run()` calls, not by sharing compiler context. + +#### Scenario: A new Environment with no constructor argument starts empty + +- **WHEN** a new `Environment` is constructed with no arguments +- **AND** `env.get("x")` is called +- **THEN** `UndefinedVariable` is thrown +- **AND** the thrown error's `name` property is `"x"` + +#### Scenario: A new Environment with no constructor argument has no built-in bindings + +- **WHEN** a new `Environment` is constructed with no arguments +- **AND** `env.get("pi")` is called +- **THEN** `UndefinedVariable` is thrown +- **AND** the thrown error's `name` property is `"pi"` + +#### Scenario: The constructor copies initialBindings into storage + +- **WHEN** a new `Environment` is constructed with `{ foo: 1, bar: 2 }` +- **AND** `env.get("foo")` is called +- **THEN** the returned value is `1` +- **AND** `env.get("bar")` returns `2` + +#### Scenario: set followed by get returns the stored value + +- **WHEN** `env.set("x", 42)` is called +- **AND** `env.get("x")` is then called +- **THEN** the returned value is `42` + +#### Scenario: set overwrites an existing binding + +- **WHEN** `env.set("x", 1)` is called +- **AND** `env.set("x", 2)` is then called +- **AND** `env.get("x")` is then called +- **THEN** the returned value is `2` + +#### Scenario: set overwrites a binding seeded by the constructor + +- **WHEN** a new `Environment` is constructed with `{ foo: 1 }` +- **AND** `env.set("foo", 99)` is called +- **AND** `env.get("foo")` is then called +- **THEN** the returned value is `99` + +#### Scenario: UndefinedVariable carries the queried name + +- **WHEN** `env.get("not_bound")` is called on a fresh `Environment` constructed with no arguments +- **THEN** an `UndefinedVariable` is thrown +- **AND** the thrown error's `name` property equals `"not_bound"` +- **AND** the thrown error's `message` equals `"undefined variable: not_bound"` + +### Requirement: The VM accepts an optional Environment and resolves variables against it + +The file `src/v2-calculator/vm/run.ts` SHALL export `run(bytecode: Bytecode, env: Environment): number`. The `env` parameter SHALL be **required** — there SHALL NOT be a default argument. The VM is mechanism; it does not synthesize calculator defaults. The operand stack SHALL remain typed `number[]`. + +The opcode handlers SHALL behave as follows: + +- `LOAD_CONST { index }` — pushes `bytecode.constants[index]`. +- `LOAD_VAR { index }` — resolves `bytecode.names[index]` via `env.get(name)` and pushes the result. If no binding exists in `env`, `env.get` SHALL throw `UndefinedVariable`, which propagates out of `run`. +- `STORE_VAR { index }` — reads `stack[stack.length - 1]` (peek, no pop) and calls `env.set(bytecode.names[index], value)`. +- `POP` — discards the top of the operand stack. +- `ADD` / `SUB` / `MUL` / `DIV` — pop `right`, then `left`, push the binary result. `DIV` uses the native JavaScript `/` operator (no special-casing of `1/0` or `0/0`). +- `NEG` — pops one number, pushes its arithmetic negation. +- `HALT` — pops the top of the stack and returns it. + +The VM SHALL NOT bounds-check the operand stack, the constant pool, the names pool, or the instruction sequence. Stack-effect invariants are the compiler's responsibility. + +The `Environment` passed in SHALL be observable by the caller after `run` returns: any `STORE_VAR` executed during the call mutates the caller's `Environment`. This is the mechanism by which the REPL persists state across lines, and the mechanism by which `BUILTIN_CONSTANTS` reach the user — `index.ts` constructs the env with `BUILTIN_CONSTANTS` and threads it through. + +#### Scenario: A passed Environment is mutated by STORE_VAR + +- **WHEN** `run(compile(parse("x = 7")), new Environment())` is called +- **THEN** the return value is `7` +- **AND** the passed `env`'s `get("x")` returns `7` after the call + +#### Scenario: A second run sees variables written by the first + +- **WHEN** an `Environment` is constructed +- **AND** `run(compile(parse("x = 5")), env)` is called +- **AND** then `run(compile(parse("x + 1")), env)` is called with the same `env` +- **THEN** the second call returns `6` + +#### Scenario: An undefined variable throws UndefinedVariable + +- **WHEN** `run(compile(parse("y")), new Environment())` is called +- **THEN** an `UndefinedVariable` error is thrown +- **AND** the thrown error's `name` property is `"y"` + +#### Scenario: A read of a defined variable returns its value + +- **WHEN** `run(compile(parse("x = 3\nx + 4")), new Environment())` is called +- **THEN** the return value is `7` + +#### Scenario: Sum of two literals (unchanged) + +- **WHEN** `run(compile(parse("1 + 2")), new Environment())` is called +- **THEN** the result is `3` + +#### Scenario: Division-by-zero produces Infinity (unchanged) + +- **WHEN** `run(compile(parse("1 / 0")), new Environment())` is called +- **THEN** the result is `Infinity` + +#### Scenario: Zero-divided-by-zero produces NaN (unchanged) + +- **WHEN** `run(compile(parse("0 / 0")), new Environment())` is called +- **THEN** the result satisfies `Number.isNaN(result)` + +#### Scenario: Reassignment overwrites the prior binding + +- **WHEN** `run(compile(parse("x = 1\nx = 2\nx")), new Environment())` is called +- **THEN** the result is `2` + +#### Scenario: Assignment is an expression yielding the assigned value + +- **WHEN** `run(compile(parse("1 + (x = 2)")), new Environment())` is called +- **THEN** the result is `3` +- **AND** after the call, the passed `Environment` has `x = 2` + +### Requirement: The public entry point composes parse, compile, and run with an optional Environment + +The file `src/v2-calculator/index.ts` SHALL export a function with signature `run(source: string, env?: Environment): number` that composes `parse`, `compile`, and the VM's `run`: + +```ts +import { BUILTIN_CONSTANTS } from "./vm/builtins"; +import { Environment } from "./vm/Environment"; + +export function run( + source: string, + env: Environment = new Environment(BUILTIN_CONSTANTS), +): number { + return runBytecode(compile(parse(source)), env); +} +``` + +`index.ts` SHALL be a composition root: it SHALL import `BUILTIN_CONSTANTS` from `./vm/builtins` and SHALL default the `env` parameter to `new Environment(BUILTIN_CONSTANTS)` when the caller omits it. When `env` is supplied by the caller, the supplied `Environment` SHALL be used as-is — `index.ts` SHALL NOT merge `BUILTIN_CONSTANTS` into a caller-supplied env, augment it, or otherwise transform it. + +When `env` is omitted, each call SHALL operate on a freshly-constructed env pre-loaded with built-ins — so `run("pi")` returns `Math.PI` and `run("x = 1")` followed by `run("x")` (each without env) throws `UndefinedVariable` on the second call (the second call constructs its own fresh env). + +The function SHALL be the only public surface re-exported from `index.ts`. `parse`, `compile`, `tryParse`, `Bytecode`, `Instruction`, `Environment`, `UndefinedVariable`, `BUILTIN_CONSTANTS`, and other layer types SHALL remain accessible only through their respective layer modules (`./parser`, `./compiler/...`, `./vm/run`, `./vm/Environment`, `./vm/builtins`, `./vm/errors`). The function SHALL propagate parse-time, compile-time, and runtime errors as thrown `Error` objects with no wrapping, swallowing, or transformation. + +#### Scenario: Numeric result is returned as a JavaScript number + +- **WHEN** `run("1.5 + 0.5")` is called +- **THEN** the return value is `2` + +#### Scenario: The default env has built-in constants pre-bound + +- **WHEN** `run("pi")` is called with no env argument +- **THEN** the return value is exactly `Math.PI` + +#### Scenario: A shared Environment lets variables persist across calls + +- **WHEN** an `Environment` is created via `new Environment(BUILTIN_CONSTANTS)` +- **AND** `run("x = 10", env)` is called +- **AND** `run("x * 2", env)` is then called with the same `env` +- **THEN** the second call returns `20` + +#### Scenario: Omitting the Environment isolates calls + +- **WHEN** `run("x = 1")` is called (no env) +- **AND** `run("x")` is then called (no env) +- **THEN** the second call throws `UndefinedVariable` + +#### Scenario: A caller-supplied env without built-ins is honored as-is + +- **WHEN** `run("pi", new Environment())` is called (explicit empty env) +- **THEN** an `UndefinedVariable` is thrown for `pi` + +#### Scenario: Parse error propagates as a thrown Error + +- **WHEN** `run("1 +")` is called +- **THEN** an `Error` is thrown whose message comes from the Ohm parse-error + +### Requirement: Mathematical built-in constants are pre-bound in every Environment + +The file `src/v2-calculator/vm/builtins.ts` SHALL export a frozen `BUILTIN_CONSTANTS` constant of type `Readonly>` containing exactly three entries: + +```ts +export const BUILTIN_CONSTANTS: Readonly> = + Object.freeze({ + pi: Math.PI, + e: Math.E, + tau: 2 * Math.PI, + }); +``` + +The `BUILTIN_CONSTANTS` object SHALL be frozen via `Object.freeze`. The keys SHALL be the complete, ordered set of pre-bound names. + +`BUILTIN_CONSTANTS` SHALL be wired into the calculator's `Environment` by exactly two **composition roots** — and no other module: + +- `src/v2-calculator/index.ts` — the public `run` function, when `env` is omitted, SHALL default it to `new Environment(BUILTIN_CONSTANTS)`. +- `src/v2-calculator/cli.ts` — the REPL SHALL construct its persistent env as `new Environment(BUILTIN_CONSTANTS)`. + +These two modules SHALL be the SOLE importers of `BUILTIN_CONSTANTS` outside `vm/builtins.ts` itself. `Environment.ts` SHALL NOT import `BUILTIN_CONSTANTS`; `run.ts` (the VM) SHALL NOT import `BUILTIN_CONSTANTS`. The pre-binding is a property of the calculator's _composition_, not of the storage class. + +Once wired by a composition root, built-in constants reach the user as ordinary `Environment` bindings: they participate in `Environment.set` exactly like user variables. A user assignment such as `pi = 5` SHALL overwrite the binding for the lifetime of the wired `Environment` instance. A second `Environment` constructed by another composition-root call SHALL NOT be affected by reassignments in the first. + +The parser, compiler, AST, bytecode, and VM layers SHALL NOT be aware that any identifier is a built-in. Resolution SHALL happen exclusively at runtime through the existing `LOAD_VAR` opcode and `Environment.get` lookup path. A read of a built-in identifier SHALL be compiled as `LOAD_VAR { index }` against the `bytecode.names` pool, indistinguishable from a read of a user-defined name. + +#### Scenario: The public run wires pi to Math.PI by default + +- **WHEN** `run("pi")` is called with no env argument +- **THEN** the result is exactly `Math.PI` + +#### Scenario: The public run wires e to Math.E by default + +- **WHEN** `run("e")` is called with no env argument +- **THEN** the result is exactly `Math.E` + +#### Scenario: The public run wires tau to 2 \* Math.PI by default + +- **WHEN** `run("tau")` is called with no env argument +- **THEN** the result is exactly `2 * Math.PI` + +#### Scenario: Built-ins compose with arithmetic through the default env + +- **WHEN** `run("2 * pi")` is called with no env argument +- **THEN** the result is exactly `2 * Math.PI` + +#### Scenario: Tau equals two pi end-to-end through the default env + +- **WHEN** `run("tau / 2")` is called with no env argument +- **THEN** the result is exactly `Math.PI` + +#### Scenario: Shadowing roundtrips through the full pipeline + +- **WHEN** `run("pi = 5\npi")` is called with no env argument +- **THEN** the result is `5` + +#### Scenario: A directly-constructed empty Environment does not resolve built-ins + +- **WHEN** a fresh `new Environment()` is constructed with no arguments +- **AND** `env.get("pi")` is called +- **THEN** `UndefinedVariable` is thrown for `pi` + +#### Scenario: A caller can opt out of built-ins by passing an empty Environment to run + +- **WHEN** `run("pi", new Environment())` is called (explicit empty env) +- **THEN** `UndefinedVariable` is thrown for `pi` + +#### Scenario: Environment.ts does not import BUILTIN_CONSTANTS + +- **WHEN** the source of `src/v2-calculator/vm/Environment.ts` is read +- **THEN** the file SHALL NOT contain an `import` of `BUILTIN_CONSTANTS` from `./builtins` or any path + +#### Scenario: vm/run.ts does not import BUILTIN_CONSTANTS + +- **WHEN** the source of `src/v2-calculator/vm/run.ts` is read +- **THEN** the file SHALL NOT contain an `import` of `BUILTIN_CONSTANTS` from `./builtins` or any path diff --git a/openspec/changes/archive/2026-05-28-refactor-v2-calculator-environment-to-injected-builtins/tasks.md b/openspec/changes/archive/2026-05-28-refactor-v2-calculator-environment-to-injected-builtins/tasks.md new file mode 100644 index 0000000..2812ea7 --- /dev/null +++ b/openspec/changes/archive/2026-05-28-refactor-v2-calculator-environment-to-injected-builtins/tasks.md @@ -0,0 +1,52 @@ +## 1. Environment Refactor + +- [x] 1.1 In `src/v2-calculator/vm/Environment.ts`: remove the `import { BUILTIN_CONSTANTS } from "./builtins"` line. +- [x] 1.2 Change the constructor signature to `constructor(initialBindings: Readonly> = {})`. Body iterates `Object.entries(initialBindings)` and calls `this.records.set(name, value)` for each entry — same loop as before, but the source is the parameter, not the imported constant. +- [x] 1.3 Update the class-level docstring so it no longer claims that every fresh instance is pre-populated with `pi`/`e`/`tau`. Replace with language describing the optional `initialBindings` parameter and the empty-by-default behavior. + +## 2. VM Refactor + +- [x] 2.1 In `src/v2-calculator/vm/run.ts`: change the `run` signature from `run(bytecode: Bytecode, env: Environment = new Environment())` to `run(bytecode: Bytecode, env: Environment)`. Remove the default argument so `env` is required. +- [x] 2.2 Confirm no internal call inside `vm/run.ts` constructs an `Environment` (it didn't before either — the default was the only such construction). +- [x] 2.3 Confirm `vm/run.ts` does NOT import `BUILTIN_CONSTANTS` (it didn't before; this confirms the layering invariant per the spec). + +## 3. Public Entry Point Composition Root + +- [x] 3.1 In `src/v2-calculator/index.ts`: add `import { BUILTIN_CONSTANTS } from "./vm/builtins";` and `import { Environment } from "./vm/Environment";` (if not already imported). +- [x] 3.2 Change the exported `run` signature from `run(source: string, env?: Environment): number` to `run(source: string, env: Environment = new Environment(BUILTIN_CONSTANTS)): number`. Threads the defaulted `env` into `runBytecode`. +- [x] 3.3 Confirm the public behavior is unchanged: `run("pi")` (no env) returns `Math.PI`; `run("1 + 2")` returns `3`; `run("y")` throws `UndefinedVariable`. + +## 4. REPL Composition Root + +- [x] 4.1 In `src/v2-calculator/cli.ts`: add `import { BUILTIN_CONSTANTS } from "./vm/builtins";` (Environment is already imported). +- [x] 4.2 Change `const env = new Environment();` to `const env = new Environment(BUILTIN_CONSTANTS);` so the REPL pre-binds built-ins explicitly. +- [x] 4.3 Confirm the REPL still binds `_` via the existing `env.set("_", 0);` call immediately after construction. + +## 5. Trim Environment Unit Tests + +- [x] 5.1 In `src/v2-calculator/vm/Environment.test.ts`: remove the `import { BUILTIN_CONSTANTS } from "./builtins";` line (no longer needed). +- [x] 5.2 Delete the entire `describe("built-in constants", ...)` block (8 tests). The behavior they tested (pre-population in every Environment) no longer holds at the `Environment` class level. +- [x] 5.3 Add a single replacement test: `it("copies initialBindings into storage on construction", ...)` that constructs `new Environment({ foo: 1, bar: 2 })` and asserts both bindings resolve via `get`. +- [x] 5.4 Add a test: `it("seeded bindings can still be overwritten by set", ...)` that constructs `new Environment({ foo: 1 })`, calls `env.set("foo", 99)`, and confirms `env.get("foo") === 99`. Covers the "seeded values are ordinary bindings" invariant. +- [x] 5.5 Add a test: `it("starts empty when constructed with no arguments", ...)` that confirms `new Environment().get("pi")` throws `UndefinedVariable`. Confirms `Environment` no longer pre-populates from `BUILTIN_CONSTANTS`. + +## 6. Adjust Calculator Integration Tests + +- [x] 6.1 In `src/v2-calculator/calculator.test.ts`: the existing test `it("lets user assignment shadow a built-in through the full pipeline", ...)` currently constructs `const env = new Environment()` (which is now empty) and calls `run("pi = 5\npi", env)`. Rewrite it to drop the explicit env — `expect(run("pi = 5\npi")).toBe(5)` — so the default `index.ts`-supplied built-in-loaded env is used, and the test actually tests _shadowing_ (you can only shadow something that's there). +- [x] 6.2 Add an opt-out test: `it("can opt out of built-ins by passing a fresh empty Environment", ...)` that calls `run("pi", new Environment())` and expects `UndefinedVariable`. This is a _new_ affordance enabled by DI and worth covering. +- [x] 6.3 Confirm the other 6 e2e tests in the `describe("built-in constants", ...)` block (`run("pi")`, `run("e")`, `run("tau")`, `run("2 * pi")`, `run("tau / 2")`, `run("phi")` throws) all still pass without modification — they exercise the default-env path, which now goes through `index.ts`'s wiring. +- [x] 6.4 Confirm the existing `describe("variables", ...)` tests that construct their own `Environment` (lines around 96, 112, 149 pre-refactor) still pass — they don't use built-in names like `pi`, so the empty-env-by-default semantic is fine. + +## 7. Quality Gate + +- [x] 7.1 Run `yarn lint` and fix any violations. +- [x] 7.2 Run `yarn typecheck` and fix any errors. +- [x] 7.3 Run `yarn test` and confirm the entire suite is green (not just the v2-calculator slice). The 7 e2e built-in tests must remain green; the Environment unit tests should reflect the new shape (3 new tests added, 8 removed). + +## 8. Documentation + +- [x] 8.1 No `src/v2-calculator/todo.md` change needed — built-in constants were already ticked by the previous change, and the IEEE-754 follow-up remains in place. This change is internal restructuring with no user-visible feature delta. + +## 9. Post-Merge + +- [x] 9.1 After approval and merge, sync the delta spec into `openspec/specs/v2-calculator/spec.md` using the `openspec-sync-specs` skill. Note this sync overwrites four MODIFIED requirement blocks from the previous change's sync (`Variable storage…`, `The VM accepts an optional Environment…`, `The public entry point…`, `Mathematical built-in constants…`). diff --git a/openspec/specs/v2-calculator/spec.md b/openspec/specs/v2-calculator/spec.md new file mode 100644 index 0000000..b60dba5 --- /dev/null +++ b/openspec/specs/v2-calculator/spec.md @@ -0,0 +1,685 @@ +# v2-calculator Specification + +## Purpose + +TBD - created by syncing change add-v2-calculator. Update after archive. + +## Requirements + +### Requirement: Calculator grammar accepts a newline-separated sequence of expressions + +The file `src/v2-calculator/parser/grammar.ohm` SHALL define an Ohm grammar named `Calculator` whose top rule is `Program = NonemptyListOf`. Every legal program SHALL be one or more expressions separated by one or more `"\n"` characters; the final newline before EOF is optional. + +The grammar SHALL override Ohm's default whitespace handling with `space := " " | "\t"` so that newlines are syntactically significant. A `newline = "\n"+` rule SHALL collapse runs of blank lines into a single separator. Empty source (zero expressions) SHALL be a parse error. + +The expression hierarchy SHALL be: + +``` +Expr = AssignExp +AssignExp = ident "=" AssignExp -- assign + | AddExp +AddExp = AddExp "+" MulExp -- plus + | AddExp "-" MulExp -- minus + | MulExp +MulExp = MulExp "*" UnaryExp -- times + | MulExp "/" UnaryExp -- divide + | UnaryExp +UnaryExp = "+" UnaryExp -- pos + | "-" UnaryExp -- neg + | PriExp +PriExp = "(" Expr ")" -- paren + | ident -- var + | number +``` + +The `AssignExp_assign` rule's LHS SHALL be `ident` only — `(x) = 1`, `1 = 2`, and any other non-identifier LHS SHALL be rejected by the grammar without reaching the compiler. The `AssignExp_assign` rule SHALL be right-recursive on its own RHS (chained `x = y = 1` parses as `x = (y = 1)`). + +The `ident` rule SHALL be `(letter | "_") (letter | digit | "_")*`. The `number` rule SHALL remain `digit+ ("." digit+)?`. The leading sign on a literal SHALL be parsed as unary prefix `Unary`, not as part of `number`. + +The grammar SHALL NOT include keywords (no `let`, no `var`, no reserved words), string literals, boolean literals, `none`, array literals, range expressions, comparison or logical operators, postfix call/index/property, function definitions, or any block construct. + +#### Scenario: A single expression on a single line parses + +- **WHEN** the source `1 + 2` is parsed +- **THEN** parsing succeeds +- **AND** the AST is a one-element `Program` whose sole element is `Arithmetic("+", Literal(1), Literal(2))` + +#### Scenario: Two expressions separated by a newline parse + +- **WHEN** the source is `x = 1 + 2\n1 + x * 2` +- **THEN** parsing succeeds +- **AND** the resulting `Program` has length `2` +- **AND** the first element is `Assign("x", Arithmetic("+", Literal(1), Literal(2)))` +- **AND** the second element is `Arithmetic("+", Literal(1), Arithmetic("*", Var("x"), Literal(2)))` + +#### Scenario: Blank lines between expressions are legal + +- **WHEN** the source is `x = 1\n\n\ny = 2` +- **THEN** parsing succeeds +- **AND** the resulting `Program` has length `2` + +#### Scenario: Empty source is a parse error + +- **WHEN** the source is the empty string `""` +- **THEN** parsing fails + +#### Scenario: A bare identifier is a legal expression + +- **WHEN** the source `x` is parsed +- **THEN** parsing succeeds +- **AND** the AST is a one-element `Program` whose sole element is `Var("x")` + +#### Scenario: Identifiers may begin with underscore + +- **WHEN** the source `_foo = 1\n_` is parsed +- **THEN** parsing succeeds +- **AND** the first element is `Assign("_foo", Literal(1))` +- **AND** the second element is `Var("_")` + +#### Scenario: Identifiers may contain underscores and digits after the first character + +- **WHEN** the source `foo_bar = 1\nx1 = 2\nfoo123 = 3` is parsed +- **THEN** parsing succeeds +- **AND** each `Assign` node has the expected name + +#### Scenario: Identifiers may not begin with a digit + +- **WHEN** the source `1foo = 1` is parsed +- **THEN** parsing fails + +#### Scenario: Chained assignment parses right-associatively + +- **WHEN** the source `x = y = 123` is parsed +- **THEN** the AST is `Assign("x", Assign("y", Literal(123)))` + +#### Scenario: Assignment binds looser than addition + +- **WHEN** the source `1 + x = 2` is parsed +- **THEN** parsing fails + +#### Scenario: Parenthesised assignment is a valid sub-expression + +- **WHEN** the source `(x = 1) + 2` is parsed +- **THEN** parsing succeeds +- **AND** the AST is `Arithmetic("+", Assign("x", Literal(1)), Literal(2))` + +#### Scenario: Newlines are no longer ordinary whitespace + +- **WHEN** the source is `1 +\n2` (newline between `+` and `2`) +- **THEN** parsing fails + +#### Scenario: Subtraction remains left-associative + +- **WHEN** the source `1 - 2 - 3` is parsed +- **THEN** the resulting AST element is `Arithmetic("-", Arithmetic("-", Literal(1), Literal(2)), Literal(3))` + +#### Scenario: Multiplication binds tighter than addition + +- **WHEN** the source `1 + 2 * 3` is parsed +- **THEN** the resulting AST element is `Arithmetic("+", Literal(1), Arithmetic("*", Literal(2), Literal(3)))` + +#### Scenario: Unary minus stacks + +- **WHEN** the source `--5` is parsed +- **THEN** the resulting AST element is `Unary("-", Unary("-", Literal(5)))` + +#### Scenario: A trailing operator is a parse error + +- **WHEN** the source `1 +` is parsed +- **THEN** parsing fails + +### Requirement: The AST adds Var and Assign nodes; Program becomes a sequence + +The file `src/v2-calculator/parser/ast.ts` SHALL export the discriminated union: + +```ts +export type Expr = + | { kind: "Literal"; value: number } + | { kind: "Arithmetic"; op: "+" | "-" | "*" | "/"; left: Expr; right: Expr } + | { kind: "Unary"; op: "+" | "-"; expr: Expr } + | { kind: "Var"; name: string } + | { kind: "Assign"; name: string; expr: Expr }; + +export type Program = readonly Expr[]; +``` + +The five discriminant names (`Literal`, `Arithmetic`, `Unary`, `Var`, `Assign`) SHALL be the complete set of `Expr` node kinds. AST nodes SHALL NOT carry source spans, parent pointers, or any field beyond those listed. + +The `Assign` node's `name` field SHALL be the raw identifier string captured by the grammar. The compiler — not the AST — is responsible for resolving names to slots. + +#### Scenario: The five AST kinds are distinct + +- **WHEN** the sources `5`, `1+2`, `-5`, `x`, and `x = 1` are each parsed as one-element programs +- **THEN** the sole element's `kind` is exactly `"Literal"`, `"Arithmetic"`, `"Unary"`, `"Var"`, and `"Assign"` respectively + +#### Scenario: AST nodes have no span field + +- **WHEN** any source is parsed +- **THEN** no AST node in the result contains a key named `span` + +#### Scenario: Program is an array + +- **WHEN** the source `1\n2\n3` is parsed +- **THEN** `program.length` is `3` +- **AND** each element is a `Literal` + +### Requirement: Parser surface mirrors v2's API exactly + +The file `src/v2-calculator/parser/parser.ts` SHALL export `parse(code: string): Program` and `tryParse(code: string): TryParseResult` with the same `TryParseResult` discriminated union as v2: + +```ts +export type TryParseResult = + | { ok: true; program: Program } + | { ok: false; incomplete: true; message: string } + | { ok: false; incomplete: false; message: string }; +``` + +`tryParse` SHALL return `{ ok: false, incomplete: true, … }` when the Ohm match fails at end-of-input (i.e., `matchResult.getRightmostFailurePosition() === code.length`) and `{ ok: false, incomplete: false, … }` otherwise. `parse` SHALL throw `new Error(message)` whenever `tryParse` returns `ok: false`, regardless of the `incomplete` flag. + +The semantics layer (`src/v2-calculator/parser/semantics.ts`) SHALL register a single `toAst` operation that builds the `Program` array and the per-node `Expr` AST defined above. The `Program` action SHALL iterate the `NonemptyListOf` children and collect their `toAst()` results. + +#### Scenario: tryParse flags trailing-operator failure as incomplete + +- **WHEN** `tryParse("1 +")` is called +- **THEN** the result is `{ ok: false, incomplete: true, message: }` + +#### Scenario: tryParse flags mid-expression failure as not incomplete + +- **WHEN** `tryParse("1 1")` is called +- **THEN** the result is `{ ok: false, incomplete: false, message: }` + +#### Scenario: tryParse flags unclosed-paren as incomplete + +- **WHEN** `tryParse("(1 + 2")` is called +- **THEN** the result is `{ ok: false, incomplete: true, message: }` + +#### Scenario: tryParse flags a dangling `=` as incomplete + +- **WHEN** `tryParse("x =")` is called +- **THEN** the result is `{ ok: false, incomplete: true, message: }` + +#### Scenario: parse throws on any failure + +- **WHEN** `parse("1 +")` is called +- **THEN** an `Error` is thrown whose message comes from the Ohm matcher + +#### Scenario: parse returns the AST on success + +- **WHEN** `parse("1 + 2")` is called +- **THEN** the return value is a one-element array `[Arithmetic("+", Literal(1), Literal(2))]` + +### Requirement: Bytecode is a flat instruction list plus a numeric constant pool and a names pool + +The file `src/v2-calculator/compiler/bytecode.ts` SHALL export: + +```ts +export type Instruction = + | { opcode: "LOAD_CONST"; index: number } + | { opcode: "LOAD_VAR"; index: number } + | { opcode: "STORE_VAR"; index: number } + | { opcode: "POP" } + | { opcode: "ADD" } + | { opcode: "SUB" } + | { opcode: "MUL" } + | { opcode: "DIV" } + | { opcode: "NEG" } + | { opcode: "HALT" }; + +export interface Bytecode { + readonly code: readonly Instruction[]; + readonly constants: readonly number[]; + readonly names: readonly string[]; +} +``` + +`STORE_VAR { index }` SHALL document (via TS comment) that it reads the top of the operand stack **without popping** and writes the value into the environment under `names[index]`. The value remains on the stack so that assignment behaves as an expression. `LOAD_VAR { index }` SHALL document that it reads `names[index]` from the environment and pushes the value, throwing `UndefinedVariable` when no binding exists. `POP` SHALL pop and discard the top of the operand stack. + +The compiler SHALL maintain `names` as the unique set of identifiers referenced by the program, in first-seen order. Each unique identifier SHALL occupy exactly one slot regardless of how many times it is read or written. The `constants` pool SHALL continue to permit duplicate numeric entries (no deduplication). + +The compiler SHALL emit each top-level `Expr` of the `Program` in source order. After each emitted expression except the last, the compiler SHALL emit a single `POP` instruction. After the last expression, the compiler SHALL emit `HALT`. + +The grammar guarantees that every legal `Program` has at least one expression, so the emitted code SHALL always end with `… HALT` preceded by at least one expression-producing sequence. + +#### Scenario: A single literal compiles to LOAD_CONST + HALT + +- **WHEN** the source `42` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, HALT]` +- **AND** `bytecode.constants` is `[42]` +- **AND** `bytecode.names` is `[]` + +#### Scenario: A bare assignment compiles to LOAD_CONST + STORE_VAR + HALT + +- **WHEN** the source `x = 5` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, STORE_VAR 0, HALT]` +- **AND** `bytecode.constants` is `[5]` +- **AND** `bytecode.names` is `["x"]` + +#### Scenario: Chained assignment emits multiple STORE_VAR over a single value + +- **WHEN** the source `x = y = 5` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, STORE_VAR 1, STORE_VAR 0, HALT]` +- **AND** `bytecode.names` is `["x", "y"]` + +#### Scenario: A two-expression program emits POP between expressions + +- **WHEN** the source `x = 1\n2` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, STORE_VAR 0, POP, LOAD_CONST 1, HALT]` +- **AND** `bytecode.constants` is `[1, 2]` +- **AND** `bytecode.names` is `["x"]` + +#### Scenario: Variable reads reuse the same names slot as their writes + +- **WHEN** the source `x = 1\nx + x` is compiled +- **THEN** `bytecode.names` is `["x"]` (length `1`) +- **AND** both reads emit `LOAD_VAR 0` + +#### Scenario: Reading an identifier before any write still allocates a slot + +- **WHEN** the source `y` is compiled +- **THEN** `bytecode.code` is `[LOAD_VAR 0, HALT]` +- **AND** `bytecode.names` is `["y"]` +- **AND** compilation does NOT throw + +#### Scenario: Multi-statement final-expression result is the program's value + +- **WHEN** the source `x = 1 + 2\n1 + x * 2` is compiled +- **THEN** the final non-`HALT` instruction sequence ends with `… ADD, HALT` +- **AND** there is exactly one `POP` instruction in the emitted code (between the two top-level expressions) + +#### Scenario: Operator precedence reflects in bytecode order (unchanged) + +- **WHEN** the source `1 + 2 * 3` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, LOAD_CONST 1, LOAD_CONST 2, MUL, ADD, HALT]` + +#### Scenario: Unary minus emits NEG (unchanged) + +- **WHEN** the source `-5` is compiled +- **THEN** `bytecode.code` is `[LOAD_CONST 0, NEG, HALT]` + +### Requirement: The VM accepts an optional Environment and resolves variables against it + +The file `src/v2-calculator/vm/run.ts` SHALL export `run(bytecode: Bytecode, env: Environment): number`. The `env` parameter SHALL be **required** — there SHALL NOT be a default argument. The VM is mechanism; it does not synthesize calculator defaults. The operand stack SHALL remain typed `number[]`. + +The opcode handlers SHALL behave as follows: + +- `LOAD_CONST { index }` — pushes `bytecode.constants[index]`. +- `LOAD_VAR { index }` — resolves `bytecode.names[index]` via `env.get(name)` and pushes the result. If no binding exists in `env`, `env.get` SHALL throw `UndefinedVariable`, which propagates out of `run`. +- `STORE_VAR { index }` — reads `stack[stack.length - 1]` (peek, no pop) and calls `env.set(bytecode.names[index], value)`. +- `POP` — discards the top of the operand stack. +- `ADD` / `SUB` / `MUL` / `DIV` — pop `right`, then `left`, push the binary result. `DIV` uses the native JavaScript `/` operator (no special-casing of `1/0` or `0/0`). +- `NEG` — pops one number, pushes its arithmetic negation. +- `HALT` — pops the top of the stack and returns it. + +The VM SHALL NOT bounds-check the operand stack, the constant pool, the names pool, or the instruction sequence. Stack-effect invariants are the compiler's responsibility. + +The `Environment` passed in SHALL be observable by the caller after `run` returns: any `STORE_VAR` executed during the call mutates the caller's `Environment`. This is the mechanism by which the REPL persists state across lines, and the mechanism by which `BUILTIN_CONSTANTS` reach the user — `index.ts` constructs the env with `BUILTIN_CONSTANTS` and threads it through. + +#### Scenario: A passed Environment is mutated by STORE_VAR + +- **WHEN** `run(compile(parse("x = 7")), new Environment())` is called +- **THEN** the return value is `7` +- **AND** the passed `env`'s `get("x")` returns `7` after the call + +#### Scenario: A second run sees variables written by the first + +- **WHEN** an `Environment` is constructed +- **AND** `run(compile(parse("x = 5")), env)` is called +- **AND** then `run(compile(parse("x + 1")), env)` is called with the same `env` +- **THEN** the second call returns `6` + +#### Scenario: An undefined variable throws UndefinedVariable + +- **WHEN** `run(compile(parse("y")), new Environment())` is called +- **THEN** an `UndefinedVariable` error is thrown +- **AND** the thrown error's `name` property is `"y"` + +#### Scenario: A read of a defined variable returns its value + +- **WHEN** `run(compile(parse("x = 3\nx + 4")), new Environment())` is called +- **THEN** the return value is `7` + +#### Scenario: Sum of two literals (unchanged) + +- **WHEN** `run(compile(parse("1 + 2")), new Environment())` is called +- **THEN** the result is `3` + +#### Scenario: Division-by-zero produces Infinity (unchanged) + +- **WHEN** `run(compile(parse("1 / 0")), new Environment())` is called +- **THEN** the result is `Infinity` + +#### Scenario: Zero-divided-by-zero produces NaN (unchanged) + +- **WHEN** `run(compile(parse("0 / 0")), new Environment())` is called +- **THEN** the result satisfies `Number.isNaN(result)` + +#### Scenario: Reassignment overwrites the prior binding + +- **WHEN** `run(compile(parse("x = 1\nx = 2\nx")), new Environment())` is called +- **THEN** the result is `2` + +#### Scenario: Assignment is an expression yielding the assigned value + +- **WHEN** `run(compile(parse("1 + (x = 2)")), new Environment())` is called +- **THEN** the result is `3` +- **AND** after the call, the passed `Environment` has `x = 2` + +### Requirement: The public entry point composes parse, compile, and run with an optional Environment + +The file `src/v2-calculator/index.ts` SHALL export a function with signature `run(source: string, env?: Environment): number` that composes `parse`, `compile`, and the VM's `run`: + +```ts +import { BUILTIN_CONSTANTS } from "./vm/builtins"; +import { Environment } from "./vm/Environment"; + +export function run( + source: string, + env: Environment = new Environment(BUILTIN_CONSTANTS), +): number { + return runBytecode(compile(parse(source)), env); +} +``` + +`index.ts` SHALL be a composition root: it SHALL import `BUILTIN_CONSTANTS` from `./vm/builtins` and SHALL default the `env` parameter to `new Environment(BUILTIN_CONSTANTS)` when the caller omits it. When `env` is supplied by the caller, the supplied `Environment` SHALL be used as-is — `index.ts` SHALL NOT merge `BUILTIN_CONSTANTS` into a caller-supplied env, augment it, or otherwise transform it. + +When `env` is omitted, each call SHALL operate on a freshly-constructed env pre-loaded with built-ins — so `run("pi")` returns `Math.PI` and `run("x = 1")` followed by `run("x")` (each without env) throws `UndefinedVariable` on the second call (the second call constructs its own fresh env). + +The function SHALL be the only public surface re-exported from `index.ts`. `parse`, `compile`, `tryParse`, `Bytecode`, `Instruction`, `Environment`, `UndefinedVariable`, `BUILTIN_CONSTANTS`, and other layer types SHALL remain accessible only through their respective layer modules (`./parser`, `./compiler/...`, `./vm/run`, `./vm/Environment`, `./vm/builtins`, `./vm/errors`). The function SHALL propagate parse-time, compile-time, and runtime errors as thrown `Error` objects with no wrapping, swallowing, or transformation. + +#### Scenario: Numeric result is returned as a JavaScript number + +- **WHEN** `run("1.5 + 0.5")` is called +- **THEN** the return value is `2` + +#### Scenario: The default env has built-in constants pre-bound + +- **WHEN** `run("pi")` is called with no env argument +- **THEN** the return value is exactly `Math.PI` + +#### Scenario: A shared Environment lets variables persist across calls + +- **WHEN** an `Environment` is created via `new Environment(BUILTIN_CONSTANTS)` +- **AND** `run("x = 10", env)` is called +- **AND** `run("x * 2", env)` is then called with the same `env` +- **THEN** the second call returns `20` + +#### Scenario: Omitting the Environment isolates calls + +- **WHEN** `run("x = 1")` is called (no env) +- **AND** `run("x")` is then called (no env) +- **THEN** the second call throws `UndefinedVariable` + +#### Scenario: A caller-supplied env without built-ins is honored as-is + +- **WHEN** `run("pi", new Environment())` is called (explicit empty env) +- **THEN** an `UndefinedVariable` is thrown for `pi` + +#### Scenario: Parse error propagates as a thrown Error + +- **WHEN** `run("1 +")` is called +- **THEN** an `Error` is thrown whose message comes from the Ohm parse-error + +### Requirement: `yarn lucky-calculator` launches a REPL that persists variables across lines + +The file `package.json` SHALL retain the script `"lucky-calculator": "ts-node src/v2-calculator/cli.ts"`. Running `yarn lucky-calculator` SHALL start an interactive REPL with the following contract: + +- The prompt SHALL be `> ` for every input. There SHALL NOT be a continuation prompt; the prompt SHALL NOT change shape across lines. +- The REPL SHALL parse each line standalone; multi-line continuation SHALL NOT be implemented. The `tryParse.incomplete` flag SHALL be ignored at the REPL level — a parse failure for any reason is reported and the prompt re-displayed. +- The bare typed words `exit` or `quit` (after `trim()`) SHALL exit the REPL with code `0`. +- Ctrl-D (the readline `close` event) SHALL exit the REPL with code `0`. +- A successful evaluation SHALL print the result with `console.log(result)`. No formatting SHALL be applied; the JavaScript `String(number)` representation passes through unchanged. +- A parse error SHALL be printed to `console.error` (the parse-error message string) and the prompt SHALL be re-displayed. +- A runtime-thrown `Error` (including `UndefinedVariable`) SHALL be printed to `console.error` (the error's `message`) and the prompt SHALL be re-displayed. +- An empty or whitespace-only line SHALL be treated as a no-op — the prompt is re-displayed without evaluation. + +The REPL SHALL hold a single `Environment` instance for the entire session and pass it to every `run` call. Variables written by a successful evaluation SHALL be visible to subsequent lines. A line that throws (parse or runtime) SHALL NOT roll back partial writes — assignments that completed before the throw remain in the `Environment`; this falls out of the VM operating directly on the live `Environment`. + +The REPL SHALL bind `_` to the most recently produced result per the _REPL binds `_` to the last successful result\_ requirement below. + +#### Scenario: A simple expression evaluates and prints + +- **WHEN** the user types `1 + 2` at the prompt +- **THEN** the REPL prints `3` to stdout +- **AND** re-displays the `> ` prompt + +#### Scenario: A variable defined on one line is visible on the next + +- **WHEN** the user types `x = 5` at the prompt +- **AND** then types `x + 10` at the next prompt +- **THEN** the REPL prints `5` to stdout for the first line +- **AND** prints `15` to stdout for the second line + +#### Scenario: An undefined variable reports an error and the REPL continues + +- **WHEN** the user types `y` at the prompt with no prior assignment to `y` +- **THEN** the REPL prints an `UndefinedVariable` message to stderr +- **AND** re-displays the `> ` prompt +- **AND** the process does NOT exit + +#### Scenario: A parse error is reported and the REPL continues + +- **WHEN** the user types `1 +` at the prompt +- **THEN** the REPL prints the Ohm parse-error message to stderr +- **AND** re-displays the `> ` prompt +- **AND** the process does NOT exit + +#### Scenario: Reassignment is visible to subsequent lines + +- **WHEN** the user types `x = 1`, then `x = 2`, then `x` +- **THEN** the third line prints `2` + +#### Scenario: `exit` closes the REPL + +- **WHEN** the user types `exit` at the prompt +- **THEN** the REPL exits with code `0` + +#### Scenario: `quit` closes the REPL + +- **WHEN** the user types `quit` at the prompt +- **THEN** the REPL exits with code `0` + +#### Scenario: An empty line re-prompts without output + +- **WHEN** the user presses Enter on an empty prompt +- **THEN** the REPL re-displays `> ` +- **AND** nothing is printed to stdout or stderr + +### Requirement: REPL binds `_` to the last successful result + +When `startRepl()` constructs its `Environment`, the REPL SHALL seed it with `_ = 0` before reading the first line. After every line that `run` returns from without throwing, the REPL SHALL set `_` to the returned number prior to printing the result and re-displaying the prompt. After any line that throws (parse error, `UndefinedVariable`, or any other runtime error), `_` SHALL be unchanged. + +`_` SHALL remain a plain variable in every other respect: the grammar already permits `_` as an identifier, so users MAY read it (`_ + 1`), assign through it (`_ = 99`, `x = _`), or self-update it (`_ = _ + 1`). The REPL's post-eval write applies uniformly regardless of whether the user's expression mentioned `_`. + +The seeding and post-eval update SHALL live entirely inside `src/v2-calculator/cli.ts`. The `Environment` class, the compiler, the VM, and `index.ts`'s `run(source, env?)` SHALL remain unaware of `_` — callers of `run()` that do not pre-seed an `Environment` with `_` SHALL still see `UndefinedVariable: _` if they evaluate `_`. + +#### Scenario: `_` reads `0` before any expression has been evaluated + +- **WHEN** a REPL session has just started and no line has been entered +- **AND** the user types `_` at the prompt +- **THEN** the REPL prints `0` to stdout + +#### Scenario: `_` is updated to the result of the previous successful line + +- **WHEN** the user types `2 + 3` at the prompt +- **AND** then types `_` at the next prompt +- **THEN** the second line prints `5` to stdout + +#### Scenario: `_` is usable inside the next expression + +- **WHEN** the user types `10` at the prompt +- **AND** then types `_ * 4` at the next prompt +- **THEN** the second line prints `40` to stdout + +#### Scenario: `_` is unchanged after a parse error + +- **WHEN** the user types `5` at the prompt (so `_` becomes `5`) +- **AND** then types `1 +` at the next prompt (parse error) +- **AND** then types `_` at the next prompt +- **THEN** the third line prints `5` to stdout + +#### Scenario: `_` is unchanged after a runtime error + +- **WHEN** the user types `7` at the prompt (so `_` becomes `7`) +- **AND** then types `y` at the next prompt (with no prior assignment to `y`) +- **AND** then types `_` at the next prompt +- **THEN** the third line prints `7` to stdout + +#### Scenario: An assignment line updates `_` to the assigned value + +- **WHEN** the user types `x = 5` at the prompt +- **AND** then types `_` at the next prompt +- **THEN** the second line prints `5` to stdout + +#### Scenario: A self-referential update advances `_` step by step + +- **WHEN** the user types `_ = _ + 1` at the prompt three times in succession (starting from the default `_ = 0`) +- **THEN** the printed results are `1`, `2`, and `3` in that order + +### Requirement: Variable storage is a single-scope Environment with explicit undefined-read errors + +The file `src/v2-calculator/vm/Environment.ts` SHALL export a class `Environment` whose state is a single `Map`. There SHALL NOT be an `enclosing` chain, a `define`-vs-`assign` distinction, or any `Value` wrapper around stored numbers. + +The class SHALL expose exactly two methods plus a constructor that accepts an optional `initialBindings` parameter: + +```ts +export class Environment { + constructor(initialBindings?: Readonly>); + get(name: string): number; // throws UndefinedVariable when name is unbound + set(name: string, value: number): void; // create-or-overwrite +} +``` + +The `initialBindings` parameter SHALL default to an empty object (`{}`). When supplied, the constructor SHALL iterate `Object.entries(initialBindings)` and write every `(name, value)` pair into the internal map via the same path as `set` — there SHALL NOT be a separate "define" code path that bypasses normal storage. When omitted (or explicitly `{}`), the constructed `Environment` SHALL be genuinely empty — `env.get(name)` SHALL throw `UndefinedVariable` for every `name`. + +The `Environment` SHALL NOT import `BUILTIN_CONSTANTS` (or any other calculator-policy module). The storage class is pure mechanism; what populates the env is the responsibility of whichever module constructs it. + +`get(name)` SHALL throw `UndefinedVariable` (a new error class exported from `src/v2-calculator/vm/errors.ts`) when no binding exists for `name`. The thrown error SHALL carry a public `name: string` property equal to the queried identifier and a `message` of the form `undefined variable: `. + +`set(name, value)` SHALL be idempotent on the binding's existence — it creates a new binding when absent and overwrites the existing one when present. There SHALL NOT be a separate "redefine" error; reassignment is the default. + +The `Environment` SHALL be the sole owner of variable state. The compiler SHALL NOT carry state across `compile()` calls; the REPL achieves persistence by sharing an `Environment` instance across `run()` calls, not by sharing compiler context. + +#### Scenario: A new Environment with no constructor argument starts empty + +- **WHEN** a new `Environment` is constructed with no arguments +- **AND** `env.get("x")` is called +- **THEN** `UndefinedVariable` is thrown +- **AND** the thrown error's `name` property is `"x"` + +#### Scenario: A new Environment with no constructor argument has no built-in bindings + +- **WHEN** a new `Environment` is constructed with no arguments +- **AND** `env.get("pi")` is called +- **THEN** `UndefinedVariable` is thrown +- **AND** the thrown error's `name` property is `"pi"` + +#### Scenario: The constructor copies initialBindings into storage + +- **WHEN** a new `Environment` is constructed with `{ foo: 1, bar: 2 }` +- **AND** `env.get("foo")` is called +- **THEN** the returned value is `1` +- **AND** `env.get("bar")` returns `2` + +#### Scenario: set followed by get returns the stored value + +- **WHEN** `env.set("x", 42)` is called +- **AND** `env.get("x")` is then called +- **THEN** the returned value is `42` + +#### Scenario: set overwrites an existing binding + +- **WHEN** `env.set("x", 1)` is called +- **AND** `env.set("x", 2)` is then called +- **AND** `env.get("x")` is then called +- **THEN** the returned value is `2` + +#### Scenario: set overwrites a binding seeded by the constructor + +- **WHEN** a new `Environment` is constructed with `{ foo: 1 }` +- **AND** `env.set("foo", 99)` is called +- **AND** `env.get("foo")` is then called +- **THEN** the returned value is `99` + +#### Scenario: UndefinedVariable carries the queried name + +- **WHEN** `env.get("not_bound")` is called on a fresh `Environment` constructed with no arguments +- **THEN** an `UndefinedVariable` is thrown +- **AND** the thrown error's `name` property equals `"not_bound"` +- **AND** the thrown error's `message` equals `"undefined variable: not_bound"` + +### Requirement: Mathematical built-in constants are pre-bound in every Environment + +The file `src/v2-calculator/vm/builtins.ts` SHALL export a frozen `BUILTIN_CONSTANTS` constant of type `Readonly>` containing exactly three entries: + +```ts +export const BUILTIN_CONSTANTS: Readonly> = + Object.freeze({ + pi: Math.PI, + e: Math.E, + tau: 2 * Math.PI, + }); +``` + +The `BUILTIN_CONSTANTS` object SHALL be frozen via `Object.freeze`. The keys SHALL be the complete, ordered set of pre-bound names. + +`BUILTIN_CONSTANTS` SHALL be wired into the calculator's `Environment` by exactly two **composition roots** — and no other module: + +- `src/v2-calculator/index.ts` — the public `run` function, when `env` is omitted, SHALL default it to `new Environment(BUILTIN_CONSTANTS)`. +- `src/v2-calculator/cli.ts` — the REPL SHALL construct its persistent env as `new Environment(BUILTIN_CONSTANTS)`. + +These two modules SHALL be the SOLE importers of `BUILTIN_CONSTANTS` outside `vm/builtins.ts` itself. `Environment.ts` SHALL NOT import `BUILTIN_CONSTANTS`; `run.ts` (the VM) SHALL NOT import `BUILTIN_CONSTANTS`. The pre-binding is a property of the calculator's _composition_, not of the storage class. + +Once wired by a composition root, built-in constants reach the user as ordinary `Environment` bindings: they participate in `Environment.set` exactly like user variables. A user assignment such as `pi = 5` SHALL overwrite the binding for the lifetime of the wired `Environment` instance. A second `Environment` constructed by another composition-root call SHALL NOT be affected by reassignments in the first. + +The parser, compiler, AST, bytecode, and VM layers SHALL NOT be aware that any identifier is a built-in. Resolution SHALL happen exclusively at runtime through the existing `LOAD_VAR` opcode and `Environment.get` lookup path. A read of a built-in identifier SHALL be compiled as `LOAD_VAR { index }` against the `bytecode.names` pool, indistinguishable from a read of a user-defined name. + +#### Scenario: The public run wires pi to Math.PI by default + +- **WHEN** `run("pi")` is called with no env argument +- **THEN** the result is exactly `Math.PI` + +#### Scenario: The public run wires e to Math.E by default + +- **WHEN** `run("e")` is called with no env argument +- **THEN** the result is exactly `Math.E` + +#### Scenario: The public run wires tau to 2 \* Math.PI by default + +- **WHEN** `run("tau")` is called with no env argument +- **THEN** the result is exactly `2 * Math.PI` + +#### Scenario: Built-ins compose with arithmetic through the default env + +- **WHEN** `run("2 * pi")` is called with no env argument +- **THEN** the result is exactly `2 * Math.PI` + +#### Scenario: Tau equals two pi end-to-end through the default env + +- **WHEN** `run("tau / 2")` is called with no env argument +- **THEN** the result is exactly `Math.PI` + +#### Scenario: Shadowing roundtrips through the full pipeline + +- **WHEN** `run("pi = 5\npi")` is called with no env argument +- **THEN** the result is `5` + +#### Scenario: A directly-constructed empty Environment does not resolve built-ins + +- **WHEN** a fresh `new Environment()` is constructed with no arguments +- **AND** `env.get("pi")` is called +- **THEN** `UndefinedVariable` is thrown for `pi` + +#### Scenario: A caller can opt out of built-ins by passing an empty Environment to run + +- **WHEN** `run("pi", new Environment())` is called (explicit empty env) +- **THEN** `UndefinedVariable` is thrown for `pi` + +#### Scenario: Environment.ts does not import BUILTIN_CONSTANTS + +- **WHEN** the source of `src/v2-calculator/vm/Environment.ts` is read +- **THEN** the file SHALL NOT contain an `import` of `BUILTIN_CONSTANTS` from `./builtins` or any path + +#### Scenario: vm/run.ts does not import BUILTIN_CONSTANTS + +- **WHEN** the source of `src/v2-calculator/vm/run.ts` is read +- **THEN** the file SHALL NOT contain an `import` of `BUILTIN_CONSTANTS` from `./builtins` or any path diff --git a/package.json b/package.json index c7d8a3a..a0139b0 100644 --- a/package.json +++ b/package.json @@ -24,6 +24,7 @@ "test": "vitest run", "build": "tsc && mkdir -p dist/v1 dist/sketchbook && cp src/v1/grammar.ohm dist/v1/ && cp src/sketchbook/grammar.ohm dist/sketchbook/ && cp src/sketchbook/json.ohm dist/sketchbook/", "lucky": "ts-node src/v2/cli.ts", + "lucky-calculator": "ts-node src/v2-calculator/cli.ts", "ohm": "ohm generateBundles --withTypes 'src/**/*.ohm'" }, "dependencies": { diff --git a/src/v2-calculator/calculator.test.ts b/src/v2-calculator/calculator.test.ts new file mode 100644 index 0000000..be6b070 --- /dev/null +++ b/src/v2-calculator/calculator.test.ts @@ -0,0 +1,172 @@ +import { describe, expect, it } from "vitest"; + +import { run } from "./index"; +import { Environment } from "./vm/Environment"; +import { UndefinedVariable } from "./vm/errors"; + +describe("run", () => { + describe("basic operations", () => { + it("adds", () => { + expect(run("1+2")).toBe(3); + }); + + it("subtracts", () => { + expect(run("5-3")).toBe(2); + }); + + it("multiplies", () => { + expect(run("2*3")).toBe(6); + }); + + it("divides", () => { + expect(run("10/4")).toBe(2.5); + }); + }); + + describe("precedence", () => { + it("multiplies before adding", () => { + expect(run("1+2*3")).toBe(7); + }); + + it("honors parentheses", () => { + expect(run("(1+2)*3")).toBe(9); + }); + + it("makes division left-associative", () => { + expect(run("8/2/2")).toBe(2); + }); + }); + + describe("unary", () => { + it("negates a literal", () => { + expect(run("-5")).toBe(-5); + }); + + it("negates a parenthesized expression", () => { + expect(run("-(1+2)")).toBe(-3); + }); + + it("treats stacked unary minus as identity", () => { + expect(run("--5")).toBe(5); + }); + + it("treats unary plus as identity", () => { + expect(run("+5")).toBe(5); + }); + }); + + describe("decimals", () => { + it("adds decimals precisely when representable", () => { + expect(run("1.5+0.5")).toBe(2); + }); + }); + + describe("whitespace", () => { + it("tolerates surrounding and interleaved whitespace", () => { + expect(run(" 1 + 2 ")).toBe(3); + }); + }); + + describe("native float passthrough", () => { + it("returns Infinity for x/0", () => { + expect(run("1/0")).toBe(Infinity); + }); + + it("returns NaN for 0/0", () => { + expect(Number.isNaN(run("0/0"))).toBe(true); + }); + }); + + describe("errors", () => { + it("throws on a parse error", () => { + expect(() => run("1 +")).toThrow(Error); + }); + }); + + describe("variables", () => { + it("reads a variable set on a previous line of the same program", () => { + expect(run("x = 1 + 2\n1 + x * 2")).toBe(7); + }); + + it("supports chained assignment binding both names to the same value", () => { + expect(run("x = y = 5\nx + y")).toBe(10); + }); + + it("treats assignment as an expression yielding the assigned value", () => { + const env = new Environment(); + expect(run("1 + (x = 2)", env)).toBe(3); + expect(env.get("x")).toBe(2); + }); + + it("throws UndefinedVariable when reading an unbound name", () => { + try { + run("y"); + expect.unreachable("run should have thrown"); + } catch (err) { + expect(err).toBeInstanceOf(UndefinedVariable); + expect((err as UndefinedVariable).name).toBe("y"); + } + }); + + it("persists variables across run() calls that share an Environment", () => { + const env = new Environment(); + run("x = 10", env); + expect(run("x * 2", env)).toBe(20); + }); + + it("isolates variables when each call uses a fresh Environment", () => { + run("x = 1"); + expect(() => run("x")).toThrow(UndefinedVariable); + }); + + it("overwrites the prior binding on reassignment", () => { + expect(run("x = 1\nx = 2\nx")).toBe(2); + }); + }); + + describe("built-in constants", () => { + it("resolves pi to Math.PI", () => { + expect(run("pi")).toBe(Math.PI); + }); + + it("resolves e to Math.E", () => { + expect(run("e")).toBe(Math.E); + }); + + it("resolves tau to 2 * Math.PI", () => { + expect(run("tau")).toBe(2 * Math.PI); + }); + + it("composes pi with arithmetic", () => { + expect(run("2 * pi")).toBe(2 * Math.PI); + }); + + it("composes tau with arithmetic to recover pi", () => { + expect(run("tau / 2")).toBe(Math.PI); + }); + + it("lets user assignment shadow a built-in through the full pipeline", () => { + expect(run("pi = 5\npi")).toBe(5); + }); + + it("still throws UndefinedVariable for names outside the built-in set", () => { + try { + run("phi"); + expect.unreachable("run should have thrown"); + } catch (err) { + expect(err).toBeInstanceOf(UndefinedVariable); + expect((err as UndefinedVariable).name).toBe("phi"); + } + }); + + it("can opt out of built-ins by passing a fresh empty Environment", () => { + try { + run("pi", new Environment()); + expect.unreachable("run should have thrown"); + } catch (err) { + expect(err).toBeInstanceOf(UndefinedVariable); + expect((err as UndefinedVariable).name).toBe("pi"); + } + }); + }); +}); diff --git a/src/v2-calculator/cli.ts b/src/v2-calculator/cli.ts new file mode 100644 index 0000000..9f167b3 --- /dev/null +++ b/src/v2-calculator/cli.ts @@ -0,0 +1,63 @@ +import * as readline from "readline"; + +import { compile } from "./compiler/compiler"; +import { tryParse } from "./parser/parser"; +import { BUILTIN_CONSTANTS } from "./vm/builtins"; +import { Environment } from "./vm/Environment"; +import { run as runBytecode } from "./vm/run"; + +const PROMPT = "> "; + +export function startRepl(): void { + const rl = readline.createInterface({ + input: process.stdin, + output: process.stdout, + prompt: PROMPT, + }); + + const env = new Environment(BUILTIN_CONSTANTS); + env.set("_", 0); + + rl.prompt(); + + rl.on("close", () => { + process.exit(0); + }); + + rl.on("line", (line) => { + const trimmed = line.trim(); + + if (trimmed === "") { + rl.prompt(); + return; + } + + if (trimmed === "exit" || trimmed === "quit") { + rl.close(); + return; + } + + const parsed = tryParse(line); + if (!parsed.ok) { + console.error(parsed.message); + rl.prompt(); + return; + } + + try { + const bytecode = compile(parsed.program); + const result = runBytecode(bytecode, env); + + env.set("_", result); + console.log(result); + } catch (err) { + console.error(err instanceof Error ? err.message : String(err)); + } + + rl.prompt(); + }); +} + +if (require.main === module) { + startRepl(); +} diff --git a/src/v2-calculator/compiler/bytecode.ts b/src/v2-calculator/compiler/bytecode.ts new file mode 100644 index 0000000..3ef3fa7 --- /dev/null +++ b/src/v2-calculator/compiler/bytecode.ts @@ -0,0 +1,27 @@ +export type Instruction = + | { readonly opcode: "LOAD_CONST"; readonly index: number } + /** + * Resolves `bytecode.names[index]` against the runtime `Environment` and + * pushes the result. Throws `UndefinedVariable` if the name has no binding. + */ + | { readonly opcode: "LOAD_VAR"; readonly index: number } + /** + * Reads the top of the operand stack WITHOUT popping and writes it into + * the environment under `bytecode.names[index]`. The value remains on the + * stack so that assignment behaves as an expression yielding the assigned + * value. + */ + | { readonly opcode: "STORE_VAR"; readonly index: number } + | { readonly opcode: "POP" } + | { readonly opcode: "ADD" } + | { readonly opcode: "SUB" } + | { readonly opcode: "MUL" } + | { readonly opcode: "DIV" } + | { readonly opcode: "NEG" } + | { readonly opcode: "HALT" }; + +export interface Bytecode { + readonly constants: readonly number[]; + readonly names: readonly string[]; + readonly code: readonly Instruction[]; +} diff --git a/src/v2-calculator/compiler/compiler.test.ts b/src/v2-calculator/compiler/compiler.test.ts new file mode 100644 index 0000000..b053e02 --- /dev/null +++ b/src/v2-calculator/compiler/compiler.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from "vitest"; + +import { parse } from "../parser/parser"; +import { compile } from "./compiler"; + +describe("compile", () => { + it("emits LOAD_CONST + STORE_VAR + HALT for a bare assignment", () => { + const bc = compile(parse("x = 5")); + expect(bc.code).toEqual([ + { opcode: "LOAD_CONST", index: 0 }, + { opcode: "STORE_VAR", index: 0 }, + { opcode: "HALT" }, + ]); + expect(bc.constants).toEqual([5]); + expect(bc.names).toEqual(["x"]); + }); + + it("emits multiple STORE_VAR over a single value for chained assignment", () => { + const bc = compile(parse("x = y = 5")); + expect(bc.code).toEqual([ + { opcode: "LOAD_CONST", index: 0 }, + { opcode: "STORE_VAR", index: 1 }, + { opcode: "STORE_VAR", index: 0 }, + { opcode: "HALT" }, + ]); + expect(bc.constants).toEqual([5]); + expect(bc.names).toEqual(["x", "y"]); + }); + + it("emits POP between two top-level expressions", () => { + const bc = compile(parse("x = 1\n2")); + expect(bc.code).toEqual([ + { opcode: "LOAD_CONST", index: 0 }, + { opcode: "STORE_VAR", index: 0 }, + { opcode: "POP" }, + { opcode: "LOAD_CONST", index: 1 }, + { opcode: "HALT" }, + ]); + expect(bc.constants).toEqual([1, 2]); + expect(bc.names).toEqual(["x"]); + }); + + it("reuses the same names slot for repeated identifier reads", () => { + const bc = compile(parse("x = 1\nx + x")); + expect(bc.names).toEqual(["x"]); + const loadVars = bc.code.filter((i) => i.opcode === "LOAD_VAR"); + expect(loadVars).toHaveLength(2); + expect(loadVars.every((i) => i.index === 0)).toBe(true); + }); + + it("compiles a read-before-write identifier without throwing", () => { + const bc = compile(parse("y")); + expect(bc.code).toEqual([ + { opcode: "LOAD_VAR", index: 0 }, + { opcode: "HALT" }, + ]); + expect(bc.names).toEqual(["y"]); + }); +}); diff --git a/src/v2-calculator/compiler/compiler.ts b/src/v2-calculator/compiler/compiler.ts new file mode 100644 index 0000000..be675bd --- /dev/null +++ b/src/v2-calculator/compiler/compiler.ts @@ -0,0 +1,75 @@ +import type { Expr, Program } from "../parser/ast"; +import type { Bytecode, Instruction } from "./bytecode"; + +const ARITHMETIC_OPCODE: Record< + "+" | "-" | "*" | "/", + "ADD" | "SUB" | "MUL" | "DIV" +> = { + "+": "ADD", + "-": "SUB", + "*": "MUL", + "/": "DIV", +}; + +export function compile(program: Program): Bytecode { + const code: Instruction[] = []; + const constants: number[] = []; + const names: string[] = []; + + function nameIndex(name: string): number { + const existing = names.indexOf(name); + if (existing !== -1) return existing; + names.push(name); + return names.length - 1; + } + + function emit(expr: Expr): void { + switch (expr.kind) { + case "Literal": { + constants.push(expr.value); + code.push({ opcode: "LOAD_CONST", index: constants.length - 1 }); + return; + } + + case "Arithmetic": { + emit(expr.left); + emit(expr.right); + code.push({ opcode: ARITHMETIC_OPCODE[expr.op] }); + return; + } + + case "Unary": { + emit(expr.expr); + if (expr.op === "-") { + code.push({ opcode: "NEG" }); + } + return; + } + + case "Var": { + code.push({ opcode: "LOAD_VAR", index: nameIndex(expr.name) }); + return; + } + + case "Assign": { + // Reserve the slot for the assignment target BEFORE compiling the + // RHS so chained assignments append names in outer-to-inner order + // (`x = y = 5` → names = ["x", "y"]). + const idx = nameIndex(expr.name); + emit(expr.expr); + code.push({ opcode: "STORE_VAR", index: idx }); + return; + } + } + } + + program.forEach((expr, i) => { + emit(expr); + if (i < program.length - 1) { + code.push({ opcode: "POP" }); + } + }); + code.push({ opcode: "HALT" }); + + return { code, constants, names }; +} diff --git a/src/v2-calculator/index.ts b/src/v2-calculator/index.ts new file mode 100644 index 0000000..a90d0a1 --- /dev/null +++ b/src/v2-calculator/index.ts @@ -0,0 +1,12 @@ +import { compile } from "./compiler/compiler"; +import { parse } from "./parser/parser"; +import { BUILTIN_CONSTANTS } from "./vm/builtins"; +import { Environment } from "./vm/Environment"; +import { run as runBytecode } from "./vm/run"; + +export function run( + source: string, + env: Environment = new Environment(BUILTIN_CONSTANTS), +): number { + return runBytecode(compile(parse(source)), env); +} diff --git a/src/v2-calculator/parser/ast.ts b/src/v2-calculator/parser/ast.ts new file mode 100644 index 0000000..be94f1f --- /dev/null +++ b/src/v2-calculator/parser/ast.ts @@ -0,0 +1,17 @@ +export type Expr = + | { readonly kind: "Literal"; readonly value: number } + | { + readonly kind: "Arithmetic"; + readonly op: "+" | "-" | "*" | "/"; + readonly left: Expr; + readonly right: Expr; + } + | { + readonly kind: "Unary"; + readonly op: "+" | "-"; + readonly expr: Expr; + } + | { readonly kind: "Var"; readonly name: string } + | { readonly kind: "Assign"; readonly name: string; readonly expr: Expr }; + +export type Program = readonly Expr[]; diff --git a/src/v2-calculator/parser/grammar.ohm b/src/v2-calculator/parser/grammar.ohm new file mode 100644 index 0000000..09cdb06 --- /dev/null +++ b/src/v2-calculator/parser/grammar.ohm @@ -0,0 +1,35 @@ +Calculator { + Program = NonemptyListOf newline? + + Expr = AssignExp + + AssignExp + = ident "=" AssignExp -- assign + | AddExp + + AddExp + = AddExp "+" MulExp -- plus + | AddExp "-" MulExp -- minus + | MulExp + + MulExp + = MulExp "*" UnaryExp -- times + | MulExp "/" UnaryExp -- divide + | UnaryExp + + UnaryExp + = "+" UnaryExp -- pos + | "-" UnaryExp -- neg + | PriExp + + PriExp + = "(" Expr ")" -- paren + | ident -- var + | number + + number = digit+ ("." digit+)? + ident = (letter | "_") (letter | digit | "_")* + + newline = "\n"+ + space := " " | "\t" +} diff --git a/src/v2-calculator/parser/grammar.ohm-bundle.d.ts b/src/v2-calculator/parser/grammar.ohm-bundle.d.ts new file mode 100644 index 0000000..b5d35cb --- /dev/null +++ b/src/v2-calculator/parser/grammar.ohm-bundle.d.ts @@ -0,0 +1,51 @@ +// AUTOGENERATED FILE +// This file was generated from grammar.ohm by `ohm generateBundles`. + +import { + BaseActionDict, + Grammar, + IterationNode, + Node, + NonterminalNode, + Semantics, + TerminalNode +} from 'ohm-js'; + +export interface CalculatorActionDict extends BaseActionDict { + Program?: (this: NonterminalNode, arg0: NonterminalNode, arg1: IterationNode) => T; + Expr?: (this: NonterminalNode, arg0: NonterminalNode) => T; + AssignExp_assign?: (this: NonterminalNode, arg0: NonterminalNode, arg1: TerminalNode, arg2: NonterminalNode) => T; + AssignExp?: (this: NonterminalNode, arg0: NonterminalNode) => T; + AddExp_plus?: (this: NonterminalNode, arg0: NonterminalNode, arg1: TerminalNode, arg2: NonterminalNode) => T; + AddExp_minus?: (this: NonterminalNode, arg0: NonterminalNode, arg1: TerminalNode, arg2: NonterminalNode) => T; + AddExp?: (this: NonterminalNode, arg0: NonterminalNode) => T; + MulExp_times?: (this: NonterminalNode, arg0: NonterminalNode, arg1: TerminalNode, arg2: NonterminalNode) => T; + MulExp_divide?: (this: NonterminalNode, arg0: NonterminalNode, arg1: TerminalNode, arg2: NonterminalNode) => T; + MulExp?: (this: NonterminalNode, arg0: NonterminalNode) => T; + UnaryExp_pos?: (this: NonterminalNode, arg0: TerminalNode, arg1: NonterminalNode) => T; + UnaryExp_neg?: (this: NonterminalNode, arg0: TerminalNode, arg1: NonterminalNode) => T; + UnaryExp?: (this: NonterminalNode, arg0: NonterminalNode) => T; + PriExp_paren?: (this: NonterminalNode, arg0: TerminalNode, arg1: NonterminalNode, arg2: TerminalNode) => T; + PriExp_var?: (this: NonterminalNode, arg0: NonterminalNode) => T; + PriExp?: (this: NonterminalNode, arg0: NonterminalNode) => T; + number?: (this: NonterminalNode, arg0: IterationNode, arg1: IterationNode, arg2: IterationNode) => T; + ident?: (this: NonterminalNode, arg0: NonterminalNode | TerminalNode, arg1: IterationNode) => T; + newline?: (this: NonterminalNode, arg0: IterationNode) => T; + space?: (this: NonterminalNode, arg0: TerminalNode) => T; +} + +export interface CalculatorSemantics extends Semantics { + addOperation(name: string, actionDict: CalculatorActionDict): this; + extendOperation(name: string, actionDict: CalculatorActionDict): this; + addAttribute(name: string, actionDict: CalculatorActionDict): this; + extendAttribute(name: string, actionDict: CalculatorActionDict): this; +} + +export interface CalculatorGrammar extends Grammar { + createSemantics(): CalculatorSemantics; + extendSemantics(superSemantics: CalculatorSemantics): CalculatorSemantics; +} + +declare const grammar: CalculatorGrammar; +export default grammar; + diff --git a/src/v2-calculator/parser/grammar.ohm-bundle.js b/src/v2-calculator/parser/grammar.ohm-bundle.js new file mode 100644 index 0000000..278d4c2 --- /dev/null +++ b/src/v2-calculator/parser/grammar.ohm-bundle.js @@ -0,0 +1 @@ +'use strict';const {makeRecipe}=require('ohm-js');const result=makeRecipe(["grammar",{"source":"Calculator {\n Program = NonemptyListOf newline?\n\n Expr = AssignExp\n\n AssignExp\n = ident \"=\" AssignExp -- assign\n | AddExp\n\n AddExp\n = AddExp \"+\" MulExp -- plus\n | AddExp \"-\" MulExp -- minus\n | MulExp\n\n MulExp\n = MulExp \"*\" UnaryExp -- times\n | MulExp \"/\" UnaryExp -- divide\n | UnaryExp\n\n UnaryExp\n = \"+\" UnaryExp -- pos\n | \"-\" UnaryExp -- neg\n | PriExp\n\n PriExp\n = \"(\" Expr \")\" -- paren\n | ident -- var\n | number\n\n number = digit+ (\".\" digit+)?\n ident = (letter | \"_\") (letter | digit | \"_\")*\n\n newline = \"\\n\"+\n space := \" \" | \"\\t\"\n}"},"Calculator",null,"Program",{"Program":["define",{"sourceInterval":[15,63]},null,[],["seq",{"sourceInterval":[25,63]},["app",{"sourceInterval":[25,54]},"NonemptyListOf",[["app",{"sourceInterval":[40,44]},"Expr",[]],["app",{"sourceInterval":[46,53]},"newline",[]]]],["opt",{"sourceInterval":[55,63]},["app",{"sourceInterval":[55,62]},"newline",[]]]]],"Expr":["define",{"sourceInterval":[67,83]},null,[],["app",{"sourceInterval":[74,83]},"AssignExp",[]]],"AssignExp_assign":["define",{"sourceInterval":[103,132]},null,[],["seq",{"sourceInterval":[103,122]},["app",{"sourceInterval":[103,108]},"ident",[]],["terminal",{"sourceInterval":[109,112]},"="],["app",{"sourceInterval":[113,122]},"AssignExp",[]]]],"AssignExp":["define",{"sourceInterval":[87,145]},null,[],["alt",{"sourceInterval":[103,145]},["app",{"sourceInterval":[103,122]},"AssignExp_assign",[]],["app",{"sourceInterval":[139,145]},"AddExp",[]]]],"AddExp_plus":["define",{"sourceInterval":[162,187]},null,[],["seq",{"sourceInterval":[162,179]},["app",{"sourceInterval":[162,168]},"AddExp",[]],["terminal",{"sourceInterval":[169,172]},"+"],["app",{"sourceInterval":[173,179]},"MulExp",[]]]],"AddExp_minus":["define",{"sourceInterval":[194,220]},null,[],["seq",{"sourceInterval":[194,211]},["app",{"sourceInterval":[194,200]},"AddExp",[]],["terminal",{"sourceInterval":[201,204]},"-"],["app",{"sourceInterval":[205,211]},"MulExp",[]]]],"AddExp":["define",{"sourceInterval":[149,233]},null,[],["alt",{"sourceInterval":[162,233]},["app",{"sourceInterval":[162,179]},"AddExp_plus",[]],["app",{"sourceInterval":[194,211]},"AddExp_minus",[]],["app",{"sourceInterval":[227,233]},"MulExp",[]]]],"MulExp_times":["define",{"sourceInterval":[250,278]},null,[],["seq",{"sourceInterval":[250,269]},["app",{"sourceInterval":[250,256]},"MulExp",[]],["terminal",{"sourceInterval":[257,260]},"*"],["app",{"sourceInterval":[261,269]},"UnaryExp",[]]]],"MulExp_divide":["define",{"sourceInterval":[285,314]},null,[],["seq",{"sourceInterval":[285,304]},["app",{"sourceInterval":[285,291]},"MulExp",[]],["terminal",{"sourceInterval":[292,295]},"/"],["app",{"sourceInterval":[296,304]},"UnaryExp",[]]]],"MulExp":["define",{"sourceInterval":[237,329]},null,[],["alt",{"sourceInterval":[250,329]},["app",{"sourceInterval":[250,269]},"MulExp_times",[]],["app",{"sourceInterval":[285,304]},"MulExp_divide",[]],["app",{"sourceInterval":[321,329]},"UnaryExp",[]]]],"UnaryExp_pos":["define",{"sourceInterval":[348,367]},null,[],["seq",{"sourceInterval":[348,360]},["terminal",{"sourceInterval":[348,351]},"+"],["app",{"sourceInterval":[352,360]},"UnaryExp",[]]]],"UnaryExp_neg":["define",{"sourceInterval":[374,393]},null,[],["seq",{"sourceInterval":[374,386]},["terminal",{"sourceInterval":[374,377]},"-"],["app",{"sourceInterval":[378,386]},"UnaryExp",[]]]],"UnaryExp":["define",{"sourceInterval":[333,406]},null,[],["alt",{"sourceInterval":[348,406]},["app",{"sourceInterval":[348,360]},"UnaryExp_pos",[]],["app",{"sourceInterval":[374,386]},"UnaryExp_neg",[]],["app",{"sourceInterval":[400,406]},"PriExp",[]]]],"PriExp_paren":["define",{"sourceInterval":[423,444]},null,[],["seq",{"sourceInterval":[423,435]},["terminal",{"sourceInterval":[423,426]},"("],["app",{"sourceInterval":[427,431]},"Expr",[]],["terminal",{"sourceInterval":[432,435]},")"]]],"PriExp_var":["define",{"sourceInterval":[451,463]},null,[],["app",{"sourceInterval":[451,456]},"ident",[]]],"PriExp":["define",{"sourceInterval":[410,476]},null,[],["alt",{"sourceInterval":[423,476]},["app",{"sourceInterval":[423,435]},"PriExp_paren",[]],["app",{"sourceInterval":[451,456]},"PriExp_var",[]],["app",{"sourceInterval":[470,476]},"number",[]]]],"number":["define",{"sourceInterval":[480,509]},null,[],["seq",{"sourceInterval":[489,509]},["plus",{"sourceInterval":[489,495]},["app",{"sourceInterval":[489,494]},"digit",[]]],["opt",{"sourceInterval":[496,509]},["seq",{"sourceInterval":[497,507]},["terminal",{"sourceInterval":[497,500]},"."],["plus",{"sourceInterval":[501,507]},["app",{"sourceInterval":[501,506]},"digit",[]]]]]]],"ident":["define",{"sourceInterval":[512,558]},null,[],["seq",{"sourceInterval":[520,558]},["alt",{"sourceInterval":[521,533]},["app",{"sourceInterval":[521,527]},"letter",[]],["terminal",{"sourceInterval":[530,533]},"_"]],["star",{"sourceInterval":[535,558]},["alt",{"sourceInterval":[536,556]},["app",{"sourceInterval":[536,542]},"letter",[]],["app",{"sourceInterval":[545,550]},"digit",[]],["terminal",{"sourceInterval":[553,556]},"_"]]]]],"newline":["define",{"sourceInterval":[562,577]},null,[],["plus",{"sourceInterval":[572,577]},["terminal",{"sourceInterval":[572,576]},"\n"]]],"space":["override",{"sourceInterval":[580,599]},null,[],["alt",{"sourceInterval":[589,599]},["terminal",{"sourceInterval":[589,592]}," "],["terminal",{"sourceInterval":[595,599]},"\t"]]]}]);module.exports=result; \ No newline at end of file diff --git a/src/v2-calculator/parser/parser.test.ts b/src/v2-calculator/parser/parser.test.ts new file mode 100644 index 0000000..a8906de --- /dev/null +++ b/src/v2-calculator/parser/parser.test.ts @@ -0,0 +1,326 @@ +import { describe, expect, it } from "vitest"; + +import { parse, tryParse } from "./parser"; + +describe("parse", () => { + describe("literals", () => { + it("parses a bare integer", () => { + expect(parse("42")).toEqual([{ kind: "Literal", value: 42 }]); + }); + + it("parses a decimal", () => { + expect(parse("3.14")).toEqual([{ kind: "Literal", value: 3.14 }]); + }); + }); + + describe("arithmetic", () => { + it("makes subtraction left-associative", () => { + expect(parse("1 - 2 - 3")).toEqual([ + { + kind: "Arithmetic", + op: "-", + left: { + kind: "Arithmetic", + op: "-", + left: { kind: "Literal", value: 1 }, + right: { kind: "Literal", value: 2 }, + }, + right: { kind: "Literal", value: 3 }, + }, + ]); + }); + + it("binds multiplication tighter than addition", () => { + expect(parse("1 + 2 * 3")).toEqual([ + { + kind: "Arithmetic", + op: "+", + left: { kind: "Literal", value: 1 }, + right: { + kind: "Arithmetic", + op: "*", + left: { kind: "Literal", value: 2 }, + right: { kind: "Literal", value: 3 }, + }, + }, + ]); + }); + + it("lets parentheses override precedence", () => { + expect(parse("(1+2)*3")).toEqual([ + { + kind: "Arithmetic", + op: "*", + left: { + kind: "Arithmetic", + op: "+", + left: { kind: "Literal", value: 1 }, + right: { kind: "Literal", value: 2 }, + }, + right: { kind: "Literal", value: 3 }, + }, + ]); + }); + }); + + describe("unary", () => { + it("parses unary minus", () => { + expect(parse("-5")).toEqual([ + { + kind: "Unary", + op: "-", + expr: { kind: "Literal", value: 5 }, + }, + ]); + }); + + it("stacks unary minus", () => { + expect(parse("--5")).toEqual([ + { + kind: "Unary", + op: "-", + expr: { + kind: "Unary", + op: "-", + expr: { kind: "Literal", value: 5 }, + }, + }, + ]); + }); + + it("represents unary plus as a Unary node, not a collapsed Literal", () => { + expect(parse("+5")).toEqual([ + { + kind: "Unary", + op: "+", + expr: { kind: "Literal", value: 5 }, + }, + ]); + }); + }); + + describe("variables", () => { + it("parses a bare identifier as Var", () => { + expect(parse("x")).toEqual([{ kind: "Var", name: "x" }]); + }); + + it("parses a bare assignment", () => { + expect(parse("x = 1")).toEqual([ + { + kind: "Assign", + name: "x", + expr: { kind: "Literal", value: 1 }, + }, + ]); + }); + + it("parses chained assignment right-associatively", () => { + expect(parse("x = y = 1")).toEqual([ + { + kind: "Assign", + name: "x", + expr: { + kind: "Assign", + name: "y", + expr: { kind: "Literal", value: 1 }, + }, + }, + ]); + }); + + it("treats parenthesised assignment as a sub-expression", () => { + expect(parse("(x = 1) + 2")).toEqual([ + { + kind: "Arithmetic", + op: "+", + left: { + kind: "Assign", + name: "x", + expr: { kind: "Literal", value: 1 }, + }, + right: { kind: "Literal", value: 2 }, + }, + ]); + }); + + it("allows identifiers starting with underscore", () => { + expect(parse("_foo = 1")).toEqual([ + { + kind: "Assign", + name: "_foo", + expr: { kind: "Literal", value: 1 }, + }, + ]); + }); + + it("treats a bare underscore as a legal identifier", () => { + expect(parse("_foo = 1\n_")).toEqual([ + { + kind: "Assign", + name: "_foo", + expr: { kind: "Literal", value: 1 }, + }, + { kind: "Var", name: "_" }, + ]); + }); + + it("allows identifiers with underscores after the first character", () => { + expect(parse("foo_bar = 1")).toEqual([ + { + kind: "Assign", + name: "foo_bar", + expr: { kind: "Literal", value: 1 }, + }, + ]); + }); + + it("allows identifiers with digits after the first character", () => { + expect(parse("foo123 = 1")).toEqual([ + { + kind: "Assign", + name: "foo123", + expr: { kind: "Literal", value: 1 }, + }, + ]); + }); + }); + + describe("multi-line programs", () => { + it("parses two expressions separated by a newline", () => { + expect(parse("x = 1 + 2\n1 + x * 2")).toEqual([ + { + kind: "Assign", + name: "x", + expr: { + kind: "Arithmetic", + op: "+", + left: { kind: "Literal", value: 1 }, + right: { kind: "Literal", value: 2 }, + }, + }, + { + kind: "Arithmetic", + op: "+", + left: { kind: "Literal", value: 1 }, + right: { + kind: "Arithmetic", + op: "*", + left: { kind: "Var", name: "x" }, + right: { kind: "Literal", value: 2 }, + }, + }, + ]); + }); + + it("accepts an optional trailing newline before EOF", () => { + expect(parse("1\n")).toEqual([{ kind: "Literal", value: 1 }]); + expect(parse("x = 1\ny = 2\n")).toEqual([ + { + kind: "Assign", + name: "x", + expr: { kind: "Literal", value: 1 }, + }, + { + kind: "Assign", + name: "y", + expr: { kind: "Literal", value: 2 }, + }, + ]); + }); + + it("collapses blank lines between expressions", () => { + const program = parse("x = 1\n\n\ny = 2"); + expect(program).toHaveLength(2); + expect(program[0]).toEqual({ + kind: "Assign", + name: "x", + expr: { kind: "Literal", value: 1 }, + }); + expect(program[1]).toEqual({ + kind: "Assign", + name: "y", + expr: { kind: "Literal", value: 2 }, + }); + }); + }); + + describe("rejected sources", () => { + it("rejects assignment to the right of an arithmetic operator", () => { + expect(() => parse("1 + x = 2")).toThrow(Error); + }); + + it("rejects an identifier that starts with a digit", () => { + expect(() => parse("1foo = 1")).toThrow(Error); + }); + + it("rejects a newline inside an expression", () => { + expect(() => parse("1 +\n2")).toThrow(Error); + }); + + it("rejects the empty source", () => { + expect(() => parse("")).toThrow(Error); + }); + + it("rejects assignment with a parenthesised LHS", () => { + expect(() => parse("(x) = 1")).toThrow(Error); + }); + + it("rejects juxtaposed numbers", () => { + expect(() => parse("1 1")).toThrow(Error); + }); + + it("throws on a trailing operator", () => { + expect(() => parse("1 +")).toThrow(Error); + }); + }); +}); + +describe("tryParse", () => { + it("flags a trailing operator as incomplete", () => { + const result = tryParse("1 +"); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.incomplete).toBe(true); + expect(typeof result.message).toBe("string"); + } + }); + + it("flags juxtaposed numbers as not incomplete", () => { + const result = tryParse("1 1"); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.incomplete).toBe(false); + } + }); + + it("flags an unclosed paren as incomplete", () => { + const result = tryParse("(1 + 2"); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.incomplete).toBe(true); + } + }); + + it("flags a dangling `=` as incomplete", () => { + const result = tryParse("x ="); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.incomplete).toBe(true); + } + }); + + it("returns the program on success", () => { + const result = tryParse("1 + 2"); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.program).toEqual([ + { + kind: "Arithmetic", + op: "+", + left: { kind: "Literal", value: 1 }, + right: { kind: "Literal", value: 2 }, + }, + ]); + } + }); +}); diff --git a/src/v2-calculator/parser/parser.ts b/src/v2-calculator/parser/parser.ts new file mode 100644 index 0000000..ad13d04 --- /dev/null +++ b/src/v2-calculator/parser/parser.ts @@ -0,0 +1,33 @@ +import type { Program } from "./ast"; +import grammar from "./grammar.ohm-bundle"; +import { semantics } from "./semantics"; + +export type TryParseResult = + | { ok: true; program: Program } + | { ok: false; incomplete: true; message: string } + | { ok: false; incomplete: false; message: string }; + +export function tryParse(code: string): TryParseResult { + const matchResult = grammar.match(code); + + if (matchResult.failed()) { + const message = matchResult.message; + if (matchResult.getRightmostFailurePosition() === code.length) { + return { ok: false, incomplete: true, message }; + } + return { ok: false, incomplete: false, message }; + } + + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + return { ok: true, program: semantics(matchResult).toAst() as Program }; +} + +export function parse(code: string): Program { + const result = tryParse(code); + + if (!result.ok) { + throw new Error(result.message); + } + + return result.program; +} diff --git a/src/v2-calculator/parser/semantics.ts b/src/v2-calculator/parser/semantics.ts new file mode 100644 index 0000000..110192f --- /dev/null +++ b/src/v2-calculator/parser/semantics.ts @@ -0,0 +1,120 @@ +import type { IterationNode, Node, NonterminalNode } from "ohm-js"; + +import type { Expr, Program } from "./ast"; +import grammar from "./grammar.ohm-bundle"; + +/* eslint-disable @typescript-eslint/no-unsafe-call */ + +/** + * Heterogeneous return shape for `toAst`. Ohm operations are uni-typed (`T`), + * but our actions legitimately return different node kinds depending on the + * rule. Callers narrow via `as` at the boundary. + */ +type AstResult = Program | Expr | string; + +export const semantics = grammar.createSemantics(); + +semantics.addOperation("toAst", { + Program( + this: NonterminalNode, + nelist: NonterminalNode, + _trailingNewline: IterationNode, + ): Program { + return nelist.asIteration().children.map((c) => c.toAst() as Expr); + }, + AssignExp_assign( + this: NonterminalNode, + name: NonterminalNode, + _eq: Node, + rhs: NonterminalNode, + ): Expr { + return { + kind: "Assign", + name: name.sourceString, + expr: rhs.toAst() as Expr, + }; + }, + AddExp_plus( + this: NonterminalNode, + left: NonterminalNode, + _op: Node, + right: NonterminalNode, + ): Expr { + return { + kind: "Arithmetic", + op: "+", + left: left.toAst() as Expr, + right: right.toAst() as Expr, + }; + }, + AddExp_minus( + this: NonterminalNode, + left: NonterminalNode, + _op: Node, + right: NonterminalNode, + ): Expr { + return { + kind: "Arithmetic", + op: "-", + left: left.toAst() as Expr, + right: right.toAst() as Expr, + }; + }, + MulExp_times( + this: NonterminalNode, + left: NonterminalNode, + _op: Node, + right: NonterminalNode, + ): Expr { + return { + kind: "Arithmetic", + op: "*", + left: left.toAst() as Expr, + right: right.toAst() as Expr, + }; + }, + MulExp_divide( + this: NonterminalNode, + left: NonterminalNode, + _op: Node, + right: NonterminalNode, + ): Expr { + return { + kind: "Arithmetic", + op: "/", + left: left.toAst() as Expr, + right: right.toAst() as Expr, + }; + }, + UnaryExp_pos( + this: NonterminalNode, + _plus: Node, + operand: NonterminalNode, + ): Expr { + return { kind: "Unary", op: "+", expr: operand.toAst() as Expr }; + }, + UnaryExp_neg( + this: NonterminalNode, + _minus: Node, + operand: NonterminalNode, + ): Expr { + return { kind: "Unary", op: "-", expr: operand.toAst() as Expr }; + }, + PriExp_paren( + this: NonterminalNode, + _lp: Node, + inner: NonterminalNode, + _rp: Node, + ): Expr { + return inner.toAst() as Expr; + }, + PriExp_var(this: NonterminalNode, idNode: NonterminalNode): Expr { + return { kind: "Var", name: idNode.sourceString }; + }, + number(this: NonterminalNode, _digits: Node, _dot: Node, _frac: Node): Expr { + return { kind: "Literal", value: parseFloat(this.sourceString) }; + }, + ident(this: NonterminalNode, _first: Node, _rest: IterationNode): string { + return this.sourceString; + }, +}); diff --git a/src/v2-calculator/todo.md b/src/v2-calculator/todo.md new file mode 100644 index 0000000..be5eacc --- /dev/null +++ b/src/v2-calculator/todo.md @@ -0,0 +1,14 @@ +- [x] Add variables, like x = 1 + 2 (no let keyword for simplicity) +- [x] Repl bind last result to \_ +- [ ] Write integration tests +- [x] Add built-in constants, like pi +- [ ] Add IEEE-754 named constants: `inf`, `nan` +- [ ] Add built-in functions, like sin, cos, tan, etc. +- [ ] Underscores in numeric literals: 1_000_000 +- [ ] Scientific notation, like 1e3; 1e-3 +- [ ] Binary literals: 0b1010 +- [ ] Hexadecimal literals: 0x1A +- [ ] Factorial postfix operator: 5! +- [ ] Power operator: 2^3 +- [ ] Error messages with line:column pointers +- [ ] User defined functions: f(x) = x^2; add(x, y) = x + y diff --git a/src/v2-calculator/vm/Environment.test.ts b/src/v2-calculator/vm/Environment.test.ts new file mode 100644 index 0000000..cadbbdc --- /dev/null +++ b/src/v2-calculator/vm/Environment.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from "vitest"; + +import { Environment } from "./Environment"; +import { UndefinedVariable } from "./errors"; + +describe("Environment", () => { + it("throws UndefinedVariable when getting a name that was never set", () => { + const env = new Environment(); + expect(() => env.get("x")).toThrow(UndefinedVariable); + }); + + it("returns the stored value after set", () => { + const env = new Environment(); + env.set("x", 42); + expect(env.get("x")).toBe(42); + }); + + it("overwrites an existing binding on subsequent set", () => { + const env = new Environment(); + env.set("x", 1); + env.set("x", 2); + expect(env.get("x")).toBe(2); + }); + + it("attaches the queried name to the thrown error", () => { + const env = new Environment(); + try { + env.get("not_bound"); + expect.unreachable("env.get should have thrown"); + } catch (err) { + expect(err).toBeInstanceOf(UndefinedVariable); + expect((err as UndefinedVariable).name).toBe("not_bound"); + expect((err as UndefinedVariable).message).toBe( + "undefined variable: not_bound", + ); + } + }); + + describe("initialBindings constructor parameter", () => { + it("copies initialBindings into storage on construction", () => { + const env = new Environment({ foo: 1, bar: 2 }); + expect(env.get("foo")).toBe(1); + expect(env.get("bar")).toBe(2); + }); + + it("starts empty when constructed with no arguments", () => { + const env = new Environment(); + expect(() => env.get("pi")).toThrow(UndefinedVariable); + }); + + it("seeded bindings can still be overwritten by set", () => { + const env = new Environment({ foo: 1 }); + env.set("foo", 99); + expect(env.get("foo")).toBe(99); + }); + }); +}); diff --git a/src/v2-calculator/vm/Environment.ts b/src/v2-calculator/vm/Environment.ts new file mode 100644 index 0000000..b9fa366 --- /dev/null +++ b/src/v2-calculator/vm/Environment.ts @@ -0,0 +1,30 @@ +import { UndefinedVariable } from "./errors"; + +/** + * Single-scope variable storage. No enclosing chain, no `define` vs `assign` + * split — reassignment is implicit (`x = 1; x = 2` just overwrites). Policy- + * free mechanism: this class does not know about calculator built-ins. + * Callers seed initial bindings via the optional `initialBindings` parameter + * (the calculator's composition roots pass `BUILTIN_CONSTANTS`; tests typically + * pass nothing for a genuinely empty env). + */ +export class Environment { + private readonly records = new Map(); + + constructor(initialBindings: Readonly> = {}) { + for (const [name, value] of Object.entries(initialBindings)) { + this.records.set(name, value); + } + } + + get(name: string): number { + if (!this.records.has(name)) { + throw new UndefinedVariable(name); + } + return this.records.get(name)!; + } + + set(name: string, value: number): void { + this.records.set(name, value); + } +} diff --git a/src/v2-calculator/vm/Stack.test.ts b/src/v2-calculator/vm/Stack.test.ts new file mode 100644 index 0000000..8eaf05a --- /dev/null +++ b/src/v2-calculator/vm/Stack.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it } from "vitest"; + +import { StackOverflow, StackUnderflow } from "./errors"; +import { Stack } from "./Stack"; + +describe("Stack", () => { + it("pop returns the most recently pushed value", () => { + const stack = new Stack(); + stack.push(1); + stack.push(2); + expect(stack.pop()).toBe(2); + expect(stack.pop()).toBe(1); + }); + + it("pop on an empty stack throws StackUnderflow", () => { + const stack = new Stack(); + expect(() => stack.pop()).toThrow(StackUnderflow); + }); + + it("peek returns the top without removing it", () => { + const stack = new Stack(); + stack.push(42); + expect(stack.peek()).toBe(42); + expect(stack.peek()).toBe(42); + expect(stack.size).toBe(1); + }); + + it("peek on an empty stack throws StackUnderflow", () => { + const stack = new Stack(); + expect(() => stack.peek()).toThrow(StackUnderflow); + }); + + it("isEmpty reflects whether anything has been pushed", () => { + const stack = new Stack(); + expect(stack.isEmpty()).toBe(true); + stack.push(1); + expect(stack.isEmpty()).toBe(false); + stack.pop(); + expect(stack.isEmpty()).toBe(true); + }); + + it("size tracks pushes and pops", () => { + const stack = new Stack(); + expect(stack.size).toBe(0); + stack.push(1); + stack.push(2); + expect(stack.size).toBe(2); + stack.pop(); + expect(stack.size).toBe(1); + }); + + it("pushing past the limit throws StackOverflow carrying the limit", () => { + const stack = new Stack(2); + stack.push(1); + stack.push(2); + try { + stack.push(3); + expect.unreachable("third push should have thrown"); + } catch (err) { + expect(err).toBeInstanceOf(StackOverflow); + expect((err as StackOverflow).limit).toBe(2); + expect((err as StackOverflow).message).toBe("stack overflow (limit: 2)"); + } + }); + + it("a rejected push does not commit the slot", () => { + const stack = new Stack(2); + stack.push(1); + stack.push(2); + expect(() => stack.push(3)).toThrow(StackOverflow); + expect(stack.size).toBe(2); + expect(stack.peek()).toBe(2); + }); + + it("defaults to a limit of 1024", () => { + const stack = new Stack(); + for (let i = 0; i < 1024; i++) { + stack.push(i); + } + expect(stack.size).toBe(1024); + expect(() => stack.push(1024)).toThrow(StackOverflow); + }); +}); diff --git a/src/v2-calculator/vm/Stack.ts b/src/v2-calculator/vm/Stack.ts new file mode 100644 index 0000000..62072f5 --- /dev/null +++ b/src/v2-calculator/vm/Stack.ts @@ -0,0 +1,51 @@ +import { StackOverflow, StackUnderflow } from "./errors"; + +const DEFAULT_LIMIT = 1024; + +/** + * Fixed-capacity LIFO stack of numbers used by the V2-calculator VM. + * + * Centralises the "is it empty?" and "is it full?" checks so every opcode + * handler can rely on `push`/`pop`/`peek` returning a `number` or throwing a + * typed error — no `undefined` ever leaks into arithmetic. + */ +export class Stack { + private readonly values: number[] = []; + private readonly limit: number; + + constructor(limit: number = DEFAULT_LIMIT) { + this.limit = limit; + } + + push(value: number): void { + if (this.values.length >= this.limit) { + throw new StackOverflow(this.limit); + } + + this.values.push(value); + } + + pop(): number { + if (this.isEmpty()) { + throw new StackUnderflow(); + } + + return this.values.pop()!; + } + + peek(): number { + if (this.isEmpty()) { + throw new StackUnderflow(); + } + + return this.values[this.values.length - 1]!; + } + + isEmpty(): boolean { + return this.size === 0; + } + + get size(): number { + return this.values.length; + } +} diff --git a/src/v2-calculator/vm/builtins.ts b/src/v2-calculator/vm/builtins.ts new file mode 100644 index 0000000..9eb6c56 --- /dev/null +++ b/src/v2-calculator/vm/builtins.ts @@ -0,0 +1,13 @@ +/** + * Canonical registry of names pre-bound in every fresh `Environment`. The + * `Environment` constructor copies these entries into its internal `Map`, so + * `env.get("pi")` resolves to `Math.PI` without any user setup. Built-ins are + * ordinary bindings: a user assignment (`pi = 5`) silently overwrites the slot + * for that `Environment` instance only. + */ +export const BUILTIN_CONSTANTS: Readonly> = + Object.freeze({ + pi: Math.PI, + e: Math.E, + tau: 2 * Math.PI, + }); diff --git a/src/v2-calculator/vm/errors.ts b/src/v2-calculator/vm/errors.ts new file mode 100644 index 0000000..f1ebb09 --- /dev/null +++ b/src/v2-calculator/vm/errors.ts @@ -0,0 +1,31 @@ +/** + * Thrown by `Environment.get` when no binding exists for the queried name. + * + * The `name` field deliberately shadows `Error.name` (normally the class name) + * to hold the *variable identifier* — the calculator's spec keeps the error + * class minimal: one public field, one message format. Disambiguate the kind + * with `instanceof UndefinedVariable`, not `err.name === "UndefinedVariable"`. + */ +export class UndefinedVariable extends Error { + override readonly name: string; + + constructor(name: string) { + super(`undefined variable: ${name}`); + this.name = name; + } +} + +export class StackUnderflow extends Error { + constructor() { + super("stack underflow"); + } +} + +export class StackOverflow extends Error { + readonly limit: number; + + constructor(limit: number) { + super(`stack overflow (limit: ${limit})`); + this.limit = limit; + } +} diff --git a/src/v2-calculator/vm/run.ts b/src/v2-calculator/vm/run.ts new file mode 100644 index 0000000..53096b3 --- /dev/null +++ b/src/v2-calculator/vm/run.ts @@ -0,0 +1,75 @@ +import type { Bytecode } from "../compiler/bytecode"; +import type { Environment } from "./Environment"; +import { Stack } from "./Stack"; + +export function run(bytecode: Bytecode, env: Environment): number { + const stack = new Stack(); + let ip = 0; + + for (;;) { + const instr = bytecode.code[ip]!; + ip++; + + switch (instr.opcode) { + case "LOAD_CONST": { + const value = bytecode.constants[instr.index]!; + stack.push(value); + break; + } + + case "LOAD_VAR": { + const name = bytecode.names[instr.index]!; + const value = env.get(name); + stack.push(value); + break; + } + + case "STORE_VAR": { + const name = bytecode.names[instr.index]!; + env.set(name, stack.peek()); + break; + } + + case "POP": + stack.pop(); + break; + + case "ADD": { + const right = stack.pop(); + const left = stack.pop(); + stack.push(left + right); + break; + } + + case "SUB": { + const right = stack.pop(); + const left = stack.pop(); + stack.push(left - right); + break; + } + + case "MUL": { + const right = stack.pop(); + const left = stack.pop(); + stack.push(left * right); + break; + } + + case "DIV": { + const right = stack.pop(); + const left = stack.pop(); + stack.push(left / right); + break; + } + + case "NEG": { + const value = stack.pop(); + stack.push(-value); + break; + } + + case "HALT": + return stack.pop(); + } + } +}