From 87c5e4376ad05926ef4ae3f28be4c1405d35c051 Mon Sep 17 00:00:00 2001 From: hyperpolymath Date: Sun, 17 May 2026 03:13:30 +0100 Subject: [PATCH] fix(checker): general non-overflowing AST teardown + measured MAX_EXPR_DEPTH (#37) Closes the Linux-side of hyperpolymath/my-lang#37 (subtleties 1 & 3). 1. General iterative AST teardown (subtlety 1) The recursive-`Drop` stack overflow is shape-independent; the old test-only `drop_program_iteratively` only flattened 2-arg `Call` chains. Replaced with `ast::drop_program_iteratively`: pops owned nodes from an explicit heap worklist and destructures them by value (legal because `Expr` has no `Drop` impl), moving every recursive child into the worklist before the shallow remainder drops. O(n) time, O(n) heap, O(1) stack, zero shape assumptions. Covers every recursive edge (Box, Vec, Block/Stmt, Match/Record/ Lambda/Ai). Measured: tears down a 1,000,000-deep non-Call chain on a 512 KiB stack (recursive Drop cliffs at ~5k). `impl Drop for Expr` was deliberately rejected: it would forbid by-value destructuring of `Expr` across the parser/HIR/MIR crates ("cannot move out of a type which implements Drop"). Documented. 2. MAX_EXPR_DEPTH re-derived from a measured budget (subtlety 3) `examples/measure_depth.rs` probes one `check_expr` walk on a fixed-size thread stack: ~4.4 KiB/level. The checker is never dispatched onto a large-stack thread, so the binding constraint is the OS main thread = 1 MiB on Windows. The old 256 needs ~1.14 MiB -- it could overflow *before* the guard fires on Windows (not just "unjustified" as the issue said, but unsafe there). Re-derived to 128 (~569 KiB, ~54% of a 1 MiB Windows stack). The parser's independent MAX_PARSE_EXPR_DEPTH=64 is the real operational ceiling, so no parseable program reaches the guard and lowering it rejects zero real code. Stale "quarter of 256" parser docs corrected. 3. Regression test (DoD item 4) `test_deep_non_call_ast_teardown_does_not_overflow`: a 50,000-deep `Unary` chain (no `Call`) -- the exact shape the old helper could not handle. `checker_alloc_scaling` depth ladder made constant- relative so it can't silently exceed a re-tuned guard. Still open in #37: the Windows-CI half of the stack measurement (needs CI access; the Linux measurement + harness land here). Full `cargo test -p my-lang` green (137 unit + 3 alloc-scaling + rest). Co-Authored-By: Claude Opus 4.7 --- crates/my-lang/examples/measure_depth.rs | 83 +++++++++++ crates/my-lang/src/ast.rs | 137 ++++++++++++++++++ crates/my-lang/src/checker.rs | 118 ++++++++++----- crates/my-lang/src/parser.rs | 19 ++- crates/my-lang/tests/checker_alloc_scaling.rs | 10 +- 5 files changed, 318 insertions(+), 49 deletions(-) create mode 100644 crates/my-lang/examples/measure_depth.rs diff --git a/crates/my-lang/examples/measure_depth.rs b/crates/my-lang/examples/measure_depth.rs new file mode 100644 index 0000000..60159d2 --- /dev/null +++ b/crates/my-lang/examples/measure_depth.rs @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: PMPL-1.0-or-later +//! Stack-budget probe for hyperpolymath/my-lang#37. +//! +//! Builds a deep, *non-`Call`-shaped* AST (a `Unary::Not` chain — the shape +//! the old test helper could not handle) and exercises one recursive walk on +//! a thread with a known, fixed stack size. The process aborts on stack +//! overflow, so a wrapper runs this at increasing depths and reads the exit +//! status to find the overflow cliff. From `cliff_depth` at a known +//! `stack_bytes` we get `bytes_per_level ≈ stack_bytes / cliff_depth`. +//! +//! Usage: measure_depth + +use my_lang::ast::*; +use my_lang::token::Span; + +fn build_unary_chain(depth: usize) -> Expr { + let span = Span::default(); + let mut e = Expr::Literal(Literal::Bool(true, span)); + for _ in 0..depth { + e = Expr::Unary { + op: UnaryOp::Not, + operand: Box::new(e), + span, + }; + } + e +} + +fn wrap(value: Expr) -> Program { + let span = Span::default(); + Program { + items: vec![TopLevel::Function(FnDecl { + modifiers: vec![], + name: Ident::new("main", span), + params: vec![], + return_type: None, + contract: None, + body: Block { + stmts: vec![Stmt::Let { + mutable: false, + name: Ident::new("s", span), + ty: None, + value, + span, + }], + span, + }, + span, + })], + } +} + +fn main() { + let a: Vec = std::env::args().collect(); + let mode = a.get(1).cloned().unwrap_or_default(); + let depth: usize = a.get(2).and_then(|s| s.parse().ok()).unwrap_or(1000); + let stack_kib: usize = a.get(3).and_then(|s| s.parse().ok()).unwrap_or(1024); + + let h = std::thread::Builder::new() + .stack_size(stack_kib * 1024) + .spawn(move || { + let program = wrap(build_unary_chain(depth)); + match mode.as_str() { + "check" => { + // check_expr recursion (the guard is what #37 re-derives). + let _ = my_lang::check(&program); + my_lang::ast::drop_program_iteratively(program); + } + "drop_recursive" => { + // The unguarded danger: auto-derived recursive Drop. + drop(program); + } + "drop_iter" => { + // The #37 fix: general iterative teardown. + my_lang::ast::drop_program_iteratively(program); + } + _ => eprintln!("unknown mode"), + } + println!("OK depth={depth} stack_kib={stack_kib} mode={mode}"); + }) + .unwrap(); + h.join().unwrap(); +} diff --git a/crates/my-lang/src/ast.rs b/crates/my-lang/src/ast.rs index d2b735f..bb5b5ae 100644 --- a/crates/my-lang/src/ast.rs +++ b/crates/my-lang/src/ast.rs @@ -694,3 +694,140 @@ impl Literal { } } } + +// ============================================ +// Non-recursive AST teardown (hyperpolymath/my-lang#37) +// ============================================ + +/// One pending node in the iterative teardown worklist. +enum DropNode { + Expr(Expr), + Block(Block), +} + +/// Drop an entire [`Program`] without recursing through the AST. +/// +/// # Why this exists +/// +/// `Expr` is a tree of `Box` (plus `Block`/`Stmt`/`Match`/… edges). +/// The compiler-derived `Drop` recurses once per nesting level, so a +/// sufficiently deep AST overflows the thread stack *on drop* — independently +/// of the type checker, and for **any** shape, not just the `Call` chains the +/// former test-only `drop_program_iteratively` helper special-cased +/// (hyperpolymath/my-lang#37, subtlety 1). +/// +/// Implementing `Drop for Expr` would be the other option, but it is rejected +/// deliberately: a `Drop` impl forbids by-value destructuring of `Expr` +/// (`cannot move out of a type which implements Drop`), which the parser / +/// HIR / MIR crates rely on pervasively. A standalone teardown keeps the AST a +/// plain movable tree while still giving any owner a non-overflowing drop. +/// +/// # How it works +/// +/// Owned nodes are pushed onto an explicit heap worklist. Each node is popped +/// **by value** and destructured (legal precisely because `Expr` has no `Drop` +/// impl); every recursive child is moved into the worklist *before* the +/// shallow remainder of the node goes out of scope. The derived drop therefore +/// only ever runs at depth 1. O(n) time, O(n) heap, **O(1) stack** — no +/// recursion and no assumptions about AST shape. +pub fn drop_program_iteratively(mut program: Program) { + let mut work: Vec = Vec::new(); + + // Seed from function bodies — the only place unbounded nesting can arise. + // Every other `TopLevel` is a shallow declaration and drops at O(1) depth. + for item in program.items.drain(..) { + if let TopLevel::Function(f) = item { + work.push(DropNode::Block(f.body)); + } + } + + while let Some(node) = work.pop() { + match node { + DropNode::Expr(e) => detach_expr_children(e, &mut work), + DropNode::Block(b) => detach_block_children(b, &mut work), + } + // `node` is fully consumed above; its non-recursive remainder (spans, + // operators, identifiers, literals) drops here at depth 1. + } +} + +fn detach_expr_children(e: Expr, work: &mut Vec) { + match e { + Expr::Call { callee, args, .. } => { + work.push(DropNode::Expr(*callee)); + work.extend(args.into_iter().map(DropNode::Expr)); + } + Expr::Field { object, .. } => work.push(DropNode::Expr(*object)), + Expr::Binary { left, right, .. } => { + work.push(DropNode::Expr(*left)); + work.push(DropNode::Expr(*right)); + } + Expr::Unary { operand, .. } + | Expr::Try { operand, .. } + | Expr::Restrict { operand, .. } => work.push(DropNode::Expr(*operand)), + Expr::Block(b) => work.push(DropNode::Block(b)), + Expr::Ai(ai) => detach_ai_expr(ai, work), + Expr::Lambda { body, .. } => match body { + LambdaBody::Expr(b) => work.push(DropNode::Expr(*b)), + LambdaBody::Block(bl) => work.push(DropNode::Block(bl)), + }, + Expr::Match { scrutinee, arms, .. } => { + work.push(DropNode::Expr(*scrutinee)); + work.extend(arms.into_iter().map(|a| DropNode::Expr(a.body))); + } + Expr::Array { elements, .. } => { + work.extend(elements.into_iter().map(DropNode::Expr)); + } + Expr::Record { fields, .. } => { + work.extend(fields.into_iter().map(|f| DropNode::Expr(f.value))); + } + Expr::Literal(_) | Expr::Ident(_) => {} + } +} + +fn detach_ai_expr(ai: AiExpr, work: &mut Vec) { + match ai { + AiExpr::Block { body, .. } => { + for item in body { + if let AiBodyItem::Field { value, .. } = item { + work.push(DropNode::Expr(value)); + } + } + } + AiExpr::Call { args, .. } | AiExpr::PromptInvocation { args, .. } => { + work.extend(args.into_iter().map(DropNode::Expr)); + } + AiExpr::Quick { .. } => {} + } +} + +fn detach_block_children(b: Block, work: &mut Vec) { + for stmt in b.stmts { + match stmt { + Stmt::Expr(e) + | Stmt::Let { value: e, .. } + | Stmt::Await { value: e, .. } + | Stmt::Try { value: e, .. } => work.push(DropNode::Expr(e)), + Stmt::If { condition, then_block, else_block, .. } => { + work.push(DropNode::Expr(condition)); + work.push(DropNode::Block(then_block)); + if let Some(eb) = else_block { + work.push(DropNode::Block(eb)); + } + } + Stmt::Go { block, .. } | Stmt::Comptime { block, .. } => { + work.push(DropNode::Block(block)) + } + Stmt::Return { value, .. } => { + if let Some(v) = value { + work.push(DropNode::Expr(v)); + } + } + Stmt::Ai(ai) => match ai.body { + AiStmtBody::Block(bl) => work.push(DropNode::Block(bl)), + AiStmtBody::Expr(e) => work.push(DropNode::Expr(*e)), + }, + Stmt::Belief { .. } => {} + } + } +} diff --git a/crates/my-lang/src/checker.rs b/crates/my-lang/src/checker.rs index b6bc632..2a53f88 100644 --- a/crates/my-lang/src/checker.rs +++ b/crates/my-lang/src/checker.rs @@ -118,12 +118,30 @@ pub enum CheckError { /// `check_expr` / `is_assignable_from` to be *linear* in AST size on Linux /// and on a Windows CI leg — there is no super-linear allocation for the /// guard to defend against. What deep nesting *does* threaten is the -/// recursive descent through `check_expr` (and the recursive `Drop` of the -/// resulting AST). The original #1 OOM was shown not to be checker -/// algorithmic complexity; this limit therefore exists to keep recursion -/// depth finite, and rejecting deep-but-legal programs at 256 is its cost, -/// not a memory safeguard. Re-tuning the value is tracked separately. -pub const MAX_EXPR_DEPTH: usize = 256; +/// recursive descent through `check_expr`. (The recursive `Drop` of a deep +/// AST is a *separate* overflow; it is now handled structurally and +/// shape-independently by [`crate::ast::drop_program_iteratively`], so it no +/// longer constrains this value — hyperpolymath/my-lang#37.) +/// +/// # Re-derived from a measured stack budget (my-lang#37) +/// +/// The previous value (256) was an inherited guess, not a budget. Probing one +/// recursive `check_expr` walk on a fixed-size thread stack +/// (`examples/measure_depth.rs`) measures **≈4.4 KiB of stack per nesting +/// level**. The binding constraint is the smallest stack the checker runs on: +/// the OS main thread (the checker is *not* dispatched onto a large-stack +/// thread anywhere), i.e. **1 MiB on Windows**. At 256 levels that walk needs +/// ≈1.14 MiB — it can overflow *before* the guard fires on Windows, so 256 was +/// not merely unjustified but unsafe there. +/// +/// `128 × 4.4 KiB ≈ 569 KiB ≈ 54%` of a 1 MiB Windows stack, leaving ~46% +/// headroom for the rest of the call graph above `check_expr`; vastly safe on +/// Linux (8 MiB) and Rust's 2 MiB default threads. It is still 2× the parser's +/// independently-derived [`crate::parser::MAX_PARSE_EXPR_DEPTH`] (64) ceiling, +/// so — because no *parseable* program nests deeper than 64 — lowering it +/// rejects zero real programs; it only ever fires on programmatically-built +/// ASTs, which is its sole remaining purpose. +pub const MAX_EXPR_DEPTH: usize = 128; pub type CheckResult = Result; @@ -1450,40 +1468,10 @@ mod tests { check(&program) } - /// Iteratively flattens any `str_concat(_, inner)`-style right-recursive - /// AST chains inside `program` so that the auto-derived recursive `Drop` - /// for `Box` does not overflow the test thread's stack. Only the - /// shapes we construct in the deep-nesting tests are handled. - fn drop_program_iteratively(mut program: Program) { - use std::mem; - for item in program.items.iter_mut() { - if let TopLevel::Function(f) = item { - for stmt in f.body.stmts.iter_mut() { - if let Stmt::Let { value, .. } = stmt { - flatten_call_chain(value); - } - } - } - } - - fn flatten_call_chain(expr: &mut Expr) { - // Repeatedly peel the deepest argument out of a Call and let it - // be dropped on its own once we've broken the chain. - loop { - let next = match expr { - Expr::Call { args, .. } if args.len() == 2 => { - // Replace the recursive arg with a tiny placeholder - // and take ownership of the giant subtree. - let placeholder = - Expr::Literal(Literal::Bool(false, crate::token::Span::default())); - mem::replace(&mut args[1], placeholder) - } - _ => return, - }; - *expr = next; - } - } - } + // The former shape-specific `drop_program_iteratively` test helper (it + // only flattened 2-arg `Call` chains) is replaced by the general, + // shape-independent `crate::ast::drop_program_iteratively`, already in + // scope here via `super::*` (hyperpolymath/my-lang#37). #[test] fn test_basic_function() { @@ -1644,6 +1632,56 @@ mod tests { drop_program_iteratively(program); } + #[test] + fn test_deep_non_call_ast_teardown_does_not_overflow() { + // Regression for hyperpolymath/my-lang#37, subtlety 1: the recursive + // `Drop` overflow is *shape-independent*. The former test helper only + // flattened 2-arg `Call` chains; a differently-shaped deep AST (here a + // `Unary::Not` chain, with no `Call` anywhere) would still overflow. + // The general `ast::drop_program_iteratively` must tear it down with + // O(1) stack regardless of shape. + // + // Depth is far beyond the measured recursive-Drop cliff (≈4–6k at a + // 512 KiB stack) so a recursion-based teardown would abort the test + // process; survival proves the teardown is non-recursive. + use crate::token::Span; + + let span = Span::default(); + let mut expr = Expr::Literal(Literal::Bool(true, span)); + for _ in 0..50_000 { + expr = Expr::Unary { + op: UnaryOp::Not, + operand: Box::new(expr), + span, + }; + } + + let program = Program { + items: vec![TopLevel::Function(FnDecl { + modifiers: vec![], + name: Ident::new("main", span), + params: vec![], + return_type: None, + contract: None, + body: Block { + stmts: vec![Stmt::Let { + mutable: false, + name: Ident::new("s", span), + ty: None, + value: expr, + span, + }], + span, + }, + span, + })], + }; + + // The sole assertion is that this returns at all: a recursive teardown + // would `STATUS_STACK_OVERFLOW` / SIGABRT the test runner first. + drop_program_iteratively(program); + } + #[test] fn test_moderately_nested_str_concat_still_checks() { // Sanity: well below the limit, deeply chained str_concat must still diff --git a/crates/my-lang/src/parser.rs b/crates/my-lang/src/parser.rs index 9acb03f..d891036 100644 --- a/crates/my-lang/src/parser.rs +++ b/crates/my-lang/src/parser.rs @@ -33,16 +33,22 @@ pub type ParseResult = Result; /// through before emitting [`ParseError::ExpressionTooDeep`] and unwinding the /// in-flight expression. /// -/// Deliberately a quarter of `checker::MAX_EXPR_DEPTH` (256): each step of -/// expression nesting re-enters `parse_expr` and walks the full precedence -/// chain (`parse_or_expr` → … → `parse_postfix_expr` → `parse_primary_expr`), +/// Independently derived from the parser's own stack budget (not a fixed +/// ratio of `checker::MAX_EXPR_DEPTH`): each step of expression nesting +/// re-enters `parse_expr` and walks the full precedence chain +/// (`parse_or_expr` → … → `parse_postfix_expr` → `parse_primary_expr`), /// adding roughly a dozen native stack frames per nesting level. In debug /// builds with a 1 MiB default test-thread stack (Windows), that puts the /// hard `STATUS_STACK_OVERFLOW` boundary at roughly ~140 depth — so the /// guard has to fire well before that. 64 leaves a comfortable margin on /// every platform/build-mode the test suite runs on, while still accepting /// any human-authored expression (real code rarely nests past depth 5–10). -/// See hyperpolymath/my-lang#15 / #1. +/// +/// This is the *operational* ceiling: because the parser errors here, no +/// parsed program ever reaches `checker::MAX_EXPR_DEPTH` (re-derived to 128 +/// from a measured ≈4.4 KiB/level checker-stack budget in my-lang#37); the +/// checker guard only bounds programmatically-constructed ASTs. +/// See hyperpolymath/my-lang#15 / #1 / #37. pub const MAX_PARSE_EXPR_DEPTH: usize = 64; pub struct Parser { @@ -1097,8 +1103,9 @@ impl Parser { // Depth guard: stops pathological inputs (e.g. deeply nested // parentheses, right-recursive `str_concat("a", str_concat(...))` // chains) from overflowing the thread stack before the type-checker - // ever sees the AST. Mirrors `checker::MAX_EXPR_DEPTH`; see - // hyperpolymath/my-lang#15. + // ever sees the AST. This is the operational ceiling that keeps any + // parsed AST below `checker::MAX_EXPR_DEPTH`; see + // hyperpolymath/my-lang#15 / #37. if self.expr_depth >= MAX_PARSE_EXPR_DEPTH { let span = self.current_span(); return Err(ParseError::ExpressionTooDeep { diff --git a/crates/my-lang/tests/checker_alloc_scaling.rs b/crates/my-lang/tests/checker_alloc_scaling.rs index 57746ba..16ca14f 100644 --- a/crates/my-lang/tests/checker_alloc_scaling.rs +++ b/crates/my-lang/tests/checker_alloc_scaling.rs @@ -206,9 +206,13 @@ fn deep_chain_program(depth: usize) -> Program { /// deep-but-legal programs (well under 256) allocate pathologically. #[test] fn checker_allocation_is_linear_in_chain_depth() { - // All strictly below MAX_EXPR_DEPTH so the #12 guard never fires and we - // measure the genuine per-level cost. - let depths = [32usize, 64, 128, 200]; + // Derived from MAX_EXPR_DEPTH (not hardcoded) so this stays strictly below + // the #12 guard regardless of how the constant is tuned — it was + // re-derived 256 -> 128 from a measured stack budget in + // hyperpolymath/my-lang#37. The ladder still spans a ~6x depth range, more + // than enough to expose any super-linear per-level term. + let m = MAX_EXPR_DEPTH; + let depths = [m / 8, m / 4, m / 2, (m * 3) / 4]; assert!(*depths.last().unwrap() < MAX_EXPR_DEPTH); let mut report = Vec::new();