Skip to content

Commit 94f34c8

Browse files
committed
fix(flow): separate dynamic loop divergence from infinite loops
Track runtime-dependent loop divergence separately from proven infinite loop bodies in return-flow analysis. Keep dynamic while and repeat loops fallthrough-friendly for MissingReturn when a later explicit return is still reachable, while preserving the stronger infinite-loop signal so loops with a proven non-terminating body still block that later return.
1 parent 3530d81 commit 94f34c8

6 files changed

Lines changed: 151 additions & 39 deletions

File tree

crates/emmylua_code_analysis/src/compilation/analyzer/lua/func_body.rs

Lines changed: 63 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -24,25 +24,32 @@ enum ConditionState {
2424
#[derive(Debug, Default)]
2525
struct ReturnFlow {
2626
return_points: Vec<LuaReturnPoint>,
27-
can_continue: bool,
27+
can_fall_through: bool,
2828
can_break: bool,
29-
// `can_stall` means control can get stuck without returning, e.g. `while true do end`.
30-
can_stall: bool,
29+
// Some reachable path is proven to loop forever instead of returning or
30+
// reaching the following statements, e.g. `while true do end`.
31+
is_infinite: bool,
32+
// Some reachable path can stay inside a runtime-dependent loop without
33+
// proving the body itself is infinite. Keep this separate from
34+
// reachability so stricter loop diagnostics can opt into it later without
35+
// affecting inferred return types.
36+
may_diverge: bool,
3137
}
3238

3339
impl ReturnFlow {
34-
fn continue_flow() -> Self {
40+
fn fallthrough() -> Self {
3541
Self {
36-
can_continue: true,
42+
can_fall_through: true,
3743
..Default::default()
3844
}
3945
}
4046

4147
fn merge_choice(&mut self, mut other: Self) {
4248
self.return_points.append(&mut other.return_points);
43-
self.can_continue |= other.can_continue;
49+
self.can_fall_through |= other.can_fall_through;
4450
self.can_break |= other.can_break;
45-
self.can_stall |= other.can_stall;
51+
self.is_infinite |= other.is_infinite;
52+
self.may_diverge |= other.may_diverge;
4653
}
4754
}
4855

@@ -54,22 +61,25 @@ where
5461
F: FnMut(&LuaExpr) -> Result<LuaType, InferFailReason>,
5562
{
5663
let mut flow = analyze_block_returns(body, infer_expr_type)?;
57-
if flow.can_continue || flow.can_break {
64+
if flow.can_fall_through || flow.can_break {
5865
flow.return_points.push(LuaReturnPoint::Nil);
5966
}
6067

6168
Ok(flow.return_points)
6269
}
6370

64-
pub fn does_func_body_always_return_or_exit<F>(
71+
pub fn does_func_body_satisfy_missing_return<F>(
6572
body: LuaBlock,
6673
infer_expr_type: &mut F,
6774
) -> Result<bool, InferFailReason>
6875
where
6976
F: FnMut(&LuaExpr) -> Result<LuaType, InferFailReason>,
7077
{
7178
let flow = analyze_block_returns(body, infer_expr_type)?;
72-
Ok(!flow.can_continue && !flow.can_break && !flow.can_stall)
79+
// `MissingReturn` currently ignores runtime-dependent divergence if a
80+
// later `return` is still reachable. We keep `may_diverge` around so that
81+
// decision stays local to this check.
82+
Ok(!flow.can_fall_through && !flow.can_break && !flow.is_infinite)
7383
}
7484

7585
fn analyze_block_returns<F>(
@@ -80,21 +90,22 @@ where
8090
F: FnMut(&LuaExpr) -> Result<LuaType, InferFailReason>,
8191
{
8292
let mut flow = ReturnFlow::default();
83-
let mut can_continue = true;
93+
let mut can_fall_through = true;
8494

8595
for stat in block.get_stats() {
86-
if !can_continue {
96+
if !can_fall_through {
8797
break;
8898
}
8999

90100
let stat_flow = analyze_stat_returns(stat, infer_expr_type)?;
91101
flow.return_points.extend(stat_flow.return_points);
92102
flow.can_break |= stat_flow.can_break;
93-
flow.can_stall |= stat_flow.can_stall;
94-
can_continue = stat_flow.can_continue;
103+
flow.is_infinite |= stat_flow.is_infinite;
104+
flow.may_diverge |= stat_flow.may_diverge;
105+
can_fall_through = stat_flow.can_fall_through;
95106
}
96107

97-
flow.can_continue = can_continue;
108+
flow.can_fall_through = can_fall_through;
98109
Ok(flow)
99110
}
100111

@@ -107,7 +118,7 @@ where
107118
{
108119
match block {
109120
Some(block) => analyze_block_returns(block, infer_expr_type),
110-
None => Ok(ReturnFlow::continue_flow()),
121+
None => Ok(ReturnFlow::fallthrough()),
111122
}
112123
}
113124

@@ -135,7 +146,7 @@ where
135146
..Default::default()
136147
}),
137148
LuaStat::ReturnStat(return_stat) => Ok(analyze_return_stat_returns(return_stat)),
138-
_ => Ok(ReturnFlow::continue_flow()),
149+
_ => Ok(ReturnFlow::fallthrough()),
139150
}
140151
}
141152

@@ -159,15 +170,31 @@ where
159170
let condition_state =
160171
analyze_condition_state(while_stat.get_condition_expr(), infer_expr_type)?;
161172
match condition_state {
162-
ConditionState::Falsy => Ok(ReturnFlow::continue_flow()),
173+
ConditionState::Falsy => Ok(ReturnFlow::fallthrough()),
163174
ConditionState::Never => Ok(ReturnFlow::default()),
164-
ConditionState::Truthy | ConditionState::Dynamic => {
175+
ConditionState::Truthy => {
176+
let body = analyze_optional_block_returns(while_stat.get_block(), infer_expr_type)?;
177+
Ok(ReturnFlow {
178+
return_points: body.return_points,
179+
can_fall_through: body.can_break,
180+
can_break: false,
181+
is_infinite: body.can_fall_through || body.is_infinite,
182+
may_diverge: body.may_diverge,
183+
})
184+
}
185+
ConditionState::Dynamic => {
165186
let body = analyze_optional_block_returns(while_stat.get_block(), infer_expr_type)?;
166187
Ok(ReturnFlow {
167188
return_points: body.return_points,
168-
can_continue: matches!(condition_state, ConditionState::Dynamic) || body.can_break,
189+
can_fall_through: true,
169190
can_break: false,
170-
can_stall: body.can_continue || body.can_stall,
191+
// A dynamic condition may skip the loop entirely, but a body
192+
// that is already proven infinite remains infinite once
193+
// entered.
194+
is_infinite: body.is_infinite,
195+
// We keep dynamic loops in `may_diverge` rather than trying to
196+
// prove exit from body progress here.
197+
may_diverge: body.can_fall_through || body.may_diverge,
171198
})
172199
}
173200
}
@@ -183,25 +210,28 @@ where
183210
let body = analyze_optional_block_returns(repeat_stat.get_block(), infer_expr_type)?;
184211
let mut flow = ReturnFlow {
185212
return_points: body.return_points,
186-
can_continue: body.can_break,
213+
can_fall_through: body.can_break,
187214
can_break: false,
188-
can_stall: body.can_stall,
215+
is_infinite: body.is_infinite,
216+
may_diverge: body.may_diverge,
189217
};
190218

191-
if !body.can_continue {
219+
if !body.can_fall_through {
192220
return Ok(flow);
193221
}
194222

195223
match analyze_condition_state(repeat_stat.get_condition_expr(), infer_expr_type)? {
196224
ConditionState::Truthy => {
197-
flow.can_continue = true;
225+
flow.can_fall_through = true;
198226
}
199227
ConditionState::Falsy => {
200-
flow.can_stall = true;
228+
flow.is_infinite = true;
201229
}
202230
ConditionState::Dynamic => {
203-
flow.can_continue = true;
204-
flow.can_stall = true;
231+
flow.can_fall_through = true;
232+
// Same rule as dynamic `while`: the loop body is reachable, but
233+
// the exit still depends on runtime state.
234+
flow.may_diverge = true;
205235
}
206236
ConditionState::Never => {}
207237
}
@@ -217,7 +247,7 @@ where
217247
F: FnMut(&LuaExpr) -> Result<LuaType, InferFailReason>,
218248
{
219249
let mut flow = analyze_optional_block_returns(for_stat.get_block(), infer_expr_type)?;
220-
flow.can_continue = true;
250+
flow.can_fall_through = true;
221251
flow.can_break = false;
222252
Ok(flow)
223253
}
@@ -286,7 +316,7 @@ where
286316
}
287317

288318
if can_reach_next_clause {
289-
flow.can_continue = true;
319+
flow.can_fall_through = true;
290320
}
291321

292322
Ok(flow)
@@ -300,7 +330,7 @@ where
300330
F: FnMut(&LuaExpr) -> Result<LuaType, InferFailReason>,
301331
{
302332
let mut flow = analyze_optional_block_returns(for_range_stat.get_block(), infer_expr_type)?;
303-
flow.can_continue = true;
333+
flow.can_fall_through = true;
304334
flow.can_break = false;
305335
Ok(flow)
306336
}
@@ -309,7 +339,7 @@ fn analyze_call_expr_stat_returns(
309339
call_expr_stat: LuaCallExprStat,
310340
) -> Result<ReturnFlow, InferFailReason> {
311341
let Some(call_expr) = call_expr_stat.get_call_expr() else {
312-
return Ok(ReturnFlow::continue_flow());
342+
return Ok(ReturnFlow::fallthrough());
313343
};
314344

315345
if call_expr.is_error() {
@@ -319,7 +349,7 @@ fn analyze_call_expr_stat_returns(
319349
});
320350
}
321351

322-
Ok(ReturnFlow::continue_flow())
352+
Ok(ReturnFlow::fallthrough())
323353
}
324354

325355
fn analyze_return_stat_returns(return_stat: LuaReturnStat) -> ReturnFlow {

crates/emmylua_code_analysis/src/compilation/analyzer/lua/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use emmylua_parser::{LuaAst, LuaAstNode, LuaExpr};
1414
use for_range_stat::analyze_for_range_stat;
1515
pub use for_range_stat::infer_for_range_iter_expr_func;
1616
pub use func_body::{
17-
LuaReturnPoint, analyze_func_body_returns_with, does_func_body_always_return_or_exit,
17+
LuaReturnPoint, analyze_func_body_returns_with, does_func_body_satisfy_missing_return,
1818
};
1919
use metatable::analyze_setmetatable;
2020
use module::analyze_chunk_return;

crates/emmylua_code_analysis/src/compilation/analyzer/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ mod infer_cache_manager;
66
mod lua;
77
mod unresolve;
88

9-
pub(crate) use lua::does_func_body_always_return_or_exit;
9+
pub(crate) use lua::does_func_body_satisfy_missing_return;
1010

1111
use crate::{
1212
Emmyrc, FileId, InFiled, InferFailReason,

crates/emmylua_code_analysis/src/compilation/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ mod test;
33

44
use std::sync::Arc;
55

6-
pub(crate) use analyzer::does_func_body_always_return_or_exit;
6+
pub(crate) use analyzer::does_func_body_satisfy_missing_return;
77

88
use crate::{
99
Emmyrc, FileId, InFiled, LuaIndex, LuaInferCache, db_index::DbIndex, semantic::SemanticModel,

crates/emmylua_code_analysis/src/diagnostic/checker/check_return_count.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use emmylua_parser::{
55

66
use crate::{
77
DiagnosticCode, LuaSignatureId, LuaType, SemanticModel, SignatureReturnStatus,
8-
compilation::does_func_body_always_return_or_exit,
8+
compilation::does_func_body_satisfy_missing_return,
99
};
1010

1111
use super::{Checker, DiagnosticContext, get_return_stats};
@@ -111,7 +111,7 @@ fn check_missing_return(
111111
// 检测缺少返回语句需要处理 if while
112112
if min_expected_return_count > 0 {
113113
let range = if let Some(block) = closure_expr.get_block() {
114-
if does_func_body_always_return_or_exit(block.clone(), &mut |expr: &LuaExpr| {
114+
if does_func_body_satisfy_missing_return(block.clone(), &mut |expr: &LuaExpr| {
115115
Ok(semantic_model
116116
.infer_expr(expr.clone())
117117
.unwrap_or(LuaType::Unknown))

crates/emmylua_code_analysis/src/diagnostic/test/check_return_count_test.rs

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ mod tests {
277277
"#
278278
));
279279

280-
assert!(!ws.check_code_for(
280+
assert!(ws.check_code_for(
281281
DiagnosticCode::MissingReturn,
282282
r#"
283283
---@return number
@@ -662,6 +662,88 @@ mod tests {
662662
);
663663
}
664664

665+
#[test]
666+
fn test_missing_return_accepts_dynamic_parent_walk_before_return() {
667+
// This matches the current `MissingReturn` check: runtime-dependent
668+
// loops are allowed when the function still reaches a later `return`.
669+
assert_missing_return_ok(
670+
r#"
671+
---@class Node
672+
local Node = {}
673+
674+
---@return Node?
675+
function Node:parent()
676+
return nil
677+
end
678+
679+
---@param node Node?
680+
---@return integer
681+
local function get_indent(node)
682+
local indent = 0
683+
684+
while node do
685+
node = node:parent()
686+
end
687+
688+
return indent
689+
end
690+
"#,
691+
);
692+
}
693+
694+
#[test]
695+
fn test_missing_return_accepts_dynamic_repeat_before_return() {
696+
assert_missing_return_ok(
697+
r#"
698+
---@return number
699+
local function foo(done)
700+
repeat
701+
until done
702+
703+
return 1
704+
end
705+
"#,
706+
);
707+
}
708+
709+
#[test]
710+
fn test_missing_return_rejects_dynamic_while_with_infinite_body_before_return() {
711+
assert_missing_return_error(
712+
r#"
713+
---@return number
714+
local function foo(a)
715+
while a do
716+
while true do
717+
end
718+
end
719+
720+
return 1
721+
end
722+
"#,
723+
);
724+
}
725+
726+
#[test]
727+
fn test_missing_return_rejects_dynamic_while_with_break_or_infinite_body_before_return() {
728+
assert_missing_return_error(
729+
r#"
730+
---@return number
731+
local function foo(a, b)
732+
while a do
733+
if b then
734+
break
735+
end
736+
737+
while true do
738+
end
739+
end
740+
741+
return 1
742+
end
743+
"#,
744+
);
745+
}
746+
665747
#[test]
666748
fn test_missing_return_rejects_stalling_numeric_for_before_return() {
667749
assert_missing_return_error(

0 commit comments

Comments
 (0)