Non-consuming multi-context push can loop when unwinding pops skip the guard's armed depth

[stefanobaghino]

Summary

The pop_would_loop guard in src/parsing/parser.rs (find_best_match, lines 680–682) correctly prevents infinite loops for single-context non-consuming push + non-consuming pop. For a multi-context non-consuming push, the guard can be bypassed when unwinding pops land at depths other than pre_push_depth + 1, producing an unbounded loop in parse_line_inner_from (parser.rs:414-422, which has no outer iteration cap).

Mechanism

Storage (parser.rs:526-538):

if !consuming {
    if matches!(match_pattern.operation, MatchOperation::Push(_) | …) {
        *non_consuming_push_at = (match_end, self.stack.len() + 1);
    }
}

The +1 hard-codes a single-context push. For push: [a, b, c] (K=3), the post-push stack depth is D+3 but stored is D+1.

Arming (parser.rs:438-441):

let check_pop_loop = {
    let (pos, stack_depth) = *non_consuming_push_at;
    pos == *start && stack_depth == self.stack.len()
};

Only arms when current depth == stored depth. For K>1, multiple intermediate depths are possible during the unwind (D+K, D+K-1, …, D+1), and a multi-level pop: N can jump straight past D+1.

Action (parser.rs:680-682, post-#644):

pop_would_loop = check_pop_loop
    && !consuming
    && matches!(match_pat.operation, MatchOperation::Pop(1));

Because check_pop_loop is the gating condition, narrowing Pop(_) → Pop(1) is orthogonal to this bug — both old and new guards miss it.

Minimal reproducer

name: test
scope: source.test
contexts:
  main:
    - match: ""
      push: [a, b, c]
  a: {}
  b:
    - match: ""
      pop: 2
  c:
    - match: ""
      pop: 1

Trace at pos P, starting depth D=1 (stored denotes non_consuming_push_at):

step	stack	stored	armed?	action
1	[main] D=1	(0,0)	n/a	main's "" push → stored (P,2)
2	[main,a,b,c] D=4	(P,2)	2≠4	c's "" pop:1
3	[main,a,b] D=3	(P,2)	2≠3	b's "" pop:2
4	[main] D=1	(P,2)	2≠1	step 2 again

No depth reaches 100, so push_too_deep (parser.rs:684-689) never fires. Hang.

Scope: not observed in bundled fixtures

Real multi-push cases always end with pop: 1 in the topmost context (e.g. testdata/Packages/JavaScript/JavaScript.sublime-syntax:272-273 — shebang: match: ^|(?=\S) pop: 1). That single pop lands on the armed depth D+1, where the existing guard catches subsequent non-consuming Pop(1). Consuming multi-push triggers (e.g. testdata/Packages/Regular Expressions/RegExp (Basic).sublime-syntax:108 — match: \() don't enter the non-consuming code path at all.

So this is a latent correctness bug, reachable only with a pathological/synthetic syntax today. Worth fixing because it's a true infinite-loop hazard and the data-shape change is modest.

Proposed fix (design)

Widen the stored state from (pos, armed_depth) to something that captures the whole post-push depth interval:

non_consuming_push_at: (usize /*pos*/, usize /*pre_push_depth*/, usize /*post_push_depth*/)

• On non-consuming Push/Branch/Embed (parser.rs:531-538), compute K from the MatchOperation:
  • MatchOperation::Push(refs) / Set { ctx_refs, .. } → refs.len().
  • MatchOperation::Branch { .. } → 1 (branch pushes one alt).
  • MatchOperation::Embed { .. } → 1.
    Store (match_end, self.stack.len(), self.stack.len() + K).
• Arming (parser.rs:438-441): pos == start && pre_push_depth < self.stack.len() && self.stack.len() <= post_push_depth.
• Action (parser.rs:680-682): when armed, flag any non-consuming op that would make current_depth - pop_count <= pre_push_depth. That covers:
  • MatchOperation::Pop(N) where current_depth - N <= pre_push_depth.
  • MatchOperation::Set { pop_count, ctx_refs } where the net depth after pop_count and K pushes would land ≤ pre_push_depth (edge case; may need its own test).
• When detected, advance one char as today (parser.rs:493-500).
• Reset stored state when a consuming match fires (already implicit since the state is only used when pos == start; a consuming match advances start).
• Verify branch-point snapshot save/restore touches the new triple consistently (parser.rs:144 struct field, and save/restore/reset sites at parser.rs:869, 1044, 1090, 1133, 1236).

Regression test (new)

#[test]
fn non_consuming_multi_push_with_skip_unwind_does_not_loop() {
    // Pre-fix: parser hangs — K=3 push stores depth D+1, multi-level
    // pops (pop:1 then pop:2) unwind D+3 → D+2 → D, skipping the armed
    // depth D+1 entirely. Under the fix, b's pop:2 is flagged when
    // current_depth - 2 (= D) <= pre_push_depth (= D), so the parser
    // advances one char and makes forward progress.
    let syntax = r#"
name: test
scope: source.test
contexts:
  main:
    - match: ""
      push: [a, b, c]
    - match: z
      scope: test.main.z
  a: {}
  b:
    - match: ""
      pop: 2
  c:
    - match: ""
      pop: 1
"#;
    expect_scope_stacks("z", &["<source.test>, <test.main.z>"], syntax);
}

Verification

• cargo test --lib parsing::parser — new test terminates; existing loop tests (can_parse_non_consuming_pop_that_would_loop*, can_parse_non_consuming_set_and_pop_that_would_loop, can_parse_non_consuming_pop_that_would_loop_at_end_of_line, non_consuming_pop_n_below_pre_push_depth_is_not_a_loop, can_parse_non_consuming_pop_with_multi_push_that_does_not_loop, can_parse_non_consuming_pop_of_recursive_context_that_does_not_loop) all stay green.
• make syntest — testdata/known_syntest_failures.txt and _fancy.txt baselines unchanged (no bundled fixture affected).
• Whole test suite — cargo test clean.

Related

• Issue Hang while highlighting JavaScript using Packages#v4050 #460 (open) — "Hang while highlighting JavaScript using Packages#v4050". Different specific file but same general "parser hang on newer Packages" family; worth cross-referencing, though root-cause overlap is unconfirmed.
• PR Restrict non-consuming pop loop guard to pop:1 #644 (aa2aace) narrowed the pop_would_loop action site from Pop(_) to Pop(1). That change is correct for its scope and orthogonal to this bug — the guard's gating (check_pop_loop) is what lets this scenario bypass detection, and that gating is unchanged by Restrict non-consuming pop loop guard to pop:1 #644.