fix(flow): stop cyclic walks after unreachable continue

[lewis6991]

Problem

A recovered top-level return can leave following loop code unreachable. If that unreachable loop contains continue, flow binding can create a closed loop component and the backward flow walker can revisit the same flow ids forever.

Fixes #1146

Solution

• lazily track visited flow ids once loop control can feed a walk back to an earlier loop label
• fall back to the declared reference type when a single walk revisits a flow node
• add a LuaJIT-Ext regression test for unreachable continue-after-return code

Tests

• cargo test -p emmylua_code_analysis test_unreachable_continue_loop_after_return_does_not_cycle -- --nocapture
• cargo test -p emmylua_code_analysis flow
• cargo fmt --all --check
• git diff --check

[github-actions]

Code Review: Potential Issues and Recommendations

1. Potential Infinite Loop Risk (Critical)

File: get_type_at_flow.rs
Issue: The new cycle detection uses visited_flow_ids to break loops, but the condition !visited_flow_ids.insert(walk.antecedent_flow_id) will trigger on the second visit to any flow ID, even if the walk hasn't actually cycled back to a previously visited state. This could prematurely terminate valid walks that revisit the same flow node through different paths.

Recommendation: Consider tracking the full path or using a more robust cycle detection mechanism that distinguishes between revisiting a node in the same walk path vs. revisiting it through a different branch.

2. Inconsistent Initialization Logic

File: get_type_at_flow.rs (lines 1370-1375)
Issue: visited_flow_ids is initialized only for LoopLabel and Continue nodes, but the cycle check at line 1328 runs for all flow nodes. This means:

• Non-loop nodes will never have cycle detection
• The initialization check visited_flow_ids.is_none() is redundant since it's always None for non-loop nodes

Recommendation: Either:

• Move the cycle check inside the LoopLabel/Continue branch
• Or initialize visited_flow_ids for all node types if cycle detection is needed universally

3. Missing Error Handling for Cycle Detection

File: get_type_at_flow.rs (line 1330)
Issue: When a cycle is detected, the code returns the current variable type. This might mask genuine type inference errors that occur during the cycle.

Recommendation: Consider logging a warning or returning a more specific error type when cycles are detected, to aid debugging.

4. Test Coverage Gap

File: flow.rs (test function)
Issue: The test only verifies that the code doesn't hang (via timeout), but doesn't assert any specific type inference result. This makes it a "smoke test" that could pass even if the cycle detection produces incorrect results.

Recommendation: Add assertions to verify the expected type of continue_counter or other variables after the unreachable code.

5. Potential Performance Impact

File: get_type_at_flow.rs
Issue: Creating a new HashSet for each loop walk could have performance implications for deeply nested loops or large flow graphs.

Recommendation: Consider using a fixed-size bit vector or bloom filter if performance becomes a concern, especially for hot paths.

6. Code Style: Unnecessary mut on walk

File: get_type_at_flow.rs (line 1325)
Issue: The walk parameter is already mutable via mut walk, but the finish_walk call at line 1331 consumes it. This is fine, but the mut could be removed if walk is only used immutably after the cycle check.

Recommendation: Remove mut from the parameter if walk is not mutated after the cycle check.

Summary

The changes address a real issue (infinite loops from unreachable continue statements), but the implementation has several edge cases and potential correctness issues. The most critical concern is the premature cycle detection that could break valid walks. Consider a more targeted approach that only activates cycle detection when actually following a loop back-edge.

[gemini-code-assist]

Code Review

This pull request introduces a cycle detection mechanism in evaluate_walk to prevent infinite loops when walking through flow nodes, specifically targeting loop control nodes like LoopLabel and Continue. A test case has also been added to verify this behavior. The review feedback suggests optimizing the cycle detection by replacing the HashSet<FlowId> with a Vec<FlowId> to avoid hashing overhead and reduce heap allocation complexity, since the number of visited flow IDs is typically very small.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Since the number of visited flow IDs in a loop cycle is typically very small (usually fewer than 5), using a Vec<FlowId> with a linear search (.contains()) is more efficient than a HashSet. It avoids the hashing overhead and reduces heap allocation complexity.

Please also update the initialization of visited_flow_ids around line 1375 to use Vec.

Suggested change

	let mut visited_flow_ids = None::<HashSet<FlowId>>;
	loop {
	if let Some(visited_flow_ids) = &mut visited_flow_ids
	&& !visited_flow_ids.insert(walk.antecedent_flow_id)
	{
	let result_type = get_var_ref_type(self.db, self.cache, &walk.query.var_ref_id)?;
	return Ok(self.finish_walk(walk, result_type));
	}
	let mut visited_flow_ids = None::<Vec<FlowId>>;
	loop {
	if let Some(visited_flow_ids) = &mut visited_flow_ids {
	if visited_flow_ids.contains(&walk.antecedent_flow_id) {
	let result_type = get_var_ref_type(self.db, self.cache, &walk.query.var_ref_id)?;
	return Ok(self.finish_walk(walk, result_type));
	}
	visited_flow_ids.push(walk.antecedent_flow_id);
	}

[gemini-code-assist]

Update the initialization of visited_flow_ids to use Vec to match the change to Vec<FlowId>.

Suggested change

	let mut visited = HashSet::new();
	visited.insert(walk.antecedent_flow_id);
	visited_flow_ids = Some(visited);
	let mut visited = Vec::new();
	visited.push(walk.antecedent_flow_id);
	visited_flow_ids = Some(visited);