diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md new file mode 100644 index 0000000000000..61b914588930e --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -0,0 +1,134 @@ +# Terminal statements + +## Introduction + +Terminal statements complicate a naive control-flow analysis. + +As a simple example: + +```py +def f(cond: bool) -> str: + if cond: + x = "test" + else: + raise ValueError + return x + +def g(cond: bool): + if cond: + x = "test" + reveal_type(x) # revealed: Literal["test"] + else: + x = "unreachable" + reveal_type(x) # revealed: Literal["unreachable"] + raise ValueError + reveal_type(x) # revealed: Literal["test"] +``` + +In `f`, we should be able to determine that the `else` branch ends in a terminal statement, and that +the `return` statement can only be executed when the condition is true. We should therefore consider +the reference always bound, even though `x` is only bound in the true branch. + +Similarly, in `g`, we should see that the assignment of the value `"unreachable"` can never be seen +by the final `reveal_type`. + +## `return` is terminal + +```py +def f(cond: bool) -> str: + if cond: + x = "test" + else: + return "early" + return x + +def g(cond: bool): + if cond: + x = "test" + reveal_type(x) # revealed: Literal["test"] + else: + x = "unreachable" + reveal_type(x) # revealed: Literal["unreachable"] + return + reveal_type(x) # revealed: Literal["test"] +``` + +## `continue` is terminal within its loop scope + +```py +def f(cond: bool) -> str: + while True: + if cond: + x = "test" + else: + continue + return x + +def g(cond: bool, i: int): + x = "before" + while i < 5: + if cond: + x = "loop" + reveal_type(x) # revealed: Literal["loop"] + else: + x = "continue" + reveal_type(x) # revealed: Literal["continue"] + continue + reveal_type(x) # revealed: Literal["loop"] + reveal_type(x) # revealed: Literal["before", "loop"] +``` + +## `break` is terminal within its loop scope + +```py +def f(cond: bool) -> str: + while True: + if cond: + x = "test" + else: + break + return x + return "late" + +def g(cond: bool, i: int): + x = "before" + while i < 5: + if cond: + x = "loop" + reveal_type(x) # revealed: Literal["loop"] + else: + x = "break" + reveal_type(x) # revealed: Literal["break"] + break + reveal_type(x) # revealed: Literal["loop"] + reveal_type(x) # revealed: Literal["before", "loop", "break"] +``` + +## `return` is terminal in nested scopes + +```py +def f(cond1: bool, cond2: bool) -> str: + if cond1: + if cond2: + x = "test1" + else: + return "early" + else: + x = "test2" + return x + +def g(cond1: bool, cond2: bool): + if cond1: + if cond2: + x = "test1" + reveal_type(x) # revealed: Literal["test1"] + else: + x = "unreachable" + reveal_type(x) # revealed: Literal["unreachable"] + return + reveal_type(x) # revealed: Literal["test1"] + else: + x = "test2" + reveal_type(x) # revealed: Literal["test2"] + reveal_type(x) # revealed: Literal["test1", "test2"] +``` diff --git a/crates/red_knot_python_semantic/src/semantic_index/builder.rs b/crates/red_knot_python_semantic/src/semantic_index/builder.rs index 89a9c0a0cfd32..8b8a6b56a3e50 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/builder.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/builder.rs @@ -368,6 +368,12 @@ impl<'db> SemanticIndexBuilder<'db> { .record_visibility_constraint(VisibilityConstraint::VisibleIf(constraint)) } + /// Records that all remaining statements in the current block are unreachable, and therefore + /// not visible. + fn mark_unreachable(&mut self) { + self.current_use_def_map_mut().mark_unreachable(); + } + /// Records a [`VisibilityConstraint::Ambiguous`] constraint. fn record_ambiguous_visibility(&mut self) -> ScopedVisibilityConstraintId { self.current_use_def_map_mut() @@ -1019,11 +1025,6 @@ where } self.visit_body(body); } - ast::Stmt::Break(_) => { - if self.loop_state().is_inside() { - self.loop_break_states.push(self.flow_snapshot()); - } - } ast::Stmt::For( for_stmt @ ast::StmtFor { @@ -1270,6 +1271,21 @@ where // - https://github.com/astral-sh/ruff/pull/13633#discussion_r1788626702 self.visit_body(finalbody); } + + ast::Stmt::Raise(_) | ast::Stmt::Return(_) | ast::Stmt::Continue(_) => { + walk_stmt(self, stmt); + // Everything in the current block after a terminal statement is unreachable. + self.mark_unreachable(); + } + + ast::Stmt::Break(_) => { + if self.loop_state().is_inside() { + self.loop_break_states.push(self.flow_snapshot()); + } + // Everything in the current block after a terminal statement is unreachable. + self.mark_unreachable(); + } + _ => { walk_stmt(self, stmt); } diff --git a/crates/red_knot_python_semantic/src/semantic_index/use_def.rs b/crates/red_knot_python_semantic/src/semantic_index/use_def.rs index 7ad1703bfa3a1..efe3d1857e32a 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/use_def.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/use_def.rs @@ -476,6 +476,7 @@ impl std::iter::FusedIterator for DeclarationsIterator<'_, '_> {} pub(super) struct FlowSnapshot { symbol_states: IndexVec, scope_start_visibility: ScopedVisibilityConstraintId, + reachable: bool, } #[derive(Debug)] @@ -503,6 +504,8 @@ pub(super) struct UseDefMapBuilder<'db> { /// Currently live bindings and declarations for each symbol. symbol_states: IndexVec, + + reachable: bool, } impl Default for UseDefMapBuilder<'_> { @@ -515,11 +518,22 @@ impl Default for UseDefMapBuilder<'_> { bindings_by_use: IndexVec::new(), definitions_by_definition: FxHashMap::default(), symbol_states: IndexVec::new(), + reachable: true, } } } impl<'db> UseDefMapBuilder<'db> { + pub(super) fn mark_unreachable(&mut self) { + self.reachable = false; + let num_symbols = self.symbol_states.len(); + self.symbol_states.truncate(0); + self.symbol_states.resize( + num_symbols, + SymbolState::undefined(ScopedVisibilityConstraintId::ALWAYS_FALSE), + ); + } + pub(super) fn add_symbol(&mut self, symbol: ScopedSymbolId) { let new_symbol = self .symbol_states @@ -656,6 +670,7 @@ impl<'db> UseDefMapBuilder<'db> { FlowSnapshot { symbol_states: self.symbol_states.clone(), scope_start_visibility: self.scope_start_visibility, + reachable: self.reachable, } } @@ -678,6 +693,8 @@ impl<'db> UseDefMapBuilder<'db> { num_symbols, SymbolState::undefined(self.scope_start_visibility), ); + + self.reachable = snapshot.reachable; } /// Merge the given snapshot into the current state, reflecting that we might have taken either @@ -689,6 +706,11 @@ impl<'db> UseDefMapBuilder<'db> { // greater than the number of known symbols in a previously-taken snapshot. debug_assert!(self.symbol_states.len() >= snapshot.symbol_states.len()); + self.reachable |= snapshot.reachable; + if !snapshot.reachable { + return; + } + let mut snapshot_definitions_iter = snapshot.symbol_states.into_iter(); for current in &mut self.symbol_states { if let Some(snapshot) = snapshot_definitions_iter.next() { diff --git a/crates/red_knot_python_semantic/src/semantic_index/use_def/symbol_state.rs b/crates/red_knot_python_semantic/src/semantic_index/use_def/symbol_state.rs index 5a5a06c5b0bff..ccba2d47adb57 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/use_def/symbol_state.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/use_def/symbol_state.rs @@ -108,6 +108,12 @@ impl ScopedVisibilityConstraintId { /// present at index 0. pub(crate) const ALWAYS_TRUE: ScopedVisibilityConstraintId = ScopedVisibilityConstraintId::from_u32(0); + + /// A special ID that is used for an "always false" / "never visible" constraint. + /// When we create a new [`VisibilityConstraints`] object, this constraint is always + /// present at index 1. + pub(crate) const ALWAYS_FALSE: ScopedVisibilityConstraintId = + ScopedVisibilityConstraintId::from_u32(1); } const INLINE_VISIBILITY_CONSTRAINTS: usize = 4; diff --git a/crates/red_knot_python_semantic/src/visibility_constraints.rs b/crates/red_knot_python_semantic/src/visibility_constraints.rs index f19144fb5e072..73e7cabbdc71d 100644 --- a/crates/red_knot_python_semantic/src/visibility_constraints.rs +++ b/crates/red_knot_python_semantic/src/visibility_constraints.rs @@ -186,7 +186,10 @@ pub(crate) struct VisibilityConstraints<'db> { impl Default for VisibilityConstraints<'_> { fn default() -> Self { Self { - constraints: IndexVec::from_iter([VisibilityConstraint::AlwaysTrue]), + constraints: IndexVec::from_iter([ + VisibilityConstraint::AlwaysTrue, + VisibilityConstraint::VisibleIfNot(ScopedVisibilityConstraintId::ALWAYS_TRUE), + ]), } } } diff --git a/crates/ruff_benchmark/benches/red_knot.rs b/crates/ruff_benchmark/benches/red_knot.rs index 255e89d92af60..b791b642102db 100644 --- a/crates/ruff_benchmark/benches/red_knot.rs +++ b/crates/ruff_benchmark/benches/red_knot.rs @@ -26,22 +26,6 @@ const TOMLLIB_312_URL: &str = "https://raw.githubusercontent.com/python/cpython/ static EXPECTED_DIAGNOSTICS: &[&str] = &[ // We don't support `*` imports yet: "error[lint:unresolved-import] /src/tomllib/_parser.py:7:29 Module `collections.abc` has no member `Iterable`", - // We don't support terminal statements in control flow yet: - "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:66:18 Name `s` used when possibly not defined", - "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:98:12 Name `char` used when possibly not defined", - "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:101:12 Name `char` used when possibly not defined", - "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:104:14 Name `char` used when possibly not defined", - "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:115:14 Name `char` used when possibly not defined", - "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:126:12 Name `char` used when possibly not defined", - "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:348:20 Name `nest` used when possibly not defined", - "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:353:5 Name `nest` used when possibly not defined", - "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:453:24 Name `nest` used when possibly not defined", - "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:455:9 Name `nest` used when possibly not defined", - "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:482:16 Name `char` used when possibly not defined", - "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:566:12 Name `char` used when possibly not defined", - "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:573:12 Name `char` used when possibly not defined", - "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:579:12 Name `char` used when possibly not defined", - "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:580:63 Name `char` used when possibly not defined", // We don't handle intersections in `is_assignable_to` yet "error[lint:invalid-argument-type] /src/tomllib/_parser.py:626:46 Object of type `@Todo & ~AlwaysFalsy` cannot be assigned to parameter 1 (`match`) of function `match_to_datetime`; expected type `Match`", "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:629:38 Name `datetime_obj` used when possibly not defined",