crowmaki/catlang
Watch
1
0
Fork
You've already forked catlang
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

parser-fixes #7

Merged
crowmaki merged 19 commits from parser-fixes into main 2026-08-22 02:22:01 +00:00
Conversation 0 Commits 19 Files changed 20 +1891 -522
crowmaki commented 2026-08-22 02:21:35 +00:00
Owner
Copy link

Parser fixes based on review.

Parser fixes based on review.
Records 34 findings from a review of the lexer and parser crates against
language-grammar.md, language-spec.md, and the 56 fixtures. Grouped by area
(parser correctness, lexer correctness, grammar defects, AST design, test
contract, modernization) with a status column, so the follow-up work has one
tracking surface instead of living in a conversation.

Items marked with a check were reproduced by running code, not read off the
source.
AGENTS.md makes fixture acceptance the definition of done, but nothing in the
suite read the fixture tree, so the manifest's green rows were claims rather
than tests.

lexer/tests/fixtures.rs asserts every .cat file in the tree tokenizes and ends
in exactly one Eof. All 56 pass. It panics on an empty walk and collects
failures as file:line:col, so it reports the whole set rather than dying on the
first one; verified against a deliberately malformed fixture.

parser/tests/fixtures.rs does the same for the 49 parse/semantics fixtures
(compile implies parse; lex/ is excluded, since a fixture claims nothing higher
than its phase). It is #[ignore]d because parse() is still the step-00 stub and
accepts anything, so enabling it today would pass vacuously. It instead asserts
what the stub cannot satisfy - a non-empty fixture yields at least one
declaration - and fails 49/49 when forced with --ignored. Step 70 should turn it
green by deleting the attribute.

Phase: lex (green), parse (pending step 70).
parse_type_group read each member with parse_type_atom, so a `|` inside parens
was never consumed and the group failed with "expected ')' after type". That
rejected the grammar's own disambiguation example (language-grammar.md: "(int|
float)" is union), the spec's `type b = (int | float);` under Tuples, and two
fixtures the manifest binds to the parse phase - union-propagation.cat's
`*(x | y)` and interfaces.cat's `impl<T> ICollection<T> for ([]T | [..]T)`.

Group and tuple members now parse as full union-types, which is what the
grammar says: type-atom admits "(" union-type ")", and a tuple member is a type.

Union Propagation makes `*(x|y)` and `*x|*y` equivalent, but that is a semantic
rule - the parser preserves what was written and leaves distribution to a later
phase.

Known gap: `([]T | [..]T)` now parses, but both legs still dump as `([] T)`
because TypeRef::ArrayDyn does not record the `..` (backlog #6), so
interfaces.cat's head is not yet correct.

Phase: parse. Backlog #1.
Three findings in the step-20 type parser. They land together because they all
touch parse_type_* in one file and could not be staged apart cleanly.

#6 - []T vs [..]T. parse_type_array skipped the `..` without recording it, so
both forms became TypeRef::ArrayDyn and dumped as `([] T)`. They are different
type constructors: `[]T` is a static array whose size comes from context,
`[..]T` is the dynamic, heap-backed one - the difference between
static-arrays.cat and dynamic-arrays.cat. An existing assertion locked the
collapse in. Split into ArrayInferred and ArrayDynamic; interfaces.cat's head
now dumps `(| ([] T) ([..] T))`, which is what 60-user-types.md asserts and
which nothing could produce before. NewTarget::ArrayDyn renamed to ArrayDynamic
so both enums use one vocabulary (pure rename, same dump).

#4 - spans. Prefix wrappers reported only their operator token, so `?int` spanned
one byte; tuples stopped at the last member rather than the `)`, and `Name<...>`
at the last argument rather than the `>`. 10-ast.md promises every node spans
what it was parsed from, and diagnostics read it. All four now join through to
the terminator; parse_type_args returns the closing `>` span to allow it, and
the newly unused tok_span helper is gone.

#5 - qualified names. `A::123` parsed into a Name with a numeric segment: the
`::` loop consumed whatever token followed, and the first-token check ran after
the consume, so its error pointed one token late. Both checks now precede the
consume.

Also gates the four golden-harness helpers behind #[cfg(test)]; they are called
only from #[test] functions, so the normal build was compiling them and
reporting each as dead code.

Phase: parse. Backlog #4, #5, #6.
Records what the four fix commits changed, and two findings noticed while
making them: a needless leg-vector clone in parse_type_union, and NewTarget
lacking the size-inferred arm that #6 just added on the TypeRef side.
`Span<Span<u8>>` failed with "expected '>' to close generic arguments": the
scanner had already matched the two closers as one GtGt, and parse_type_args
only accepted Gt.

The scanner is right to match greedily - `>>` is a shift and `>>=` a compound
assignment - so rather than teach it about type context, the parser now splits
the token where it needs to. Cursor::eat_generic_close takes the leading `>` and
rewrites the token under the cursor to the remainder (GtGt -> Gt, GtEq -> Eq,
GtGtEq -> GtEq), so the next close finds a well-formed token waiting. The stream
stays consistent and nothing backtracks.

Covers the `>`-initial tokens a type annotation can run into, not just `>>`: a
`let x: A<B<u8>>= e` glues the close against the assignment.

Phase: parse. Backlog #2.
`(int) -> a|b` parsed as `(| (fun int -> a) b)` - a union of a function and a
`b`. The grammar says `function-type = "(" [ type … ] ")" "->" type`, and `type`
is a union-type, so the arrow takes the whole return: `(fun int -> (| a b))`.
unions.cat returns `bool | int` for exactly this reason.

All three arrow arms in parse_type_group read the return with parse_type_atom;
they now read it with parse_type_union. A union outside the arrow still parses
that way when written that way: `((int) -> a) | b`.

Phase: parse. Backlog #3.
Both are terminals in the grammar - type-alias uses `type`, the type-modifier
production uses `volatile` - but neither was in keyword_of, so both reached the
parser as ordinary identifiers and `type a = *(x|y);` was indistinguishable
from a variable named `type`.

A grammar that reserves a word the scanner does not is the same defect one
layer up, so this lands with the grammar pass that adds them to the reserved
list. No fixture uses either as a plain identifier, and all 56 still lex.

Phase: lex. Backlog #9, #18.
Eleven defects found by checking docs/prd/language-grammar.md against
language-spec.md, lexer/src/, and all 56 fixtures. Grammar-only except where
noted; no parser code changes.

#9  Reserved words. Added `type` (a terminal in type-alias), `void`, `true`,
    `false`, `null` (all reserved by the scanner, all missing from the list),
    and `volatile`. `const` is listed twice on purpose - one keyword, two roles.
#10 Removed the postfix `?` operator. The spec's precedence table lists
    `a++ a-- a! a() a[] a. a?.` and no bare `a?`, and UnOp has no node for it.
    It existed only to create the ternary ambiguity that the file then spent a
    paragraph resolving - conceding a parser "must peek past the ternary-true
    arm", which is unbounded lookahead and contradicts the no-backtracking
    claim two paragraphs above. Removing it removes the ambiguity.
#11 `as` and `is` take a type-ATOM, not a full type. Both bind tighter than the
    bitwise `|`, so letting the type slot swallow a union silently inverted
    `x as int | float`. `as` also repeats now, so `x as A as B` is derivable.
#12 Added `::` at level 1 and defined `qualified-name`, which the expression
    section used but never defined.
#13 `func-value`, `params`, and `param` were each defined twice, and the copies
    disagreed. Defined once, in Functions.
#14 `enum-decl`'s `{ variant { "," variant } }` was a repetition wrapped around
    a repetition: it admitted `A B` and rejected the trailing comma the enum
    fixtures write.
#15 Lexical rules now match the scanner: `123.` and exponents, escapes in char
    and string literals, `${expr}` not `${ident}`, `0X`. Notes two scanner
    behaviours the productions cannot express - unknown escapes pass through,
    and string bodies stay raw through the lex phase.
#16 `program` admits statements at file scope, which AGENTS.md sanctions and
    eleven fixtures rely on.
#17 The `;` after a func-value is optional; only global-functions.cat writes it.
#18 Added the type-modifier production (`const`, `volatile`) as an atom prefix,
    so `volatile u32` and `*const []T` derive. Both fixtures are compile-phase,
    so they have to parse first.
#19 Optional param annotations, element-type-less array literals (`[] {1,2}`),
    generic structs, and type-PARAMETERS on interface and struct heads.

Also dropped the redundant `"Span"` type-atom arm, which contradicted the note
directly beneath it saying Span is an ordinary named type.

Three findings are recorded rather than resolved, because each is a design call
rather than a transcription fix: overlapping declaration arms (#37), whether
`as` groups with the unary prefixes as the spec's table implies (#38), and a
fixture that ends a file-scope call without a semicolon (#39).

Backlog #9-#19.
`?.` and a ternary whose true-arm is an elided enum variant are the same three
tokens - `ident ?. ident` - so `const a = flag ?.Left : .Right;` scanned as a
null-conditional member access on `flag` followed by a stray `:`. Both halves
are real language: enums-elided.cat writes `.Left`, null-coalescing.cat writes
`obj?.DoMethod()`. Neither the spec nor the grammar mentioned the collision.

The scanner now emits QuestionDot only when the `?` touches the operand before
it; with whitespace in front the `?` opens a ternary and the `.` begins an
elided variant. This is Swift's rule for the same collision and costs one
comparison in punct2.

It has to be settled in the scanner. Once `?.` is one token the parser cannot
recover the alternative without scanning ahead for the `:` - the unbounded
lookahead that removing the invented postfix `?` had just eliminated.

Consequence: spaced `obj ?. DoMethod()` is no longer a member access. Nothing
in the spec or the fixtures writes it that way, and all 56 still lex.

Found by asking whether `?.` was still meaningful after the postfix `?` came
out of the grammar. Corrects the claim in that pass that the ternary needed no
lookahead at all.

Phase: lex. Backlog #40; amends #10.
Two ways a numeric literal could reach the parser carrying a value the source
did not name.

#7 Float literals were wrong. The scanner accumulated `mantissa * 10^n` in f64
   by repeated multiply or divide, which drifts: `1e40` came back as
   9.999999999999998e39, `0.000001` as 1.0000000000000002e-6, and the smallest
   normal double as 2.225073858507203e-308. Replaced with `str::parse::<f64>()`
   - core, not a dependency, and correctly rounded - deleting 45 lines. The
   comment justifying the hand-rolled version ("a scanner should not depend on
   a library float parser") was arguing against a dependency it never had.

   `parse` also reports overflow as infinity rather than as an error, so
   `1e999` would have become a silently infinite literal. Non-finite results
   are refused; the scanner only ever feeds it digits, `.`, and an exponent, so
   non-finite can only mean out of range.

#8 Integer literals overflowed instead of erroring: `18446744073709551616`
   panicked in debug and wrapped in release, and a wrapped literal is a wrong
   program that compiles. dec_value/hex_value are now checked and the literal
   is refused with a message naming it and the limit.

Also gates the five test helpers behind #[cfg(test)] - they are called only
from #[test] functions, so the normal build was compiling them and reporting
each as dead code. Workspace clippy warnings drop from 44 to 36.

Two gaps this opened or exposed are recorded rather than fixed: u128 literals
are now unwritable rather than silently wrapped (#41), and float underflow
still silently yields 0.0 (#42).

Phase: lex. Backlog #7, #8.
Five nodes could not represent what the grammar derives. Nothing constructs
them yet - steps 30-70 will - which is exactly why now is the cheap time.

#20 Decl::ImplDecl dropped the `for` target: it had a slot for the interface's
    generic arguments and none for the type being implemented, so
    60-user-types.md's asserted dump was unproducible. Now a struct variant
    with a `target` field; six positional fields, two of them a Name beside a
    Vec<TypeRef> beside a TypeRef, were too easy to transpose.
#21 No destructuring node - Stmt::VarDecl holds one Name - so `const (x, y) =
    point;` from tuples.cat had nowhere to go. Added Stmt::Destructure over a
    Target enum in which `_` is Discard rather than a Name spelled "_": a
    discard is not a name and nothing downstream should reference it as one.
#22 Expr::Call carried a parallel Vec<bool> of spread flags that dump never
    printed and that could fall out of step with the argument vector, while
    Expr::Spread already existed. Deleted; a spread is one of the arguments.
#23 Expr::ArrayLit dropped the element type while Expr::New kept it, and
    Expr::ArrayInit had no grammar counterpart and dumped identically to Paren.
    Replaced by one ArrayLit over an ArrayLitKind closed set, so "no type and
    no elements" - which the grammar cannot derive - is unrepresentable here.
#24 Interface methods were Decl::FunctionDecl, which requires a Box<Stmt> body,
    so every bodiless signature had to invent an empty block. Added MethodSig
    with a MethodName (ident or operator[]) and no body field.
#36 NewTarget has no size-inferred arm. Fixed in the grammar instead: `new`
    takes `[..]T` or `[N]T`, since `new []T` has no size to allocate from. The
    AST was right and the grammar was too loose.

The dump is the parser's test contract, so it should be the shape the step docs
write. A shared `sexp` builder drops empty parts, which removes the doubled and
trailing spaces the golden strings carried - `(program )`, `(block )`,
`(fun  -> void)`. Two dumps were also plainly wrong and disagreed with their own
documented strings: FieldDecl ran the name into the type (`(field  xfloat)`),
and VarDecl left a space before its annotation.

Adds eight golden tests that build the trees by hand and assert the documented
strings, including the two that could not be produced at all before.

Four step-doc assertions were stale and are corrected: 20-type-refs.md still
described ArrayDyn covering both array forms (left behind by #6), 60-user-types
omitted the impl body that 40-statements dumps as `(block)`, and 50-functions
spelled `->` three different ways.

Phase: parse. Backlog #20-#24, #36; adds #43.
Seven findings, none of which change behaviour except where a silent fallback
becomes a refusal.

#28 Cursor::slice ran `s.to_string()` before indexing, copying the whole source
    file to read one identifier out of it - once per name.
#29 Lexer kept a `Vec<u8>` copy of the entire source alongside the String it
    already owned, built byte-by-byte with push. Replaced by a bytes() view.
#30 Nothing derived Debug, which is why the tests are hand-rolled `if … panic!`
    and why check_kind could only say "token kind mismatch". Debug on every
    token, span, error, and AST node; TokenKind is Copy (every variant already
    was), so Cursor::err stops cloning it to build an error.
#31 Three silent fallbacks. keyword_spell answered "?" for a keyword missing
    from its table - a wrong type name rather than a failure - and it listed
    the same 32 keywords as is_base_kw, which is what let the two drift. Now
    one `base_kw_spelling -> Option<&str>` that is_base_kw's caller asks
    directly. parse_type_named's `_ => KeywordKind::Int` is an `if let`, and
    int_lit's `_ => 0` (a malformed array size became `[0]T`) is an Option.
#32 idx_of_le was a hand-rolled binary search over &Vec<usize>; partition_point
    is one line. text() returns &str.
#35 parse_type_union cloned the leg vector into the node it was building.
#33 The workspace is at zero clippy warnings, from 44.

The hook now gates that. It ran `cargo fmt --check` and nothing else, so
AGENTS.md's step 3 (VERIFY) was a convention rather than a check; it now runs
clippy with -D warnings and the suite too, cheapest first. This changes the
workflow for anyone with core.hooksPath set - it passes clean right now, and
reverting it is a one-file change if the tests get slow enough to be a drag.

Backlog #28-#33, #35.
Resolves the overlap recorded during the grammar pass. Three `declaration` arms
were derivable twice, and the AST carried the same duplication.

- `var-decl` is a statement, and only a statement. A top-level `let` reaches
  `top-level-item` by way of `statement`.
- `lambda-const` is deleted. `const f = (x: int) -> int { ... }` is a var-decl
  whose initializer is a func-value - a primary, hence an expr - so it never
  needed a production. It also gets the spec's overloading rule for free: only
  `function` overloads, and two `const f` bindings are a redeclaration rather
  than two candidates.
- `static-member` stays a declaration, because it is one. The spec gives
  `Vector3::dot = (a, b) -> {...}` as the functional-style spelling of
  `function Vector3::dot(a, b) -> {...}` (Derived > Functions > Static and
  Instance), and `Vector3::dot` is not an lvalue an assignment could target. It
  is told from an expr-stmt by shape - qualified-name, `=`, func-value - and
  builds the same FunctionDecl the C-style spelling does, which is why
  FunctionDecl no longer needs an `is_lambda` flag and there is no
  static-member node.

On the AST side that removes Decl::VarDecl, Decl::Attribute, Stmt::Attribute,
and the is_lambda bool, and adds an `Item` enum mirroring `top-level-item`
(attribute / declaration / statement) as what a Program holds.

An impl body becomes Vec<Item> rather than Box<Stmt>: `impl-member` admits
function declarations, which a Vec<Stmt> block could never have held - the
function declarations are the point of an impl. That also makes an empty body
contribute nothing to the dump, so ImplDecl now produces
`(impl <T> I for (| ([] T) ([..] T)))` - the string 60-user-types.md asserted
all along, and which I had "corrected" to include a `(block)` last pass.

Backlog #37, #43; adds #44 (attributes still float rather than attaching to the
declaration they bind to).
The spec's table grouped `as` into level 3 with the unary prefixes,
right-to-left, which reads as `-x as T` == `-(x as T)`. The grammar bound `as`
looser than a prefix, giving `(-x) as T`. The grammar had the better reading -
it is what Rust, Kotlin, C#, and Swift all do, and a prefix operator being part
of what gets cast is what people write - so the spec is what changed.

Casting is now level 3b, just below the unary prefixes and left-associative.
Numbered 3b rather than 4 so the fifteen levels below it keep the numbers both
documents already reference in prose.

This is not only a tree-shape difference: `-x as u8` and `-(x as u8)` differ in
value whenever the target type is unsigned, which is why it was worth settling
before step 30 builds the expression parser rather than after.

30-expressions.md gains the two cases that tell the readings apart:
`-x as u16` and the chained `x as A as B`.

Backlog #38.
Literal::Int was a u64, so no s128 or u128 value could be written - and after
the overflow check landed those literals went from silently wrapping to being
refused outright, which made the gap real rather than latent. token.rs called
the width "a semantics concern, not a lexer one"; that was defensible while
overflow wrapped and is not now.

The spec's smallest-type rule ("numbers are assumed to be the smallest type
that can store the literal") also needs the literal's real value to classify
it, so the payload has to hold the widest thing anyone can write either way.

Costs nothing in practice: Str(String) is already the widest Literal variant,
so the 16-byte payload grows neither Literal nor Token.

An array size stays u64 - an array indexed beyond u64 is not a thing - so the
narrowing is a checked conversion rather than a cast, reported against the
literal token rather than the cursor, which by then has moved past it and would
have named the `]`.

Phase: lex. Backlog #41.
Overflow is refused and underflow is not, which is asymmetric enough to need
stating. The reason is what a writer can have meant: infinity is never the
value someone intended by a finite string of digits, so accepting it carries a
wrong value into the program, while zero usually is what a tiny literal meant
and refusing it would reject working code. Rust and C draw the line here too.
Subnormals are representable values and are kept as written.

No behaviour change - `1e-999` was already 0.0. What changes is that it is now
a decision: a test pins it so a later reader does not "fix" it into symmetry
with the overflow rule, and the grammar's Lexical rules explain the asymmetry
alongside the new integer limit.

Backlog #42.
contextual-typing.cat ended a file-scope call without a terminator, which
`expr-stmt = expr ";"` cannot derive, and it is a compile-phase fixture so it
has to parse.

It was a typo, and it was a typo in language-spec.md - the fixture was
faithfully copying it. Fixed in both: a fixture-only fix would have diverged
from the source AGENTS.md says to prefer, and whoever next re-derived the
fixture from the spec would have put it back.

Scanned every statement line in the spec's code blocks for the same shape; this
is the only one, so expr-stmt keeps its terminator unqualified rather than
growing an end-of-file or ends-in-a-brace exception.

Backlog #39.
An attribute was a free-floating `Item::Attribute` that bound to whatever
followed it by position, so a `#[...]` at the end of a file, or two in a row
with nothing between, were both representable and meaningless.

`Item::Decl` now carries `attrs: Vec<Attribute>` and `Item::Attribute` is gone;
in the grammar, `attribute` appears only inside `attributed-decl`. An orphaned
attribute is therefore neither derivable nor constructible rather than being
something a later phase has to check for.

The binding is attribute-to-declaration in general. `#[SOA]` on a struct is the
only pair the spec documents today, but nothing in the grammar or the tree
assumes that stays true: any attribute may sit above any declaration, and any
number of them may stack.

`Attribute` is a named type rather than a bare `Name` so its span covers the
whole `#[...]` instead of just the identifier inside it, and so it has somewhere
to grow arguments if the spec ever gives it any.

A declaration with no attributes dumps exactly as itself, so the common case
gains no wrapper; attributes wrap it as `(attr SOA (struct V3B))`, stacking into
one group rather than nesting.

Phase: parse. Backlog #44.
crowmaki deleted branch parser-fixes 2026-08-22 02:22:01 +00:00
crowmaki referenced this pull request from a commit 2026-08-22 02:22:01 +00:00
parser-fixes (#7)
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
crowmaki/catlang!7
No description provided.