Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion crates/emmylua_code_analysis/src/compilation/test/flow.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#[cfg(test)]
mod test {
use crate::{DiagnosticCode, LuaType, VirtualWorkspace};
use crate::{DiagnosticCode, EmmyrcLuaVersion, LuaType, VirtualWorkspace};
use emmylua_parser::{LuaAstToken, LuaLocalName};
use ntest::timeout;

Expand Down Expand Up @@ -1475,6 +1475,30 @@ end
assert_eq!(ws.expr_ty("after_loop"), ws.ty("string?"));
}

#[test]
#[timeout(5000)]
fn test_unreachable_continue_loop_after_return_does_not_cycle() {
let mut ws = VirtualWorkspace::new();
let mut emmyrc = ws.get_emmyrc();
emmyrc.runtime.version = EmmyrcLuaVersion::LuaJITExt;
ws.update_emmyrc(emmyrc);

ws.def(
r#"
local side_effect = 0
return boolean

local continue_counter = 0
for i = 1, 5 do
if i % 2 == 0 then
continue
end
continue_counter = continue_counter + 1
end
"#,
);
}

#[test]
fn test_for_in_loop_post_flow_keeps_incoming_type_after_break() {
let mut ws = VirtualWorkspace::new_with_init_std_lib();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1322,7 +1322,18 @@ impl<'a> FlowTypeEngine<'a> {
// produces a final type, needs another query first, or reaches a saved
// continuation point like a branch merge.
fn evaluate_walk(&mut self, mut walk: QueryWalk) -> Result<SchedulerStep, InferFailReason> {
// Most walks are acyclic, so only start tracking after loop control can
// feed a later flow node back into an earlier loop label.
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));
}
Comment on lines +1327 to +1335

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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);
}


// Replays can suspend a query and later revisit an older antecedent
// that another query already finished. Use that cached type instead
// of walking back through the same replay chain again.
Expand Down Expand Up @@ -1356,6 +1367,15 @@ impl<'a> FlowTypeEngine<'a> {
| FlowNodeKind::Continue
| FlowNodeKind::Return
| FlowNodeKind::ForIStat(_) => {
if matches!(
&flow_node.kind,
FlowNodeKind::LoopLabel | FlowNodeKind::Continue
) && visited_flow_ids.is_none()
{
let mut visited = HashSet::new();
visited.insert(walk.antecedent_flow_id);
visited_flow_ids = Some(visited);
Comment on lines +1375 to +1377

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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);

}
walk.antecedent_flow_id = get_single_antecedent(flow_node)?;
}
FlowNodeKind::BranchLabel | FlowNodeKind::NamedLabel(_) => {
Expand Down
Loading