Skip to content

Commit 38fda6b

Browse files
migeed-zmeta-codesync[bot]
authored andcommitted
Fix false positive unbound-name for walrus operator in while condition
Summary: The while-loop condition always evaluates at least once, so any walrus target in the condition is guaranteed to be assigned after the loop. However, `setup_loop` snapshots the current flow as the loop's base *before* `ensure_expr` processes the test, so walrus-defined names only entered the loop flow and were missing from the base. When `teardown_loop` merged, it saw those names missing from the base and marked them `PossiblyUninitialized`. The fix propagates newly-defined names from the while condition into the loop's base flow after `ensure_expr`, mirroring the existing `propagate_new_flow_entries_to_fork_base` used for elif conditions. Fixes #3272 Reviewed By: grievejia Differential Revision: D103246547 fbshipit-source-id: 00560948fcd587cf98764a87e0d0da27e9a4eea4
1 parent 8c6a0c4 commit 38fda6b

3 files changed

Lines changed: 128 additions & 13 deletions

File tree

pyrefly/lib/binding/scope.rs

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,26 @@ impl Flow {
525525
}
526526
}
527527

528+
/// Copy new or newly-initialized entries from `source` into `target`.
529+
/// Entries absent from `target` are inserted; entries present but uninitialized
530+
/// are overwritten when `source` has an initialized binding.
531+
fn propagate_new_flow_entries(
532+
source: &SmallMap<Name, FlowInfo>,
533+
target: &mut SmallMap<Name, FlowInfo>,
534+
) {
535+
for (name, info) in source.iter() {
536+
if let Some(existing) = target.get(name) {
537+
if matches!(existing.initialized(), InitializedInFlow::No)
538+
&& !matches!(info.initialized(), InitializedInFlow::No)
539+
{
540+
target.insert(name.clone(), info.clone());
541+
}
542+
} else {
543+
target.insert(name.clone(), info.clone());
544+
}
545+
}
546+
}
547+
528548
/// Bound names can accumulate facet narrows from long assignment chains (e.g. huge
529549
/// literal dictionaries). Limiting how many consecutive narrows we remember keeps
530550
/// the flow graph shallow enough to avoid recursive explosions in the solver.
@@ -2142,19 +2162,21 @@ impl Scopes {
21422162
.forks
21432163
.last_mut()
21442164
.expect("propagate_new_flow_entries_to_fork_base called outside of a fork");
2145-
for (name, info) in scope.flow.info.iter() {
2146-
if let Some(existing) = fork.base.info.get(name) {
2147-
// Update the base entry if it is uninitialized but the branch
2148-
// has an initialized binding (e.g. walrus targeting `x: int`).
2149-
if matches!(existing.initialized(), InitializedInFlow::No)
2150-
&& !matches!(info.initialized(), InitializedInFlow::No)
2151-
{
2152-
fork.base.info.insert(name.clone(), info.clone());
2153-
}
2154-
} else {
2155-
fork.base.info.insert(name.clone(), info.clone());
2156-
}
2157-
}
2165+
propagate_new_flow_entries(&scope.flow.info, &mut fork.base.info);
2166+
}
2167+
2168+
/// Copy walrus-defined names from the current flow into the innermost
2169+
/// loop's base flow. The while-loop condition always evaluates at least
2170+
/// once, so any walrus target there is guaranteed to be assigned after the
2171+
/// loop. Without this propagation, `teardown_loop` sees the name only in
2172+
/// the loop flow and marks it `PossiblyUninitialized`.
2173+
pub fn propagate_new_flow_entries_to_loop_base(&mut self) {
2174+
let scope = self.current_mut();
2175+
let loop_ = scope
2176+
.loops
2177+
.last_mut()
2178+
.expect("propagate_new_flow_entries_to_loop_base called outside of a loop");
2179+
propagate_new_flow_entries(&scope.flow.info, &mut loop_.base.info);
21582180
}
21592181

21602182
/// Return the current binding index and flow style for `name`, if it exists

pyrefly/lib/binding/stmt.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -963,6 +963,9 @@ impl<'a> BindingsBuilder<'a> {
963963
// made in the loop (e.g. if we reassign the test variable).
964964
// Typecheck the test condition during solving.
965965
self.ensure_expr(&mut x.test, &mut Usage::Narrowing(None));
966+
// The while condition always evaluates at least once, so walrus
967+
// targets are guaranteed to be assigned after the loop.
968+
self.scopes.propagate_new_flow_entries_to_loop_base();
966969
let is_while_true = self.sys_info.evaluate_bool(&x.test) == Some(true);
967970
let narrow_ops = NarrowOps::from_expr(self, Some(&x.test));
968971
self.bind_narrow_ops(

pyrefly/lib/test/flow_branching.rs

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1487,6 +1487,96 @@ def f(a: int) -> int:
14871487
"#,
14881488
);
14891489

1490+
testcase!(
1491+
test_walrus_in_while_post_loop,
1492+
r#"
1493+
from typing import Callable, Any
1494+
1495+
class Cat:
1496+
def equals(self, other: Any) -> bool:
1497+
return False
1498+
1499+
def main(f: Callable[[], Cat]) -> None:
1500+
while (a := f()).equals(1):
1501+
break
1502+
print(a)
1503+
"#,
1504+
);
1505+
1506+
testcase!(
1507+
test_walrus_in_while_simple,
1508+
r#"
1509+
def f() -> int:
1510+
return 1
1511+
1512+
def main() -> None:
1513+
while (x := f()) > 0:
1514+
break
1515+
print(x)
1516+
"#,
1517+
);
1518+
1519+
testcase!(
1520+
test_walrus_in_while_with_else,
1521+
r#"
1522+
def f() -> int:
1523+
return 1
1524+
1525+
def main() -> None:
1526+
while (x := f()) > 0:
1527+
pass
1528+
else:
1529+
pass
1530+
print(x)
1531+
"#,
1532+
);
1533+
1534+
testcase!(
1535+
test_walrus_in_while_pre_declared_uninitialized,
1536+
r#"
1537+
def f() -> int:
1538+
return 1
1539+
1540+
def main() -> None:
1541+
x: int
1542+
while (x := f()) > 0:
1543+
break
1544+
print(x)
1545+
"#,
1546+
);
1547+
1548+
testcase!(
1549+
bug = "walrus in while overwrites pre-bound type instead of narrowing; yields str | int",
1550+
test_walrus_in_while_pre_bound_type_precision,
1551+
r#"
1552+
from typing import assert_type
1553+
1554+
def f_int() -> int:
1555+
return 1
1556+
1557+
def main() -> None:
1558+
x = ""
1559+
while (x := f_int()) > 0:
1560+
break
1561+
assert_type(x, int) # E: assert_type(Literal[''] | int, int) failed
1562+
"#,
1563+
);
1564+
1565+
testcase!(
1566+
bug =
1567+
"BoolOp laxness causes false negative for walrus in while short-circuit context, see #1251",
1568+
test_walrus_in_while_bool_op,
1569+
r#"
1570+
def cond() -> bool: ...
1571+
def get() -> int: ...
1572+
1573+
def main() -> None:
1574+
while cond() and (x := get()):
1575+
break
1576+
print(x)
1577+
"#,
1578+
);
1579+
14901580
testcase!(
14911581
test_trycatch_implicit_return,
14921582
r#"

0 commit comments

Comments
 (0)