Fix exponential flow tracing in files with loops

[MillhioreBT]

Summary

I was using the new engine from main in Zed with my OTClient project, and the language server got stuck at "Diagnose workspace 99%" forever. It was always the same file, walking.lua, which has a long walk() function with many ifs in a row and a small loop in another function.

FlowTree::has_complex_control is computed for the whole file, so one loop anywhere turns off the trace cache for every function in the file. Then a function with many ifs in a row checks every possible path, about 2^N:

lastDir = nil
walkLock = 0

function walk(dir)
  if walkLock > 1 and lastDir == dir then walkLock = walkLock + 1 end
  if walkLock > 2 and lastDir == dir then walkLock = walkLock + 1 end
  -- ... repeat up to `walkLock > 14`
  lastDir = dir
end

function other(t)
  for _ in pairs(t) do end -- without this loop it's instant
end

ifs in a row	before	after
12	6 s	0.21 s
14	> 40 s	0.07 s
18	> 40 s	0.07 s

The same project also crashed with a stack overflow, but that one is already fixed by #1306, so I left it out of this PR. Both PRs merge together without conflicts.

How it works

• FlowTree now has loop_reachable, computed once per file. A node is marked when, walking back from it, you can reach a loop, label, break or continue.
• trace_decl uses the cache for any node that is not marked, instead of turning it off for the whole file. The change in flow.rs is only that condition.

Notes

While writing the tests I saw that the binder doesn't create loop back-edges, so in practice only the code after a backward goto loses the cache. That means turning it off for the whole file was more careful than it needed to be. If back-edges are added later, the marking will follow them.

Because of the same thing, a variable assigned inside a for or while loop is still seen as nil after the loop (repeat works fine). I prefer to leave that for another PR.

Testing

I tested it on Windows 10, in a release build, with Zed pointing to it. walking.lua went from never finishing (more than 60 s) to 0.2 s. With this PR and #1306 together, emmylua_check on my whole project (~280 Lua files) went from more than 9 minutes to 3.6 s.

Tests added in semantic_db/phase_tests/flow_loop_memo_tests.rs:

• flow_loop_in_other_function_keeps_guard_chain_linear: counts how many times the trace runs. With the fix it's 16n + 3. Without it, it's 631 for n=4 (and 2 million for n=16), so the test fails right away.
• flow_loop_reachability_is_tracked_per_node: walk is not marked, and the code after a backward goto is.

All the tests in emmylua_code_analysis pass.

To test it manually:

1. Save the snippet above as a.lua in an empty folder.
2. Run emmylua_check . --severity error.
3. On main it takes more than 40 s. On this branch it finishes in less than 0.1 s.

If you have any suggestions on how to improve the code let me know <3

Checklist

• I have tested these changes locally
• My code follows the project's code style (cargo fmt --check, cargo clippy --tests clean)
• I have not introduced new compiler warnings

[github-actions]

Code Review: Per-Node Loop Reachability for Flow Memoization

Summary

The change is well-motivated and well-tested. It replaces a coarse per-file gate (has_complex_control) with a per-node gate (loop_reachable), so a loop in one function no longer disables trace-state memoization for unrelated functions in the same file. The BFS-based reachability computation is correct in structure, and the regression tests are thoughtful. Below are issues and suggestions, ordered by severity.

---

Issues

1. is_loop_reachable fallback is inconsistent with the constructor (Medium)

pub fn is_loop_reachable(&self, flow_id: FlowId) -> bool {
    self.loop_reachable
        .get(flow_id.0 as usize)
        .copied()
        .unwrap_or(self.has_complex_control)
}

When has_complex_control is false, loop_reachable is vec![false; n], so an out-of-range flow_id returns false — fine. But when has_complex_control is true, an out-of-range id returns true, which is the conservative choice. That's defensible, but note the asymmetry: the fallback depends on a field that is also used to decide whether loop_reachable was populated at all. If a future refactor ever populates loop_reachable unconditionally, this fallback silently changes meaning. Consider making the intent explicit:

.unwrap_or(true) // unknown node: be conservative, keep uncached path

or documenting why has_complex_control is the right fallback. As written, a reader has to reason about the coupling between two fields.

2. successors indexing assumes FlowId is dense and in-range (Low–Medium)

if let Some(list) = successors.get_mut(prev.0 as usize) {
    list.push(id);
}

The get_mut guard silently drops edges whose antecedent id is out of range. That's safe, but it means a malformed graph would silently produce a less conservative loop_reachable (missing propagation), which is exactly the direction that could reintroduce the exponential blowup or, worse, incorrect memoization. Since flow_nodes is built internally and ids are assigned densely, this should never happen — but a debug_assert! would catch regressions:

debug_assert!((prev.0 as usize) < flow_nodes.len());

Same for successors[id as usize] in the BFS loop, which is not guarded and would panic on a bad id. The inconsistency between the guarded edge-insert and the unguarded BFS lookup is worth aligning.

3. Multiple antecedent handling duplicates logic (Low)

The Single and Multiple arms both do the same successors.get_mut(prev.0).push(id). Consider extracting a small closure or helper to avoid the duplication, which also makes the debug_assert suggestion above apply in one place:

let mut add_edge = |prev: FlowId| {
    if let Some(list) = successors.get_mut(prev.0 as usize) {
        list.push(id);
    }
};

(Closure borrows successors mutably; may need to restructure slightly, but the duplication is real.)

4. Seed set may be incomplete for goto targets (Low — verify)

The seed kinds are LoopLabel | NamedLabel(_) | ForIStat(_) | Break | Continue. The test relies on a backward goto to a ::again:: label to mark the region behind it as loop-reachable. That works because NamedLabel is seeded. But consider:

• A backward goto to a label that is not a NamedLabel node kind (if the parser models some labels differently).
• While/Repeat/NumericFor loop nodes — are these represented as LoopLabel? The seed list includes ForIStat but not, e.g., WhileStat/RepeatStat. If those loops are modeled via LoopLabel, fine; otherwise a while loop would not seed reachability and the memo would be incorrectly enabled behind it.

Please confirm the full set of loop node kinds is covered. The test only exercises for _ in pairs(t) (ForIStat) and a goto, so a while/repeat regression would not be caught. Recommend adding a while and repeat case to the test.

5. Test bound is loose and magic-numbered (Low)

let bound = 200 + 60 * (n as u64);

The comment says the expected count is 16n + 3, but the assertion allows 60n + 200. That's a ~4x margin, which is reasonable for robustness but means the test would not catch a regression that merely doubles the work. Consider tightening toward 16n + 3 with a modest constant (e.g., 20 * n + 50) and documenting the observed values, or asserting the ratio between consecutive n is ~linear. As-is, the test is a good guard against the exponential case but weak against moderate regressions.

Also, flow_metrics::reset() is called after def_file — confirm def_file doesn't itself trigger trace walks that would pollute the counter. If it does, the reset placement is correct; if not, it's harmless. Worth a one-line comment.

6. read_every_name ignores errors silently (Low)

let _ = model.type_of_expr_at(...);

Fine for a perf test, but if type_of_expr_at starts returning Err for all names (e.g., due to an unrelated regression), the test would still pass with a low count. Consider asserting at least one call succeeds, or counting successes.

7. Doc comment on loop_reachable field is slightly imprecise (Nit)

whether a loop / label / break / continue is backward-reachable from it

"backward-reachable" is the right concept, but the field is computed by forward propagation from seeds along successor edges. A reader may wonder why. Consider: "whether any loop/label/break/continue node can reach this node by following antecedent edges (computed by forward propagation from those nodes)."

---

Correctness Assessment

The core algorithm is sound:

• Successor edges are built from antecedents (single and multi), which is the correct direction for forward propagation.
• Seeding all loop/label/break/continue nodes and BFS-ing forward marks exactly the nodes with such a node in their backward cone.
• The !*slot guard prevents re-enqueueing, giving O(V + E).
• The has_complex_control == false fast path (vec![false; n]) is correct because without complex control there are no loops/labels to reach.

The memoization gate change in flow.rs is a strict refinement of the old gate: is_loop_reachable(id) == true implies the old has_complex_control == true (since loop_reachable is only populated when has_complex_control), so no node that previously took the uncached path now takes the cached path unless it genuinely has no loop behind it. That's the intended and safe direction.

---

Recommendations (priority order)

1. Add while and repeat loop cases to the test to confirm the seed-kind list is complete (Issue 4).
2. Add debug_assert!s for id bounds and align the guarded/unguarded lookups (Issue 2).
3. Make the is_loop_reachable fallback intent explicit (Issue 1).
4. Tighten the test bound or assert linear scaling (Issue 5).
5. Deduplicate the edge-insertion arms (Issue 3).
6. Minor: doc wording, read_every_name assertion (Issues 6–7).

Overall: solid change, good test coverage for the reported regression, and the performance fix is well-targeted. The main gap is test coverage for non-for/non-goto loop constructs, which is the most likely place for a latent correctness bug.