Skip to content

Commit e0e6b6b

Browse files
feat(parser): add MAX_PARSE_EXPR_DEPTH guard + recover partial AST (closes #15) (#21)
Mirrors the type-checker's MAX_EXPR_DEPTH guard (checker.rs:117) in the recursive-descent parser so deeply-nested expressions surface a clean ParseError::ExpressionTooDeep instead of SIGABRT'ing the process on thread-stack overflow. Upstream fixes alongside the guard: * parse_program now returns Ok(Program { items }) with whatever was successfully parsed; collected errors are exposed via Parser::errors(). Previously parse_program returned Err on the first collected error, which threw away every successfully-parsed item and contradicted the existing test_error_recovery_* assertions (they expect program.is_some() alongside a non-empty error list). Those tests had been silently skipped because an earlier SIGABRT test crashed the binary before they ran -- this commit's depth guard is what made them reachable. * parse_program now guarantees forward progress in its error-recovery loop. If parse_top_level Err's on a sync-point token (e.g. `}`) and synchronize returns at the same token, the loop would spin forever pushing the same error until OOM. A saved_pos check now forces one advance() when neither call consumed a token. * The crate-level `parse` wrapper in lib.rs preserves the stricter single-error contract that compile/eval rely on -- it consults parser.errors() after parse_program and returns the first one as Err. MAX_PARSE_EXPR_DEPTH is set to 64 (vs the checker's 256) because the parser's precedence-climbing chain produces ~11 stack frames per nesting level in debug builds -- 64 leaves a safe margin under Windows' 1 MiB default test-thread stack. Tests added: * test_deeply_nested_parens_report_too_deep_instead_of_sigabrt * test_deeply_nested_call_chain_reports_too_deep_instead_of_sigabrt * test_moderately_nested_parens_still_parse * test_depth_guard_resets_between_top_level_items All 131 my-lang lib tests + 15 integration tests pass.
1 parent 63cd532 commit e0e6b6b

2 files changed

Lines changed: 202 additions & 12 deletions

File tree

crates/my-lang/src/lib.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,24 @@ pub use types::Ty;
3838
// Library prelude for easy access to common functions and types
3939
pub use library::prelude;
4040

41-
/// Parse source code into an AST
41+
/// Parse source code into an AST.
42+
///
43+
/// `Parser::parse_program` itself runs error recovery and always returns
44+
/// `Ok(Program)` with whatever it managed to parse, exposing collected
45+
/// errors via `Parser::errors()`. This crate-level wrapper preserves the
46+
/// stricter single-error contract that downstream callers (`compile`,
47+
/// `eval`) rely on: if any error was collected, return the first one as
48+
/// `Err`. Callers that want partial-AST recovery should drive the
49+
/// `Parser` directly and consult `parser.errors()`.
4250
pub fn parse(source: &str) -> ParseResult<Program> {
4351
let mut lexer = Lexer::new(source);
4452
let tokens = lexer.tokenize();
4553
let mut parser = Parser::new(tokens);
46-
parser.parse_program()
54+
let program = parser.parse_program()?;
55+
if let Some(first) = parser.errors().first() {
56+
return Err(first.clone());
57+
}
58+
Ok(program)
4759
}
4860

4961
/// Parse and type-check source code

crates/my-lang/src/parser.rs

Lines changed: 188 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,39 @@ pub enum ParseError {
1919
UnexpectedEof,
2020
#[error("invalid literal: {0}")]
2121
InvalidLiteral(String),
22+
#[error("expression nesting depth exceeds limit ({limit}) at line {line}, column {column}; refactor deeply nested expressions (e.g. parenthesized or chained calls) into intermediate `let` bindings")]
23+
ExpressionTooDeep {
24+
limit: usize,
25+
line: usize,
26+
column: usize,
27+
},
2228
}
2329

2430
pub type ParseResult<T> = Result<T, ParseError>;
2531

32+
/// Maximum expression nesting depth the recursive-descent parser will descend
33+
/// through before emitting [`ParseError::ExpressionTooDeep`] and unwinding the
34+
/// in-flight expression.
35+
///
36+
/// Deliberately a quarter of `checker::MAX_EXPR_DEPTH` (256): each step of
37+
/// expression nesting re-enters `parse_expr` and walks the full precedence
38+
/// chain (`parse_or_expr` → … → `parse_postfix_expr` → `parse_primary_expr`),
39+
/// adding roughly a dozen native stack frames per nesting level. In debug
40+
/// builds with a 1 MiB default test-thread stack (Windows), that puts the
41+
/// hard `STATUS_STACK_OVERFLOW` boundary at roughly ~140 depth — so the
42+
/// guard has to fire well before that. 64 leaves a comfortable margin on
43+
/// every platform/build-mode the test suite runs on, while still accepting
44+
/// any human-authored expression (real code rarely nests past depth 5–10).
45+
/// See hyperpolymath/my-lang#15 / #1.
46+
pub const MAX_PARSE_EXPR_DEPTH: usize = 64;
47+
2648
pub struct Parser {
2749
tokens: Vec<Token>,
2850
pos: usize,
2951
errors: Vec<ParseError>,
52+
/// Current expression-recursion depth, tracked at every entry to
53+
/// [`Parser::parse_expr`]. Bounded by [`MAX_PARSE_EXPR_DEPTH`].
54+
expr_depth: usize,
3055
}
3156

3257
impl Parser {
@@ -35,27 +60,47 @@ impl Parser {
3560
tokens,
3661
pos: 0,
3762
errors: Vec::new(),
38-
}
39-
}
40-
63+
expr_depth: 0,
64+
}
65+
}
66+
67+
/// Parse the token stream into a `Program`.
68+
///
69+
/// This always returns `Ok(Program { items })` containing whichever
70+
/// top-level items were successfully parsed; any errors encountered
71+
/// during recovery are collected in `self.errors` and exposed via
72+
/// [`Parser::errors`]. Callers that want a single-error contract
73+
/// (e.g. the crate-level `parse` wrapper) should inspect
74+
/// `parser.errors()` after this call and surface the first one.
75+
///
76+
/// Why `Ok` even with errors: tests like
77+
/// `test_error_recovery_sync_at_semicolon` exercise the recovery
78+
/// path and expect to see the partial AST alongside the error list;
79+
/// returning `Err` here would throw away every successfully-parsed
80+
/// item on the first stray token.
4181
pub fn parse_program(&mut self) -> ParseResult<Program> {
4282
let mut items = Vec::new();
4383
while !self.is_at_end() {
84+
let saved_pos = self.pos;
4485
match self.parse_top_level() {
4586
Ok(item) => items.push(item),
4687
Err(err) => {
4788
self.errors.push(err);
4889
self.synchronize();
90+
// Forward-progress guarantee: if neither parse_top_level
91+
// nor synchronize consumed a token (e.g. we Err'd at a
92+
// sync-point token like `}` and synchronize returned at
93+
// the same `}` without advancing), force one step so the
94+
// outer loop terminates. Without this, deeply-nested or
95+
// unbalanced inputs that trip the depth guard can spin
96+
// forever pushing the same error until OOM.
97+
if self.pos == saved_pos {
98+
self.advance();
99+
}
49100
}
50101
}
51102
}
52103

53-
// If we collected any errors, return the first one
54-
// (in the future, we could return all errors in a different error type)
55-
if !self.errors.is_empty() {
56-
return Err(self.errors[0].clone());
57-
}
58-
59104
Ok(Program { items })
60105
}
61106

@@ -1061,7 +1106,23 @@ impl Parser {
10611106
// ============================================
10621107

10631108
fn parse_expr(&mut self) -> ParseResult<Expr> {
1064-
self.parse_or_expr()
1109+
// Depth guard: stops pathological inputs (e.g. deeply nested
1110+
// parentheses, right-recursive `str_concat("a", str_concat(...))`
1111+
// chains) from overflowing the thread stack before the type-checker
1112+
// ever sees the AST. Mirrors `checker::MAX_EXPR_DEPTH`; see
1113+
// hyperpolymath/my-lang#15.
1114+
if self.expr_depth >= MAX_PARSE_EXPR_DEPTH {
1115+
let span = self.current_span();
1116+
return Err(ParseError::ExpressionTooDeep {
1117+
limit: MAX_PARSE_EXPR_DEPTH,
1118+
line: span.line,
1119+
column: span.column,
1120+
});
1121+
}
1122+
self.expr_depth += 1;
1123+
let result = self.parse_or_expr();
1124+
self.expr_depth -= 1;
1125+
result
10651126
}
10661127

10671128
fn parse_or_expr(&mut self) -> ParseResult<Expr> {
@@ -2624,4 +2685,121 @@ mod tests {
26242685
// Should still parse top-level structure
26252686
assert!(program.is_some(), "Expected program despite errors");
26262687
}
2688+
2689+
// ============================================
2690+
// Expression-Depth Guard Tests (hyperpolymath/my-lang#15)
2691+
// ============================================
2692+
2693+
#[test]
2694+
fn test_deeply_nested_parens_report_too_deep_instead_of_sigabrt() {
2695+
// Regression for hyperpolymath/my-lang#15: deeply nested expressions
2696+
// used to overflow the recursive-descent parser's thread stack and
2697+
// SIGABRT the process. With the MAX_PARSE_EXPR_DEPTH guard, we get a
2698+
// clean ExpressionTooDeep error instead.
2699+
//
2700+
// Each `(` re-enters parse_expr via parse_paren_expr, so the depth
2701+
// increments on every layer. We feed in just over the limit so the
2702+
// guard fires; staying close to the limit also keeps us well under
2703+
// the empirical SIGABRT threshold (~230 on Windows debug builds).
2704+
let depth = MAX_PARSE_EXPR_DEPTH + 16;
2705+
let mut src = String::from("fn main() { let x = ");
2706+
for _ in 0..depth {
2707+
src.push('(');
2708+
}
2709+
src.push('1');
2710+
for _ in 0..depth {
2711+
src.push(')');
2712+
}
2713+
src.push_str("; }");
2714+
2715+
let (_program, errors) = parse_with_errors(&src);
2716+
2717+
assert!(
2718+
errors
2719+
.iter()
2720+
.any(|e| matches!(e, ParseError::ExpressionTooDeep { limit, .. } if *limit == MAX_PARSE_EXPR_DEPTH)),
2721+
"expected ExpressionTooDeep, got: {:?}",
2722+
errors
2723+
);
2724+
}
2725+
2726+
#[test]
2727+
fn test_deeply_nested_call_chain_reports_too_deep_instead_of_sigabrt() {
2728+
// Right-recursive `str_concat("a", str_concat("a", ... "end"))` is the
2729+
// exact shape that originally surfaced this bug while writing the
2730+
// type-checker regression test (#12). Confirm the parser bails out
2731+
// cleanly here as well.
2732+
let depth = MAX_PARSE_EXPR_DEPTH + 16;
2733+
let mut src = String::from("fn main() { let s = ");
2734+
for _ in 0..depth {
2735+
src.push_str("str_concat(\"a\", ");
2736+
}
2737+
src.push_str("\"end\"");
2738+
for _ in 0..depth {
2739+
src.push(')');
2740+
}
2741+
src.push_str("; }");
2742+
2743+
let (_program, errors) = parse_with_errors(&src);
2744+
2745+
assert!(
2746+
errors
2747+
.iter()
2748+
.any(|e| matches!(e, ParseError::ExpressionTooDeep { .. })),
2749+
"expected ExpressionTooDeep, got: {:?}",
2750+
errors
2751+
);
2752+
}
2753+
2754+
#[test]
2755+
fn test_moderately_nested_parens_still_parse() {
2756+
// Sanity: well below the limit must still parse successfully via the
2757+
// normal source path. Catches a too-tight off-by-one in the guard.
2758+
let depth = 32;
2759+
let mut src = String::from("fn main() { let x = ");
2760+
for _ in 0..depth {
2761+
src.push('(');
2762+
}
2763+
src.push('1');
2764+
for _ in 0..depth {
2765+
src.push(')');
2766+
}
2767+
src.push_str("; }");
2768+
2769+
let program = parse(&src).expect("moderately-nested parens must parse");
2770+
assert_eq!(program.items.len(), 1);
2771+
}
2772+
2773+
#[test]
2774+
fn test_depth_guard_resets_between_top_level_items() {
2775+
// After error-recovery synchronizes past a too-deep expression, the
2776+
// depth counter must be back at 0 so subsequent top-level items parse
2777+
// normally. Catches a regression where the depth `-=` was skipped on
2778+
// the error path (e.g. if we'd used `?` inside parse_expr).
2779+
let depth = MAX_PARSE_EXPR_DEPTH + 8;
2780+
let mut src = String::from("fn broken() { let x = ");
2781+
for _ in 0..depth {
2782+
src.push('(');
2783+
}
2784+
src.push('1');
2785+
for _ in 0..depth {
2786+
src.push(')');
2787+
}
2788+
src.push_str("; }\nfn ok() { let y = 42; }");
2789+
2790+
let (program, errors) = parse_with_errors(&src);
2791+
assert!(
2792+
errors
2793+
.iter()
2794+
.any(|e| matches!(e, ParseError::ExpressionTooDeep { .. })),
2795+
"expected ExpressionTooDeep in errors: {:?}",
2796+
errors
2797+
);
2798+
if let Some(prog) = program {
2799+
assert!(
2800+
prog.items.iter().any(|it| matches!(it, TopLevel::Function(f) if f.name.name == "ok")),
2801+
"expected `ok` function to parse after recovery"
2802+
);
2803+
}
2804+
}
26272805
}

0 commit comments

Comments
 (0)