From f6c139eda0b68264e5cf5b875536d3406df3871d Mon Sep 17 00:00:00 2001 From: StreamDemon Date: Thu, 2 Jul 2026 14:13:24 +0800 Subject: [PATCH 1/2] Fix pipe-stage ?, impl Trait for Type, trait headers, and dyn type args Four parser bugs found by an empirical review against spec section 16, all in surface the subset contract claims to support: - `x |> f?` parsed as `x |> (f?)` because `|>` was an ordinary binary operator and the `?` postfix bound inside the right operand. Spec sections 5.7 and 16 (pipe_stage) mandate `(x |> f)?` -- the stage's trailing `?` wraps the accumulated pipe application. The RHS of `|>` now goes through a dedicated pipe_stage() production (callee path, optional turbofish, field chain, optional args) and the caller wraps the accumulated pipe in ErrorProp. Closure stages and non-final- segment turbofish remain not-yet-implemented with explicit errors. - `impl Trait for Type` failed with "expected item": impl_block() never handled `for`. It now records an optional TraitRef and parses `where` clauses. - `trait Convert` and `trait Loggable: Printable` failed: trait_def() skipped straight from the name to the body. Generic params, supertrait bounds (parsed structurally, not stored), and `where` clauses now parse. - `&dyn Iter` failed: the dyn branch of ty() ignored type_args. Type::Dyn now carries a TraitRef (grammar's trait_ref = type_path [type_args]). Grammar-correct behavior changes, regression-guarded by tests: `x |> a + b` is now `(x |> a) + b` (a stage is only a callee), and struct literals are no longer valid pipe stages. Adds 15 unit tests (AST-shape assertions for every pipe edge case plus impl/trait/dyn shapes), two corpus fixtures (traits_impls.sp, pipes.sp), and updates the crates/AGENTS.md subset contract for the lines these fixes change. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01H3CinyyqWL6SfCLrKGm3ja --- crates/AGENTS.md | 16 +- crates/sploosh-ast/src/lib.rs | 11 +- crates/sploosh-parser/src/lib.rs | 370 +++++++++++++++++++++++++- crates/sploosh-parser/tests/corpus.rs | 2 + tests/corpus/pipes.sp | 12 + tests/corpus/traits_impls.sp | 52 ++++ 6 files changed, 454 insertions(+), 9 deletions(-) create mode 100644 tests/corpus/pipes.sp create mode 100644 tests/corpus/traits_impls.sp diff --git a/crates/AGENTS.md b/crates/AGENTS.md index bbe9a96..c496b59 100644 --- a/crates/AGENTS.md +++ b/crates/AGENTS.md @@ -22,11 +22,16 @@ The parser covers a subset of §16, not the whole grammar. Treat anything outsid the accepted list as **not yet implemented** — add a corpus fixture when it lands. - **Accepted:** every item form (`fn`, `struct`, `enum` incl. tuple/struct - variants, `actor`, `mod`, `use`, `const`, `type`, `trait`, `impl`, + variants, `actor`, `mod`, `use`, `const`, `type`, `trait` incl. generics, + supertrait bounds, and `where` clauses (bounds parsed but not stored), + `impl Type` and `impl Trait for Type` (trait ref recorded in the AST), `onchain mod`, `extern`); types incl. generics, references, arrays/slices, - tuples, `fn`/`dyn`; expressions incl. calls, turbofish, field/index, - unary/binary (incl. `/`), `?`, `.await`, `|>`, `as`, struct literals (with the - §5.1 block-head restriction), `vec!`, and `if`/`else`; `let`, `return`, + tuples, `fn`, and `dyn Trait` incl. associated-type bindings; + expressions incl. calls, turbofish, field/index, + unary/binary (incl. `/`), `?`, `.await`, `as`, struct literals (with the + §5.1 block-head restriction), `vec!`, and `if`/`else`; `|>` with §16 + `pipe_stage` stages (`callee [args] [?]` — a stage's trailing `?` wraps the + accumulated pipe application per §5.7); `let`, `return`, `break`, `continue`, `send`, and expression/tail statements. - **Not yet implemented:** `match`, `while`, `for`, `loop`, `if let`, and closures (their keywords lex but have no parse production); `spawn`, `select`, @@ -36,7 +41,8 @@ the accepted list as **not yet implemented** — add a corpus fixture when it la `let Some(x) = ...` are rejected); `#[...]` compiler directives are not parsed in any position (`#[cfg(test)]`, `#[target(...)]`, `#[indexed]` all fail); `storage { }` blocks inside `onchain mod` are consumed but discarded, not - stored in the AST; generic parameters and + stored in the AST; closure pipe stages (`x |> (|v| ...)`) and turbofish on a + non-final pipe-stage segment; generic parameters and `trait`/`impl` bodies are skipped, not stored in the AST; `let mut` / `&mut` mutability and the `send` keyword are parsed but not preserved; a block-like expression (`if`/block) used as a non-tail statement needs a trailing `;`. diff --git a/crates/sploosh-ast/src/lib.rs b/crates/sploosh-ast/src/lib.rs index f3723e4..cbbba7c 100644 --- a/crates/sploosh-ast/src/lib.rs +++ b/crates/sploosh-ast/src/lib.rs @@ -171,6 +171,8 @@ pub struct Trait { #[derive(Debug, Clone, PartialEq)] pub struct ImplBlock { + /// `Some` for `impl Trait for Type`, `None` for an inherent `impl Type`. + pub trait_ref: Option, pub target: Type, } @@ -195,7 +197,14 @@ pub enum Type { Slice(Box), Tuple(Vec), Function { params: Vec, ret: Box }, - Dyn(Path), + Dyn(TraitRef), +} + +/// `trait_ref = type_path [ type_args ]` (§16). +#[derive(Debug, Clone, PartialEq)] +pub struct TraitRef { + pub path: Path, + pub args: Vec, } #[derive(Debug, Clone, PartialEq)] diff --git a/crates/sploosh-parser/src/lib.rs b/crates/sploosh-parser/src/lib.rs index 6d83a3f..7f5dcc7 100644 --- a/crates/sploosh-parser/src/lib.rs +++ b/crates/sploosh-parser/src/lib.rs @@ -370,16 +370,52 @@ impl Parser { fn trait_def(&mut self) -> Option { self.expect_keyword(Keyword::Trait)?; let name = self.ident()?; + self.maybe_generic_params(); + if self.eat(TokenKind::Colon).is_some() { + self.bounds(); + } + self.maybe_where_clause(); self.skip_item_body(); Some(Trait { name }) } + /// `bounds = bound { "+" bound }` with `bound = trait_ref | lifetime` (§16). + /// Parsed for validation and diagnostics; not stored during bootstrap. + fn bounds(&mut self) { + loop { + if self.eat(TokenKind::Lifetime).is_none() && self.trait_ref().is_none() { + self.recover_until(&[TokenKind::LBrace, TokenKind::Semi]); + return; + } + if self.eat(TokenKind::Plus).is_none() { + break; + } + } + } + + /// `trait_ref = type_path [ type_args ]` (§16). + fn trait_ref(&mut self) -> Option { + let path = self.path()?; + let args = self.type_args(); + Some(TraitRef { path, args }) + } + fn impl_block(&mut self) -> Option { self.expect_keyword(Keyword::Impl)?; self.maybe_generic_params(); - let target = self.ty()?; + let first = self.ty()?; + let (trait_ref, target) = if self.eat_keyword(Keyword::For).is_some() { + let Type::Path { path, args } = first else { + self.error_here("expected trait path before `for`"); + return None; + }; + (Some(TraitRef { path, args }), self.ty()?) + } else { + (None, first) + }; + self.maybe_where_clause(); self.skip_item_body(); - Some(ImplBlock { target }) + Some(ImplBlock { trait_ref, target }) } fn onchain_item(&mut self) -> Option { @@ -487,7 +523,7 @@ impl Parser { }); } if self.eat_ident_text("dyn").is_some() { - return Some(Type::Dyn(self.path()?)); + return Some(Type::Dyn(self.trait_ref()?)); } let path = self.path()?; let args = self.type_args(); @@ -656,6 +692,30 @@ impl Parser { break; } let op_text = self.bump().lexeme; + if op == "|>" { + // §16: the RHS of `|>` is a `pipe_stage`, not a precedence-climbed + // expression. + let stage = self.pipe_stage()?; + let span = lhs.span.join(stage.span); + lhs = Expr { + kind: ExprKind::Binary { + op: op_text, + left: Box::new(lhs), + right: Box::new(stage), + }, + span, + }; + // §5.7: a stage's trailing `?` applies to the accumulated pipe + // application result — `x |> f?` is `(x |> f)?`. + if self.eat(TokenKind::Question).is_some() { + let span = lhs.span.join(self.prev_span()); + lhs = Expr { + kind: ExprKind::ErrorProp(Box::new(lhs)), + span, + }; + } + continue; + } let rhs = self.expr(right_bp)?; let span = lhs.span.join(rhs.span); lhs = if op == "=" { @@ -819,6 +879,73 @@ impl Parser { } } + /// `pipe_stage = stage_callee [ "(" args ")" ]` with + /// `stage_callee = path_expr [ turbofish ] { "." IDENT [ turbofish ] }` (§16). + /// The stage-trailing `?` is consumed by the caller so it can wrap the + /// accumulated pipe application (§5.7). The `"(" closure ")"` stage form is + /// not yet implemented — closures have no parse production. + fn pipe_stage(&mut self) -> Option { + if self.at(TokenKind::LParen) { + self.error_here("closure pipe stages are not yet implemented"); + return None; + } + if !matches!( + self.peek_kind(), + Some(TokenKind::Ident | TokenKind::Keyword(Keyword::SelfValue | Keyword::SelfType)) + ) { + self.error_here("expected pipe stage: a function path or method chain"); + return None; + } + let path = self.path()?; + let mut callee = Expr { + span: path.span, + kind: ExprKind::Path(path), + }; + let mut type_args = self.turbofish(); + while type_args.is_none() && self.eat(TokenKind::Dot).is_some() { + let name = self.ident()?.name; + let span = callee.span.join(self.prev_span()); + callee = Expr { + kind: ExprKind::Field { + base: Box::new(callee), + name, + }, + span, + }; + type_args = self.turbofish(); + } + if type_args.is_some() && self.at(TokenKind::Dot) { + self.error_here("turbofish on a non-final pipe-stage segment is not yet implemented"); + return None; + } + if self.eat(TokenKind::LParen).is_some() { + let args = self.args(TokenKind::RParen)?; + let end = self.expect(TokenKind::RParen)?.span.end; + let span = Span::new(callee.span.start, end); + callee = Expr { + kind: ExprKind::Call { + callee: Box::new(callee), + type_args: type_args.unwrap_or_default(), + args, + }, + span, + }; + } else if let Some(type_args) = type_args { + // `x |> parse::` has no parens; keep the type args on a zero-arg + // call — §5.6 desugaring makes this identical to `x |> parse::()`. + let span = callee.span.join(self.prev_span()); + callee = Expr { + kind: ExprKind::Call { + callee: Box::new(callee), + type_args, + args: Vec::new(), + }, + span, + }; + } + Some(callee) + } + fn if_expr(&mut self) -> Option { let start = self.expect_keyword(Keyword::If)?.span.start; let condition = self.cond_expr()?; @@ -1332,4 +1459,241 @@ mod tests { }; assert_eq!(fields.len(), 1); } + + /// Parses `fn f() { let r = ; }` and returns the bound expression. + fn let_value(expr_src: &str) -> Expr { + let source = format!("fn f() {{ let r = {expr_src}; }}"); + let program = parse_program(&source).unwrap(); + let ItemKind::Function(func) = &program.items[0].kind else { + panic!("expected function"); + }; + let Stmt::Let { value, .. } = &func.body.as_ref().unwrap().statements[0] else { + panic!("expected let statement"); + }; + value.clone() + } + + fn path_named(expr: &Expr, name: &str) -> bool { + matches!(&expr.kind, ExprKind::Path(path) if path.segments == [name]) + } + + #[test] + fn pipe_stage_question_wraps_accumulated_pipe() { + // §5.7: `input |> parse?` is `(input |> parse)?`, never `input |> (parse?)`. + let value = let_value("input |> parse?"); + let ExprKind::ErrorProp(inner) = &value.kind else { + panic!("expected ErrorProp at the top, got {value:?}"); + }; + let ExprKind::Binary { op, left, right } = &inner.kind else { + panic!("expected pipe binary inside ErrorProp"); + }; + assert_eq!(op, "|>"); + assert!(path_named(left, "input")); + assert!(path_named(right, "parse")); + } + + #[test] + fn pipe_chain_question_on_each_stage() { + // `a |> f? |> g?` nests as ErrorProp(Binary(ErrorProp(Binary(a, f)), g)). + let value = let_value("a |> f? |> g?"); + let ExprKind::ErrorProp(outer) = &value.kind else { + panic!("expected outer ErrorProp"); + }; + let ExprKind::Binary { op, left, right } = &outer.kind else { + panic!("expected outer pipe binary"); + }; + assert_eq!(op, "|>"); + assert!(path_named(right, "g")); + let ExprKind::ErrorProp(mid) = &left.kind else { + panic!("expected inner ErrorProp"); + }; + let ExprKind::Binary { op, left, right } = &mid.kind else { + panic!("expected inner pipe binary"); + }; + assert_eq!(op, "|>"); + assert!(path_named(left, "a")); + assert!(path_named(right, "f")); + } + + #[test] + fn pipe_stage_with_args_keeps_question_on_pipe() { + let value = let_value("x |> f(a)?"); + let ExprKind::ErrorProp(inner) = &value.kind else { + panic!("expected ErrorProp at the top"); + }; + let ExprKind::Binary { op, right, .. } = &inner.kind else { + panic!("expected pipe binary"); + }; + assert_eq!(op, "|>"); + let ExprKind::Call { callee, args, .. } = &right.kind else { + panic!("expected call stage"); + }; + assert!(path_named(callee, "f")); + assert_eq!(args.len(), 1); + } + + #[test] + fn pipe_stage_method_chain_is_stage_callee() { + // The field chain belongs to the stage, not to the pipe expression. + let value = let_value("x |> svc.parse?"); + let ExprKind::ErrorProp(inner) = &value.kind else { + panic!("expected ErrorProp at the top"); + }; + let ExprKind::Binary { op, right, .. } = &inner.kind else { + panic!("expected pipe binary"); + }; + assert_eq!(op, "|>"); + let ExprKind::Field { base, name } = &right.kind else { + panic!("expected field-chain stage"); + }; + assert_eq!(name, "parse"); + assert!(path_named(base, "svc")); + } + + #[test] + fn pipe_stage_turbofish_without_parens() { + // `x |> parse::` desugars like `x |> parse::()`. + let value = let_value("x |> parse::?"); + let ExprKind::ErrorProp(inner) = &value.kind else { + panic!("expected ErrorProp at the top"); + }; + let ExprKind::Binary { right, .. } = &inner.kind else { + panic!("expected pipe binary"); + }; + let ExprKind::Call { + callee, + type_args, + args, + } = &right.kind + else { + panic!("expected zero-arg call stage"); + }; + assert!(path_named(callee, "parse")); + assert_eq!(type_args.len(), 1); + assert!(args.is_empty()); + } + + #[test] + fn pipe_stage_is_callee_not_arithmetic() { + // A stage is only a callee: `x |> a + b` is `(x |> a) + b`. + let value = let_value("x |> a + b"); + let ExprKind::Binary { op, left, right } = &value.kind else { + panic!("expected `+` at the top"); + }; + assert_eq!(op, "+"); + assert!(path_named(right, "b")); + let ExprKind::Binary { op, left, right } = &left.kind else { + panic!("expected pipe binary on the left"); + }; + assert_eq!(op, "|>"); + assert!(path_named(left, "x")); + assert!(path_named(right, "a")); + } + + #[test] + fn pipe_rejects_non_callee_stage() { + let errors = parse_program("fn f() { let r = x |> 42; }").unwrap_err(); + assert!(errors.iter().any(|err| err.message.contains("pipe stage"))); + } + + #[test] + fn pipe_rejects_closure_stage_for_now() { + let errors = parse_program("fn f() { let r = x |> (v); }").unwrap_err(); + assert!( + errors + .iter() + .any(|err| err.message.contains("not yet implemented")) + ); + } + + #[test] + fn question_outside_pipe_unchanged() { + let value = let_value("fetch()?"); + let ExprKind::ErrorProp(inner) = &value.kind else { + panic!("expected ErrorProp"); + }; + assert!(matches!(&inner.kind, ExprKind::Call { .. })); + } + + #[test] + fn impl_trait_for_type_records_trait_ref() { + let program = parse_program("impl Display for User {}").unwrap(); + let ItemKind::Impl(imp) = &program.items[0].kind else { + panic!("expected impl"); + }; + let trait_ref = imp.trait_ref.as_ref().expect("trait ref recorded"); + assert_eq!(trait_ref.path.segments, ["Display"]); + assert!(trait_ref.args.is_empty()); + assert!(matches!( + &imp.target, + Type::Path { path, .. } if path.segments == ["User"] + )); + } + + #[test] + fn inherent_impl_has_no_trait_ref() { + let program = parse_program("impl User {}").unwrap(); + let ItemKind::Impl(imp) = &program.items[0].kind else { + panic!("expected impl"); + }; + assert!(imp.trait_ref.is_none()); + } + + #[test] + fn impl_generic_trait_with_where_clause_parses() { + let source = "impl Convert for Wrapper where T: Clone {}"; + let program = parse_program(source).unwrap(); + let ItemKind::Impl(imp) = &program.items[0].kind else { + panic!("expected impl"); + }; + let trait_ref = imp.trait_ref.as_ref().expect("trait ref recorded"); + assert_eq!(trait_ref.path.segments, ["Convert"]); + assert_eq!(trait_ref.args.len(), 1); + } + + #[test] + fn impl_rejects_non_path_trait_before_for() { + let errors = parse_program("impl &Foo for Bar {}").unwrap_err(); + assert!(errors.iter().any(|err| err.message.contains("trait path"))); + } + + #[test] + fn trait_with_generics_supertraits_and_where_parses() { + let source = r#" + trait Convert {} + trait Loggable: Printable {} + trait Bounded: Convert + Printable + 'static where Self: Printable {} + "#; + let program = parse_program(source).unwrap(); + let names: Vec<_> = program + .items + .iter() + .map(|item| { + let ItemKind::Trait(tr) = &item.kind else { + panic!("expected trait"); + }; + tr.name.name.as_str() + }) + .collect(); + assert_eq!(names, ["Convert", "Loggable", "Bounded"]); + } + + #[test] + fn dyn_trait_type_args_with_assoc_binding() { + let program = parse_program("fn f(it: &dyn Iter) {}").unwrap(); + let ItemKind::Function(func) = &program.items[0].kind else { + panic!("expected function"); + }; + let Param::Named { ty, .. } = &func.params[0] else { + panic!("expected named param"); + }; + let Type::Reference { inner, .. } = ty else { + panic!("expected reference type"); + }; + let Type::Dyn(trait_ref) = inner.as_ref() else { + panic!("expected dyn type"); + }; + assert_eq!(trait_ref.path.segments, ["Iter"]); + assert!(matches!(&trait_ref.args[0], TypeArg::Assoc { name, .. } if name.name == "Item")); + } } diff --git a/crates/sploosh-parser/tests/corpus.rs b/crates/sploosh-parser/tests/corpus.rs index f40dc58..3b04d3f 100644 --- a/crates/sploosh-parser/tests/corpus.rs +++ b/crates/sploosh-parser/tests/corpus.rs @@ -6,6 +6,8 @@ fn parses_corpus_files() { "tests/corpus/basic.sp", "tests/corpus/actor.sp", "tests/corpus/control_flow.sp", + "tests/corpus/traits_impls.sp", + "tests/corpus/pipes.sp", ] { let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) .join("../..") diff --git a/tests/corpus/pipes.sp b/tests/corpus/pipes.sp new file mode 100644 index 0000000..d39ce22 --- /dev/null +++ b/tests/corpus/pipes.sp @@ -0,0 +1,12 @@ +/// Pipe stages per §16 pipe_stage: bare callee, call args, method chains, +/// turbofish, and the stage-trailing `?` that wraps the accumulated pipe +/// application (§5.7 — `x |> f?` is `(x |> f)?`). +fn pipeline(input: String, checker: Checker) -> Result { + let trimmed = input |> trim; + let n = trimmed |> parse::?; + let m = n |> add(5) |> add(20); + let v = m |> checker.validate(3)?; + let chained = v |> step_one? |> step_two?; + let arith = (chained |> double) + offset(1); + arith |> finish +} diff --git a/tests/corpus/traits_impls.sp b/tests/corpus/traits_impls.sp new file mode 100644 index 0000000..d3c8be8 --- /dev/null +++ b/tests/corpus/traits_impls.sp @@ -0,0 +1,52 @@ +/// Trait and impl shapes: generics, supertraits, where clauses, trait impls, +/// and `dyn` trait references with type arguments (§16 trait_def, impl_block, +/// trait_ref). +trait Printable { + fn print(&self); +} + +trait Convert { + fn convert(&self) -> T; +} + +trait Loggable: Printable { + fn log(&self); +} + +trait Bounded: Convert + Printable where Self: Printable { + fn cap(&self) -> i64; +} + +struct User { + id: u64, +} + +struct Wrapper { + value: T, +} + +impl User { + fn id(&self) -> u64 { + self.id + } +} + +impl Printable for User { + fn print(&self) {} +} + +impl Convert for Wrapper where T: Clone { + fn convert(&self) -> T { + self.value + } +} + +impl Loggable for User where User: Printable { + fn log(&self) {} +} + +struct Holder { + it: Box>, +} + +fn takes(c: &dyn Convert) {} From 1c100cbe0ab2f2af432ce617d3eb77dd2893f5ce Mon Sep 17 00:00:00 2001 From: StreamDemon Date: Thu, 2 Jul 2026 15:01:25 +0800 Subject: [PATCH 2/2] Add predicate to the pipe-stage not-yet-implemented list item MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cubic review on PR #70: the new list entry was the only clause in the not-yet-implemented list without a verb. It now states what the parser actually does — rejects both shapes with explicit not-yet-implemented parse errors. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01H3CinyyqWL6SfCLrKGm3ja --- crates/AGENTS.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/AGENTS.md b/crates/AGENTS.md index c496b59..96e5232 100644 --- a/crates/AGENTS.md +++ b/crates/AGENTS.md @@ -42,7 +42,8 @@ the accepted list as **not yet implemented** — add a corpus fixture when it la in any position (`#[cfg(test)]`, `#[target(...)]`, `#[indexed]` all fail); `storage { }` blocks inside `onchain mod` are consumed but discarded, not stored in the AST; closure pipe stages (`x |> (|v| ...)`) and turbofish on a - non-final pipe-stage segment; generic parameters and + non-final pipe-stage segment are rejected with explicit + not-yet-implemented parse errors; generic parameters and `trait`/`impl` bodies are skipped, not stored in the AST; `let mut` / `&mut` mutability and the `send` keyword are parsed but not preserved; a block-like expression (`if`/block) used as a non-tail statement needs a trailing `;`.