Skip to content

Commit 87c5e43

Browse files
hyperpolymathclaude
andcommitted
fix(checker): general non-overflowing AST teardown + measured MAX_EXPR_DEPTH (#37)
Closes the Linux-side of #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<Expr>, Vec<Expr>, 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 <noreply@anthropic.com>
1 parent 0188460 commit 87c5e43

5 files changed

Lines changed: 318 additions & 49 deletions

File tree

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
// SPDX-License-Identifier: PMPL-1.0-or-later
2+
//! Stack-budget probe for hyperpolymath/my-lang#37.
3+
//!
4+
//! Builds a deep, *non-`Call`-shaped* AST (a `Unary::Not` chain — the shape
5+
//! the old test helper could not handle) and exercises one recursive walk on
6+
//! a thread with a known, fixed stack size. The process aborts on stack
7+
//! overflow, so a wrapper runs this at increasing depths and reads the exit
8+
//! status to find the overflow cliff. From `cliff_depth` at a known
9+
//! `stack_bytes` we get `bytes_per_level ≈ stack_bytes / cliff_depth`.
10+
//!
11+
//! Usage: measure_depth <check|drop_recursive|drop_iter> <depth> <stack_kib>
12+
13+
use my_lang::ast::*;
14+
use my_lang::token::Span;
15+
16+
fn build_unary_chain(depth: usize) -> Expr {
17+
let span = Span::default();
18+
let mut e = Expr::Literal(Literal::Bool(true, span));
19+
for _ in 0..depth {
20+
e = Expr::Unary {
21+
op: UnaryOp::Not,
22+
operand: Box::new(e),
23+
span,
24+
};
25+
}
26+
e
27+
}
28+
29+
fn wrap(value: Expr) -> Program {
30+
let span = Span::default();
31+
Program {
32+
items: vec![TopLevel::Function(FnDecl {
33+
modifiers: vec![],
34+
name: Ident::new("main", span),
35+
params: vec![],
36+
return_type: None,
37+
contract: None,
38+
body: Block {
39+
stmts: vec![Stmt::Let {
40+
mutable: false,
41+
name: Ident::new("s", span),
42+
ty: None,
43+
value,
44+
span,
45+
}],
46+
span,
47+
},
48+
span,
49+
})],
50+
}
51+
}
52+
53+
fn main() {
54+
let a: Vec<String> = std::env::args().collect();
55+
let mode = a.get(1).cloned().unwrap_or_default();
56+
let depth: usize = a.get(2).and_then(|s| s.parse().ok()).unwrap_or(1000);
57+
let stack_kib: usize = a.get(3).and_then(|s| s.parse().ok()).unwrap_or(1024);
58+
59+
let h = std::thread::Builder::new()
60+
.stack_size(stack_kib * 1024)
61+
.spawn(move || {
62+
let program = wrap(build_unary_chain(depth));
63+
match mode.as_str() {
64+
"check" => {
65+
// check_expr recursion (the guard is what #37 re-derives).
66+
let _ = my_lang::check(&program);
67+
my_lang::ast::drop_program_iteratively(program);
68+
}
69+
"drop_recursive" => {
70+
// The unguarded danger: auto-derived recursive Drop.
71+
drop(program);
72+
}
73+
"drop_iter" => {
74+
// The #37 fix: general iterative teardown.
75+
my_lang::ast::drop_program_iteratively(program);
76+
}
77+
_ => eprintln!("unknown mode"),
78+
}
79+
println!("OK depth={depth} stack_kib={stack_kib} mode={mode}");
80+
})
81+
.unwrap();
82+
h.join().unwrap();
83+
}

crates/my-lang/src/ast.rs

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -694,3 +694,140 @@ impl Literal {
694694
}
695695
}
696696
}
697+
698+
// ============================================
699+
// Non-recursive AST teardown (hyperpolymath/my-lang#37)
700+
// ============================================
701+
702+
/// One pending node in the iterative teardown worklist.
703+
enum DropNode {
704+
Expr(Expr),
705+
Block(Block),
706+
}
707+
708+
/// Drop an entire [`Program`] without recursing through the AST.
709+
///
710+
/// # Why this exists
711+
///
712+
/// `Expr` is a tree of `Box<Expr>` (plus `Block`/`Stmt`/`Match`/… edges).
713+
/// The compiler-derived `Drop` recurses once per nesting level, so a
714+
/// sufficiently deep AST overflows the thread stack *on drop* — independently
715+
/// of the type checker, and for **any** shape, not just the `Call` chains the
716+
/// former test-only `drop_program_iteratively` helper special-cased
717+
/// (hyperpolymath/my-lang#37, subtlety 1).
718+
///
719+
/// Implementing `Drop for Expr` would be the other option, but it is rejected
720+
/// deliberately: a `Drop` impl forbids by-value destructuring of `Expr`
721+
/// (`cannot move out of a type which implements Drop`), which the parser /
722+
/// HIR / MIR crates rely on pervasively. A standalone teardown keeps the AST a
723+
/// plain movable tree while still giving any owner a non-overflowing drop.
724+
///
725+
/// # How it works
726+
///
727+
/// Owned nodes are pushed onto an explicit heap worklist. Each node is popped
728+
/// **by value** and destructured (legal precisely because `Expr` has no `Drop`
729+
/// impl); every recursive child is moved into the worklist *before* the
730+
/// shallow remainder of the node goes out of scope. The derived drop therefore
731+
/// only ever runs at depth 1. O(n) time, O(n) heap, **O(1) stack** — no
732+
/// recursion and no assumptions about AST shape.
733+
pub fn drop_program_iteratively(mut program: Program) {
734+
let mut work: Vec<DropNode> = Vec::new();
735+
736+
// Seed from function bodies — the only place unbounded nesting can arise.
737+
// Every other `TopLevel` is a shallow declaration and drops at O(1) depth.
738+
for item in program.items.drain(..) {
739+
if let TopLevel::Function(f) = item {
740+
work.push(DropNode::Block(f.body));
741+
}
742+
}
743+
744+
while let Some(node) = work.pop() {
745+
match node {
746+
DropNode::Expr(e) => detach_expr_children(e, &mut work),
747+
DropNode::Block(b) => detach_block_children(b, &mut work),
748+
}
749+
// `node` is fully consumed above; its non-recursive remainder (spans,
750+
// operators, identifiers, literals) drops here at depth 1.
751+
}
752+
}
753+
754+
fn detach_expr_children(e: Expr, work: &mut Vec<DropNode>) {
755+
match e {
756+
Expr::Call { callee, args, .. } => {
757+
work.push(DropNode::Expr(*callee));
758+
work.extend(args.into_iter().map(DropNode::Expr));
759+
}
760+
Expr::Field { object, .. } => work.push(DropNode::Expr(*object)),
761+
Expr::Binary { left, right, .. } => {
762+
work.push(DropNode::Expr(*left));
763+
work.push(DropNode::Expr(*right));
764+
}
765+
Expr::Unary { operand, .. }
766+
| Expr::Try { operand, .. }
767+
| Expr::Restrict { operand, .. } => work.push(DropNode::Expr(*operand)),
768+
Expr::Block(b) => work.push(DropNode::Block(b)),
769+
Expr::Ai(ai) => detach_ai_expr(ai, work),
770+
Expr::Lambda { body, .. } => match body {
771+
LambdaBody::Expr(b) => work.push(DropNode::Expr(*b)),
772+
LambdaBody::Block(bl) => work.push(DropNode::Block(bl)),
773+
},
774+
Expr::Match { scrutinee, arms, .. } => {
775+
work.push(DropNode::Expr(*scrutinee));
776+
work.extend(arms.into_iter().map(|a| DropNode::Expr(a.body)));
777+
}
778+
Expr::Array { elements, .. } => {
779+
work.extend(elements.into_iter().map(DropNode::Expr));
780+
}
781+
Expr::Record { fields, .. } => {
782+
work.extend(fields.into_iter().map(|f| DropNode::Expr(f.value)));
783+
}
784+
Expr::Literal(_) | Expr::Ident(_) => {}
785+
}
786+
}
787+
788+
fn detach_ai_expr(ai: AiExpr, work: &mut Vec<DropNode>) {
789+
match ai {
790+
AiExpr::Block { body, .. } => {
791+
for item in body {
792+
if let AiBodyItem::Field { value, .. } = item {
793+
work.push(DropNode::Expr(value));
794+
}
795+
}
796+
}
797+
AiExpr::Call { args, .. } | AiExpr::PromptInvocation { args, .. } => {
798+
work.extend(args.into_iter().map(DropNode::Expr));
799+
}
800+
AiExpr::Quick { .. } => {}
801+
}
802+
}
803+
804+
fn detach_block_children(b: Block, work: &mut Vec<DropNode>) {
805+
for stmt in b.stmts {
806+
match stmt {
807+
Stmt::Expr(e)
808+
| Stmt::Let { value: e, .. }
809+
| Stmt::Await { value: e, .. }
810+
| Stmt::Try { value: e, .. } => work.push(DropNode::Expr(e)),
811+
Stmt::If { condition, then_block, else_block, .. } => {
812+
work.push(DropNode::Expr(condition));
813+
work.push(DropNode::Block(then_block));
814+
if let Some(eb) = else_block {
815+
work.push(DropNode::Block(eb));
816+
}
817+
}
818+
Stmt::Go { block, .. } | Stmt::Comptime { block, .. } => {
819+
work.push(DropNode::Block(block))
820+
}
821+
Stmt::Return { value, .. } => {
822+
if let Some(v) = value {
823+
work.push(DropNode::Expr(v));
824+
}
825+
}
826+
Stmt::Ai(ai) => match ai.body {
827+
AiStmtBody::Block(bl) => work.push(DropNode::Block(bl)),
828+
AiStmtBody::Expr(e) => work.push(DropNode::Expr(*e)),
829+
},
830+
Stmt::Belief { .. } => {}
831+
}
832+
}
833+
}

crates/my-lang/src/checker.rs

Lines changed: 78 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,30 @@ pub enum CheckError {
118118
/// `check_expr` / `is_assignable_from` to be *linear* in AST size on Linux
119119
/// and on a Windows CI leg — there is no super-linear allocation for the
120120
/// guard to defend against. What deep nesting *does* threaten is the
121-
/// recursive descent through `check_expr` (and the recursive `Drop` of the
122-
/// resulting AST). The original #1 OOM was shown not to be checker
123-
/// algorithmic complexity; this limit therefore exists to keep recursion
124-
/// depth finite, and rejecting deep-but-legal programs at 256 is its cost,
125-
/// not a memory safeguard. Re-tuning the value is tracked separately.
126-
pub const MAX_EXPR_DEPTH: usize = 256;
121+
/// recursive descent through `check_expr`. (The recursive `Drop` of a deep
122+
/// AST is a *separate* overflow; it is now handled structurally and
123+
/// shape-independently by [`crate::ast::drop_program_iteratively`], so it no
124+
/// longer constrains this value — hyperpolymath/my-lang#37.)
125+
///
126+
/// # Re-derived from a measured stack budget (my-lang#37)
127+
///
128+
/// The previous value (256) was an inherited guess, not a budget. Probing one
129+
/// recursive `check_expr` walk on a fixed-size thread stack
130+
/// (`examples/measure_depth.rs`) measures **≈4.4 KiB of stack per nesting
131+
/// level**. The binding constraint is the smallest stack the checker runs on:
132+
/// the OS main thread (the checker is *not* dispatched onto a large-stack
133+
/// thread anywhere), i.e. **1 MiB on Windows**. At 256 levels that walk needs
134+
/// ≈1.14 MiB — it can overflow *before* the guard fires on Windows, so 256 was
135+
/// not merely unjustified but unsafe there.
136+
///
137+
/// `128 × 4.4 KiB ≈ 569 KiB ≈ 54%` of a 1 MiB Windows stack, leaving ~46%
138+
/// headroom for the rest of the call graph above `check_expr`; vastly safe on
139+
/// Linux (8 MiB) and Rust's 2 MiB default threads. It is still 2× the parser's
140+
/// independently-derived [`crate::parser::MAX_PARSE_EXPR_DEPTH`] (64) ceiling,
141+
/// so — because no *parseable* program nests deeper than 64 — lowering it
142+
/// rejects zero real programs; it only ever fires on programmatically-built
143+
/// ASTs, which is its sole remaining purpose.
144+
pub const MAX_EXPR_DEPTH: usize = 128;
127145

128146
pub type CheckResult<T> = Result<T, CheckError>;
129147

@@ -1450,40 +1468,10 @@ mod tests {
14501468
check(&program)
14511469
}
14521470

1453-
/// Iteratively flattens any `str_concat(_, inner)`-style right-recursive
1454-
/// AST chains inside `program` so that the auto-derived recursive `Drop`
1455-
/// for `Box<Expr>` does not overflow the test thread's stack. Only the
1456-
/// shapes we construct in the deep-nesting tests are handled.
1457-
fn drop_program_iteratively(mut program: Program) {
1458-
use std::mem;
1459-
for item in program.items.iter_mut() {
1460-
if let TopLevel::Function(f) = item {
1461-
for stmt in f.body.stmts.iter_mut() {
1462-
if let Stmt::Let { value, .. } = stmt {
1463-
flatten_call_chain(value);
1464-
}
1465-
}
1466-
}
1467-
}
1468-
1469-
fn flatten_call_chain(expr: &mut Expr) {
1470-
// Repeatedly peel the deepest argument out of a Call and let it
1471-
// be dropped on its own once we've broken the chain.
1472-
loop {
1473-
let next = match expr {
1474-
Expr::Call { args, .. } if args.len() == 2 => {
1475-
// Replace the recursive arg with a tiny placeholder
1476-
// and take ownership of the giant subtree.
1477-
let placeholder =
1478-
Expr::Literal(Literal::Bool(false, crate::token::Span::default()));
1479-
mem::replace(&mut args[1], placeholder)
1480-
}
1481-
_ => return,
1482-
};
1483-
*expr = next;
1484-
}
1485-
}
1486-
}
1471+
// The former shape-specific `drop_program_iteratively` test helper (it
1472+
// only flattened 2-arg `Call` chains) is replaced by the general,
1473+
// shape-independent `crate::ast::drop_program_iteratively`, already in
1474+
// scope here via `super::*` (hyperpolymath/my-lang#37).
14871475

14881476
#[test]
14891477
fn test_basic_function() {
@@ -1644,6 +1632,56 @@ mod tests {
16441632
drop_program_iteratively(program);
16451633
}
16461634

1635+
#[test]
1636+
fn test_deep_non_call_ast_teardown_does_not_overflow() {
1637+
// Regression for hyperpolymath/my-lang#37, subtlety 1: the recursive
1638+
// `Drop` overflow is *shape-independent*. The former test helper only
1639+
// flattened 2-arg `Call` chains; a differently-shaped deep AST (here a
1640+
// `Unary::Not` chain, with no `Call` anywhere) would still overflow.
1641+
// The general `ast::drop_program_iteratively` must tear it down with
1642+
// O(1) stack regardless of shape.
1643+
//
1644+
// Depth is far beyond the measured recursive-Drop cliff (≈4–6k at a
1645+
// 512 KiB stack) so a recursion-based teardown would abort the test
1646+
// process; survival proves the teardown is non-recursive.
1647+
use crate::token::Span;
1648+
1649+
let span = Span::default();
1650+
let mut expr = Expr::Literal(Literal::Bool(true, span));
1651+
for _ in 0..50_000 {
1652+
expr = Expr::Unary {
1653+
op: UnaryOp::Not,
1654+
operand: Box::new(expr),
1655+
span,
1656+
};
1657+
}
1658+
1659+
let program = Program {
1660+
items: vec![TopLevel::Function(FnDecl {
1661+
modifiers: vec![],
1662+
name: Ident::new("main", span),
1663+
params: vec![],
1664+
return_type: None,
1665+
contract: None,
1666+
body: Block {
1667+
stmts: vec![Stmt::Let {
1668+
mutable: false,
1669+
name: Ident::new("s", span),
1670+
ty: None,
1671+
value: expr,
1672+
span,
1673+
}],
1674+
span,
1675+
},
1676+
span,
1677+
})],
1678+
};
1679+
1680+
// The sole assertion is that this returns at all: a recursive teardown
1681+
// would `STATUS_STACK_OVERFLOW` / SIGABRT the test runner first.
1682+
drop_program_iteratively(program);
1683+
}
1684+
16471685
#[test]
16481686
fn test_moderately_nested_str_concat_still_checks() {
16491687
// Sanity: well below the limit, deeply chained str_concat must still

0 commit comments

Comments
 (0)