From 49403a6ea1eeb55db385bc010670fcffa198050c Mon Sep 17 00:00:00 2001 From: Sarah Wesker Date: Sun, 18 Oct 2026 18:47:42 -0400 Subject: [PATCH 1/3] Fix exponential flow tracing in files with loops FlowTree::has_complex_control is computed per file, so one loop anywhere turned off trace-state memoization for every function in the file, and a chain of sequential guards walked ~2^N routes. Compute per node whether a loop, label, break or continue is backward-reachable, and memoize every node that is not. --- .../src/semantic_db/flow/flow_tree.rs | 75 +++++++++++++++ .../phase_tests/flow_loop_memo_tests.rs | 91 +++++++++++++++++++ .../src/semantic_db/phase_tests/mod.rs | 1 + .../src/semantic_model/flow.rs | 7 +- 4 files changed, 171 insertions(+), 3 deletions(-) create mode 100644 crates/emmylua_code_analysis/src/semantic_db/phase_tests/flow_loop_memo_tests.rs diff --git a/crates/emmylua_code_analysis/src/semantic_db/flow/flow_tree.rs b/crates/emmylua_code_analysis/src/semantic_db/flow/flow_tree.rs index 19592fe25..3e431d677 100644 --- a/crates/emmylua_code_analysis/src/semantic_db/flow/flow_tree.rs +++ b/crates/emmylua_code_analysis/src/semantic_db/flow/flow_tree.rs @@ -42,6 +42,10 @@ pub struct FlowTree { /// backward edges whose source positions are not monotonic, so the member /// assignment-position fast path is conservatively disabled. has_complex_control: bool, + /// Per node: whether a loop / label / break / continue is backward-reachable from it. + /// Nodes whose backward cone is loop-free can memoize trace states even when another + /// function in the same file has loops. + loop_reachable: Vec, } impl FlowTree { @@ -71,6 +75,11 @@ impl FlowTree { | FlowNodeKind::Continue ) }); + let loop_reachable = if has_complex_control { + Self::compute_loop_reachable(&flow_nodes, &multiple_antecedents) + } else { + vec![false; flow_nodes.len()] + }; for node in &flow_nodes { let flow_id = node.id; if matches!(node.antecedent, Some(FlowAntecedent::Multiple(_))) { @@ -155,7 +164,65 @@ impl FlowTree { branch_nodes, unpositioned_branch, has_complex_control, + loop_reachable, + } + } + + /// Marks every node that can reach a loop / label / break / continue by walking its + /// antecedents, by propagating forward from those nodes along successor edges. + fn compute_loop_reachable( + flow_nodes: &[FlowNode], + multiple_antecedents: &[Vec], + ) -> Vec { + let mut reachable = vec![false; flow_nodes.len()]; + let mut successors: Vec> = vec![Vec::new(); flow_nodes.len()]; + let mut queue = VecDeque::new(); + for node in flow_nodes { + let id = node.id.0; + match &node.antecedent { + Some(FlowAntecedent::Single(prev)) => { + if let Some(list) = successors.get_mut(prev.0 as usize) { + list.push(id); + } + } + Some(FlowAntecedent::Multiple(multi_id)) => { + for prev in multiple_antecedents + .get(*multi_id as usize) + .map(|v| v.as_slice()) + .unwrap_or_default() + { + if let Some(list) = successors.get_mut(prev.0 as usize) { + list.push(id); + } + } + } + None => {} + } + if matches!( + node.kind, + FlowNodeKind::LoopLabel + | FlowNodeKind::NamedLabel(_) + | FlowNodeKind::ForIStat(_) + | FlowNodeKind::Break + | FlowNodeKind::Continue + ) && let Some(slot) = reachable.get_mut(id as usize) + && !*slot + { + *slot = true; + queue.push_back(id); + } } + while let Some(id) = queue.pop_front() { + for &next in &successors[id as usize] { + if let Some(slot) = reachable.get_mut(next as usize) + && !*slot + { + *slot = true; + queue.push_back(next); + } + } + } + reachable } pub fn get_flow_id(&self, syntax_id: LuaSyntaxId) -> Option { @@ -271,6 +338,14 @@ impl FlowTree { self.has_complex_control } + /// Whether a loop / label / break / continue is backward-reachable from `flow_id`. + 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) + } + pub fn get_multi_antecedents(&self, id: u32) -> Option<&[FlowId]> { self.multiple_antecedents .get(id as usize) diff --git a/crates/emmylua_code_analysis/src/semantic_db/phase_tests/flow_loop_memo_tests.rs b/crates/emmylua_code_analysis/src/semantic_db/phase_tests/flow_loop_memo_tests.rs new file mode 100644 index 000000000..202a2027a --- /dev/null +++ b/crates/emmylua_code_analysis/src/semantic_db/phase_tests/flow_loop_memo_tests.rs @@ -0,0 +1,91 @@ +//! Regression guards for flow backtracking in files that contain a loop. +//! +//! One loop anywhere in a file disabled trace-state memoization for every +//! function in it, so a chain of `n` guards in a loop-free function walked +//! ~2^n routes (`walking.lua` of an OTClient project never finished). + +use emmylua_parser::{LuaAstNode, LuaNameExpr}; +use rowan::TextSize; + +use crate::semantic_model::flow::flow_metrics; +use crate::{FileId, VirtualWorkspace}; + +/// `n` sequential guards in `walk`, plus an unrelated loop in `other`. +fn guard_chain_with_loop_elsewhere(n: usize) -> String { + let mut content = String::from("lastDir = nil\nwalkLock = 0\n\nfunction walk(dir)\n"); + for i in 0..n { + content.push_str(&format!( + " if walkLock > {i} and lastDir == dir then\n walkLock = walkLock + 1\n end\n" + )); + } + content + .push_str(" lastDir = dir\nend\n\nfunction other(t)\n for _ in pairs(t) do end\nend\n"); + content +} + +const BACKWARD_GOTO: &str = " +function retry(t) + ::again:: + walkLock = 0 + if t then goto again end +end +"; + +/// Read the flow type of every name, the way checkers do for each reference. +fn read_every_name(ws: &VirtualWorkspace, file_id: FileId) { + let model = ws.analysis.semantic_model(file_id); + for name in model.chunk().expect("chunk").descendants::() { + let _ = model.type_of_expr_at(name.get_syntax_id(), name.get_position()); + } +} + +/// Only nodes with a real cycle behind them (a backward `goto`) lose the memo; a loop +/// or label elsewhere in the file no longer affects `walk`. +#[test] +fn flow_loop_reachability_is_tracked_per_node() { + let mut ws = VirtualWorkspace::new(); + let source = guard_chain_with_loop_elsewhere(2) + BACKWARD_GOTO; + let file_id = ws.def_file("walking.lua", &source); + let model = ws.analysis.semantic_model(file_id); + let tree = model.flow_tree().expect("flow tree"); + let flow_at = |needle: &str| { + let offset = source.find(needle).expect("needle in source") as u32; + tree.get_flow_id_at(TextSize::from(offset)) + .unwrap_or_else(|| panic!("no flow node at `{needle}` (offset {offset})")) + }; + + assert!(tree.has_complex_control()); + assert!( + !tree.is_loop_reachable(flow_at("lastDir = dir")), + "`walk` has no loop behind it, so its trace states stay memoizable" + ); + assert!( + tree.is_loop_reachable(flow_at("walkLock = 0\n if t")), + "a statement behind the backward `goto` must keep the uncached path" + ); +} + +/// The number of uncached walks must stay linear in the number of guards even +/// though the file contains a loop: `16n + 3` with the per-node gate. With the +/// per-file gate it was `[(4, 631), (8, 52455), (16, 2065146), (32, 6638442)]`, so +/// the check runs per size and fails on the first one instead of walking the +/// larger chains. +#[test] +fn flow_loop_in_other_function_keeps_guard_chain_linear() { + let mut measurements = Vec::new(); + for n in [4usize, 8, 16, 32] { + let mut ws = VirtualWorkspace::new(); + let file_id = ws.def_file("walking.lua", &guard_chain_with_loop_elsewhere(n)); + flow_metrics::reset(); + read_every_name(&ws, file_id); + let entries = flow_metrics::trace_decl_entries(); + measurements.push((n, entries)); + + let bound = 200 + 60 * (n as u64); + assert!( + entries <= bound, + "the declaration walk ran {entries} times for {n} guards (bound {bound}); \ + a loop in another function disabled the trace-state memo. So far: {measurements:?}" + ); + } +} diff --git a/crates/emmylua_code_analysis/src/semantic_db/phase_tests/mod.rs b/crates/emmylua_code_analysis/src/semantic_db/phase_tests/mod.rs index 7bcc763b7..e1339e543 100644 --- a/crates/emmylua_code_analysis/src/semantic_db/phase_tests/mod.rs +++ b/crates/emmylua_code_analysis/src/semantic_db/phase_tests/mod.rs @@ -1,4 +1,5 @@ mod fix_tests; +mod flow_loop_memo_tests; mod module_roots_tests; mod p0_tests; mod p10_tests; diff --git a/crates/emmylua_code_analysis/src/semantic_model/flow.rs b/crates/emmylua_code_analysis/src/semantic_model/flow.rs index 8fd7eadeb..b776012f2 100644 --- a/crates/emmylua_code_analysis/src/semantic_model/flow.rs +++ b/crates/emmylua_code_analysis/src/semantic_model/flow.rs @@ -1502,9 +1502,10 @@ fn trace_decl( // a state's value depends on the node and the options alone — never on the // route. That makes the state cacheable, which matters because a nested // conditional chain otherwise re-derives the same states along exponentially - // many routes. Graphs with loops keep the uncached path (see the type note on - // `cache::TraceStateKey`). - if tree.has_complex_control() { + // many routes. Only nodes that can reach a loop backwards keep the uncached + // path (see the type note on `cache::TraceStateKey`), so a loop in one function + // does not disable memoization for the rest of the file. + if tree.is_loop_reachable(flow_id) { return trace_decl_uncached(model, decl, tree, flow_id, options, mode, visited, path); } let key = cache::TraceStateKey { From d70d4fe858ffc0aed8c663f525cfff56ea90a540 Mon Sep 17 00:00:00 2001 From: Sarah Wesker Date: Sun, 18 Oct 2026 19:05:08 -0400 Subject: [PATCH 2/3] Tidy loop reachability and test every loop kind - is_loop_reachable falls back to true for unknown nodes. - Share the antecedent walk between single and multiple predecessors, with a debug_assert on ids and the same bounds check in the BFS. - Run the linear guard-chain test with for-in, numeric for, while and repeat, and tighten its bound to 32n + 64. --- .../src/semantic_db/flow/flow_tree.rs | 56 +++++++++-------- .../phase_tests/flow_loop_memo_tests.rs | 62 ++++++++++++------- 2 files changed, 69 insertions(+), 49 deletions(-) diff --git a/crates/emmylua_code_analysis/src/semantic_db/flow/flow_tree.rs b/crates/emmylua_code_analysis/src/semantic_db/flow/flow_tree.rs index 3e431d677..e4e9d33b7 100644 --- a/crates/emmylua_code_analysis/src/semantic_db/flow/flow_tree.rs +++ b/crates/emmylua_code_analysis/src/semantic_db/flow/flow_tree.rs @@ -42,9 +42,10 @@ pub struct FlowTree { /// backward edges whose source positions are not monotonic, so the member /// assignment-position fast path is conservatively disabled. has_complex_control: bool, - /// Per node: whether a loop / label / break / continue is backward-reachable from it. - /// Nodes whose backward cone is loop-free can memoize trace states even when another - /// function in the same file has loops. + /// Per node: whether a loop / label / break / continue node can reach it by + /// following antecedent edges (computed by forward propagation from those + /// nodes). Unmarked nodes can memoize trace states even when another function + /// in the same file has loops. loop_reachable: Vec, } @@ -168,8 +169,9 @@ impl FlowTree { } } - /// Marks every node that can reach a loop / label / break / continue by walking its - /// antecedents, by propagating forward from those nodes along successor edges. + /// Marks every node that a loop / label / break / continue node can reach by + /// following antecedent edges. Computed by propagating forward from those nodes + /// along successor edges, so each node is visited once. fn compute_loop_reachable( flow_nodes: &[FlowNode], multiple_antecedents: &[Vec], @@ -179,24 +181,23 @@ impl FlowTree { let mut queue = VecDeque::new(); for node in flow_nodes { let id = node.id.0; - match &node.antecedent { - Some(FlowAntecedent::Single(prev)) => { - if let Some(list) = successors.get_mut(prev.0 as usize) { - list.push(id); - } - } - Some(FlowAntecedent::Multiple(multi_id)) => { - for prev in multiple_antecedents - .get(*multi_id as usize) - .map(|v| v.as_slice()) - .unwrap_or_default() - { - if let Some(list) = successors.get_mut(prev.0 as usize) { - list.push(id); - } - } + let antecedents: &[FlowId] = match &node.antecedent { + Some(FlowAntecedent::Single(prev)) => std::slice::from_ref(prev), + Some(FlowAntecedent::Multiple(multi_id)) => multiple_antecedents + .get(*multi_id as usize) + .map(|v| v.as_slice()) + .unwrap_or_default(), + None => &[], + }; + for prev in antecedents { + debug_assert!( + (prev.0 as usize) < flow_nodes.len(), + "flow antecedent {} is out of range", + prev.0 + ); + if let Some(list) = successors.get_mut(prev.0 as usize) { + list.push(id); } - None => {} } if matches!( node.kind, @@ -213,7 +214,10 @@ impl FlowTree { } } while let Some(id) = queue.pop_front() { - for &next in &successors[id as usize] { + let Some(next_ids) = successors.get(id as usize) else { + continue; + }; + for &next in next_ids { if let Some(slot) = reachable.get_mut(next as usize) && !*slot { @@ -338,12 +342,14 @@ impl FlowTree { self.has_complex_control } - /// Whether a loop / label / break / continue is backward-reachable from `flow_id`. + /// Whether a loop / label / break / continue node can reach `flow_id` by + /// following antecedent edges. pub fn is_loop_reachable(&self, flow_id: FlowId) -> bool { + // An unknown node stays on the conservative, uncached path. self.loop_reachable .get(flow_id.0 as usize) .copied() - .unwrap_or(self.has_complex_control) + .unwrap_or(true) } pub fn get_multi_antecedents(&self, id: u32) -> Option<&[FlowId]> { diff --git a/crates/emmylua_code_analysis/src/semantic_db/phase_tests/flow_loop_memo_tests.rs b/crates/emmylua_code_analysis/src/semantic_db/phase_tests/flow_loop_memo_tests.rs index 202a2027a..2568457f8 100644 --- a/crates/emmylua_code_analysis/src/semantic_db/phase_tests/flow_loop_memo_tests.rs +++ b/crates/emmylua_code_analysis/src/semantic_db/phase_tests/flow_loop_memo_tests.rs @@ -10,16 +10,26 @@ use rowan::TextSize; use crate::semantic_model::flow::flow_metrics; use crate::{FileId, VirtualWorkspace}; -/// `n` sequential guards in `walk`, plus an unrelated loop in `other`. -fn guard_chain_with_loop_elsewhere(n: usize) -> String { +/// Every loop statement the binder knows; each one used to turn the memo off for +/// the whole file. +const LOOP_KINDS: &[(&str, &str)] = &[ + ("for-in", "for _ in pairs(t) do end"), + ("numeric for", "for i = 1, #t do end"), + ("while", "while t do t = nil end"), + ("repeat", "repeat t = nil until not t"), +]; + +/// `n` sequential guards in `walk`, plus an unrelated `loop_stat` in `other`. +fn guard_chain_with_loop_elsewhere(n: usize, loop_stat: &str) -> String { let mut content = String::from("lastDir = nil\nwalkLock = 0\n\nfunction walk(dir)\n"); for i in 0..n { content.push_str(&format!( " if walkLock > {i} and lastDir == dir then\n walkLock = walkLock + 1\n end\n" )); } - content - .push_str(" lastDir = dir\nend\n\nfunction other(t)\n for _ in pairs(t) do end\nend\n"); + content.push_str(&format!( + " lastDir = dir\nend\n\nfunction other(t)\n {loop_stat}\nend\n" + )); content } @@ -44,7 +54,7 @@ fn read_every_name(ws: &VirtualWorkspace, file_id: FileId) { #[test] fn flow_loop_reachability_is_tracked_per_node() { let mut ws = VirtualWorkspace::new(); - let source = guard_chain_with_loop_elsewhere(2) + BACKWARD_GOTO; + let source = guard_chain_with_loop_elsewhere(2, LOOP_KINDS[0].1) + BACKWARD_GOTO; let file_id = ws.def_file("walking.lua", &source); let model = ws.analysis.semantic_model(file_id); let tree = model.flow_tree().expect("flow tree"); @@ -65,27 +75,31 @@ fn flow_loop_reachability_is_tracked_per_node() { ); } -/// The number of uncached walks must stay linear in the number of guards even -/// though the file contains a loop: `16n + 3` with the per-node gate. With the -/// per-file gate it was `[(4, 631), (8, 52455), (16, 2065146), (32, 6638442)]`, so -/// the check runs per size and fails on the first one instead of walking the -/// larger chains. +/// The number of uncached walks must stay linear in the number of guards for +/// every loop kind in another function: `16n + 3` (`for`) or `16n + 4` (`while`, +/// `repeat`) with the per-node gate. With the per-file gate it was already 631 at +/// n=4 and `[(8, 52455), (16, 2065146), (32, 6638442)]` after that, so the check +/// runs per size and fails on the first one instead of walking the larger chains. #[test] fn flow_loop_in_other_function_keeps_guard_chain_linear() { - let mut measurements = Vec::new(); - for n in [4usize, 8, 16, 32] { - let mut ws = VirtualWorkspace::new(); - let file_id = ws.def_file("walking.lua", &guard_chain_with_loop_elsewhere(n)); - flow_metrics::reset(); - read_every_name(&ws, file_id); - let entries = flow_metrics::trace_decl_entries(); - measurements.push((n, entries)); + for (kind, loop_stat) in LOOP_KINDS { + let mut measurements = Vec::new(); + for n in [4usize, 8, 16, 32] { + let mut ws = VirtualWorkspace::new(); + let source = guard_chain_with_loop_elsewhere(n, loop_stat); + let file_id = ws.def_file("walking.lua", &source); + flow_metrics::reset(); + read_every_name(&ws, file_id); + let entries = flow_metrics::trace_decl_entries(); + measurements.push((n, entries)); - let bound = 200 + 60 * (n as u64); - assert!( - entries <= bound, - "the declaration walk ran {entries} times for {n} guards (bound {bound}); \ - a loop in another function disabled the trace-state memo. So far: {measurements:?}" - ); + let bound = 32 * (n as u64) + 64; + assert!( + entries <= bound, + "the declaration walk ran {entries} times for {n} guards (bound {bound}); \ + a `{kind}` loop in another function disabled the trace-state memo. \ + So far: {measurements:?}" + ); + } } } From 8152eff39ebd0484f0d252f14625af75d74386e9 Mon Sep 17 00:00:00 2001 From: Sarah Wesker Date: Sun, 18 Oct 2026 19:13:29 -0400 Subject: [PATCH 3/3] Reword test comment flagged by typos --- .../src/semantic_db/phase_tests/flow_loop_memo_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/emmylua_code_analysis/src/semantic_db/phase_tests/flow_loop_memo_tests.rs b/crates/emmylua_code_analysis/src/semantic_db/phase_tests/flow_loop_memo_tests.rs index 2568457f8..aa16866bf 100644 --- a/crates/emmylua_code_analysis/src/semantic_db/phase_tests/flow_loop_memo_tests.rs +++ b/crates/emmylua_code_analysis/src/semantic_db/phase_tests/flow_loop_memo_tests.rs @@ -2,7 +2,7 @@ //! //! One loop anywhere in a file disabled trace-state memoization for every //! function in it, so a chain of `n` guards in a loop-free function walked -//! ~2^n routes (`walking.lua` of an OTClient project never finished). +//! ~2^n routes (`walking.lua` of a real game client project never finished). use emmylua_parser::{LuaAstNode, LuaNameExpr}; use rowan::TextSize;