From e0d213080ba682b76d722b17f11a9c9c9e0342a3 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Wed, 22 Jan 2025 11:43:02 -0500 Subject: [PATCH 01/33] Add failing test cases for terminal control flow --- .../resources/mdtest/terminal_statements.md | 134 ++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md 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 00000000000000..86b4dd22878c6a --- /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 + # TODO: no error + # error: [possibly-unresolved-reference] + return x + +def g(cond: bool): + if cond: + x = "test" + else: + x = "unreachable" + raise ValueError + # TODO: Literal["test"] + reveal_type(x) # revealed: Literal["test", "unreachable"] +``` + +In `f`, we should be able to determine that the `else` ends in a terminal statement, and that the +`return` statement can only be executed when the condition is true. Even though `x` is only bound in +the true branch, we should therefore consider the reference always bound. + +Similarly, in `g`, we should see that the assignment of the value `"unreachable"` can never be seen +by the `reveal_type`. + +## `return` is terminal + +```py +def f(cond: bool) -> str: + if cond: + x = "test" + else: + return "early" + # TODO: no error + # error: [possibly-unresolved-reference] + return x + +def g(cond: bool): + if cond: + x = "test" + else: + x = "unreachable" + return + # TODO: Literal["test"] + reveal_type(x) # revealed: Literal["test", "unreachable"] +``` + +## `continue` is terminal within its loop scope + +```py +def f(cond: bool) -> str: + while True: + if cond: + x = "test" + else: + continue + # TODO: no error + # error: [possibly-unresolved-reference] + return x + +def g(cond: bool): + while True: + if cond: + x = "test" + else: + x = "unreachable" + continue + # TODO: Literal["test"] + reveal_type(x) # revealed: Literal["test", "unreachable"] +``` + +## `break` is terminal within its loop scope + +```py +def f(cond: bool) -> str: + while True: + if cond: + x = "test" + else: + break + # TODO: no error + # error: [possibly-unresolved-reference] + return x + return "late" + +def g(cond: bool): + while True: + if cond: + x = "test" + else: + x = "unreachable" + break + # TODO: Literal["test"] + reveal_type(x) # revealed: Literal["test", "unreachable"] +``` + +## `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" + # TODO: no error + # error: [possibly-unresolved-reference] + return x + +def g(cond1: bool, cond2: bool): + if cond1: + if cond2: + x = "test1" + else: + x = "unreachable" + return + else: + x = "test2" + # TODO: no error + # TODO: Literal["test"] + reveal_type(x) # revealed: Literal["test1", "unreachable", "test2"] +``` From f45af246442d737ba4b4d7f610de66e45027f878 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Wed, 22 Jan 2025 16:32:35 -0500 Subject: [PATCH 02/33] Record unreachable visibility after terminal statements --- .../resources/mdtest/terminal_statements.md | 26 ++++--------------- .../src/semantic_index/builder.rs | 26 +++++++++++++++---- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 86b4dd22878c6a..4cbfec9729275d 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -12,8 +12,6 @@ def f(cond: bool) -> str: x = "test" else: raise ValueError - # TODO: no error - # error: [possibly-unresolved-reference] return x def g(cond: bool): @@ -22,8 +20,7 @@ def g(cond: bool): else: x = "unreachable" raise ValueError - # TODO: Literal["test"] - reveal_type(x) # revealed: Literal["test", "unreachable"] + reveal_type(x) # revealed: Literal["test"] ``` In `f`, we should be able to determine that the `else` ends in a terminal statement, and that the @@ -41,8 +38,6 @@ def f(cond: bool) -> str: x = "test" else: return "early" - # TODO: no error - # error: [possibly-unresolved-reference] return x def g(cond: bool): @@ -51,8 +46,7 @@ def g(cond: bool): else: x = "unreachable" return - # TODO: Literal["test"] - reveal_type(x) # revealed: Literal["test", "unreachable"] + reveal_type(x) # revealed: Literal["test"] ``` ## `continue` is terminal within its loop scope @@ -64,8 +58,6 @@ def f(cond: bool) -> str: x = "test" else: continue - # TODO: no error - # error: [possibly-unresolved-reference] return x def g(cond: bool): @@ -75,8 +67,7 @@ def g(cond: bool): else: x = "unreachable" continue - # TODO: Literal["test"] - reveal_type(x) # revealed: Literal["test", "unreachable"] + reveal_type(x) # revealed: Literal["test"] ``` ## `break` is terminal within its loop scope @@ -88,8 +79,6 @@ def f(cond: bool) -> str: x = "test" else: break - # TODO: no error - # error: [possibly-unresolved-reference] return x return "late" @@ -100,8 +89,7 @@ def g(cond: bool): else: x = "unreachable" break - # TODO: Literal["test"] - reveal_type(x) # revealed: Literal["test", "unreachable"] + reveal_type(x) # revealed: Literal["test"] ``` ## `return` is terminal in nested scopes @@ -115,8 +103,6 @@ def f(cond1: bool, cond2: bool) -> str: return "early" else: x = "test2" - # TODO: no error - # error: [possibly-unresolved-reference] return x def g(cond1: bool, cond2: bool): @@ -128,7 +114,5 @@ def g(cond1: bool, cond2: bool): return else: x = "test2" - # TODO: no error - # TODO: Literal["test"] - reveal_type(x) # revealed: Literal["test1", "unreachable", "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 89a9c0a0cfd323..452d90c06252db 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. + fn record_unreachable_visibility(&mut self) -> ScopedVisibilityConstraintId { + let id = self.add_visibility_constraint(VisibilityConstraint::AlwaysTrue); + self.record_negated_visibility_constraint(id) + } + /// 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.record_unreachable_visibility(); + } + + 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.record_unreachable_visibility(); + } + _ => { walk_stmt(self, stmt); } From 70b681bba107e98efcda70adb8445e97f1612828 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Wed, 22 Jan 2025 16:46:48 -0500 Subject: [PATCH 03/33] More thorough reveal_types --- .../resources/mdtest/terminal_statements.md | 44 +++++++++++++------ .../src/semantic_index/builder.rs | 3 +- 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 4cbfec9729275d..61b914588930e9 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -17,18 +17,20 @@ def f(cond: bool) -> str: 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` ends in a terminal statement, and that the -`return` statement can only be executed when the condition is true. Even though `x` is only bound in -the true branch, we should therefore consider the reference always bound. +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 `reveal_type`. +by the final `reveal_type`. ## `return` is terminal @@ -43,8 +45,10 @@ def f(cond: bool) -> str: 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"] ``` @@ -60,14 +64,18 @@ def f(cond: bool) -> str: continue return x -def g(cond: bool): - while True: +def g(cond: bool, i: int): + x = "before" + while i < 5: if cond: - x = "test" + x = "loop" + reveal_type(x) # revealed: Literal["loop"] else: - x = "unreachable" + x = "continue" + reveal_type(x) # revealed: Literal["continue"] continue - reveal_type(x) # revealed: Literal["test"] + reveal_type(x) # revealed: Literal["loop"] + reveal_type(x) # revealed: Literal["before", "loop"] ``` ## `break` is terminal within its loop scope @@ -82,14 +90,18 @@ def f(cond: bool) -> str: return x return "late" -def g(cond: bool): - while True: +def g(cond: bool, i: int): + x = "before" + while i < 5: if cond: - x = "test" + x = "loop" + reveal_type(x) # revealed: Literal["loop"] else: - x = "unreachable" + x = "break" + reveal_type(x) # revealed: Literal["break"] break - reveal_type(x) # revealed: Literal["test"] + reveal_type(x) # revealed: Literal["loop"] + reveal_type(x) # revealed: Literal["before", "loop", "break"] ``` ## `return` is terminal in nested scopes @@ -109,10 +121,14 @@ 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 452d90c06252db..74fb82a9253b30 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/builder.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/builder.rs @@ -368,7 +368,8 @@ impl<'db> SemanticIndexBuilder<'db> { .record_visibility_constraint(VisibilityConstraint::VisibleIf(constraint)) } - /// Records that all remaining statements in the current block are unreachable. + /// Records that all remaining statements in the current block are unreachable, and therefore + /// not visible. fn record_unreachable_visibility(&mut self) -> ScopedVisibilityConstraintId { let id = self.add_visibility_constraint(VisibilityConstraint::AlwaysTrue); self.record_negated_visibility_constraint(id) From 81a3164c37968936d2d8bcf2105517a8fce4e368 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Wed, 22 Jan 2025 17:01:24 -0500 Subject: [PATCH 04/33] Actually we do support those now! --- crates/ruff_benchmark/benches/red_knot.rs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/crates/ruff_benchmark/benches/red_knot.rs b/crates/ruff_benchmark/benches/red_knot.rs index 4c6822ed9aa10e..9d53e968e595e4 100644 --- a/crates/ruff_benchmark/benches/red_knot.rs +++ b/crates/ruff_benchmark/benches/red_knot.rs @@ -25,22 +25,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", From 912ed62e120682a88f11f40d42342b5f7bcec2a3 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Thu, 23 Jan 2025 10:55:52 -0500 Subject: [PATCH 05/33] Use happy constant --- crates/red_knot_python_semantic/src/semantic_index/builder.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 74fb82a9253b30..7b02dc2cea3f4a 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/builder.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/builder.rs @@ -371,8 +371,7 @@ impl<'db> SemanticIndexBuilder<'db> { /// Records that all remaining statements in the current block are unreachable, and therefore /// not visible. fn record_unreachable_visibility(&mut self) -> ScopedVisibilityConstraintId { - let id = self.add_visibility_constraint(VisibilityConstraint::AlwaysTrue); - self.record_negated_visibility_constraint(id) + self.record_negated_visibility_constraint(ScopedVisibilityConstraintId::ALWAYS_TRUE) } /// Records a [`VisibilityConstraint::Ambiguous`] constraint. From 4102c895a535d8919f7a0c3908d66c42ef190c02 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Thu, 23 Jan 2025 21:14:34 -0500 Subject: [PATCH 06/33] Record reachability in semantic index builder --- .../src/semantic_index/builder.rs | 8 +++---- .../src/semantic_index/use_def.rs | 22 +++++++++++++++++++ .../semantic_index/use_def/symbol_state.rs | 6 +++++ .../src/visibility_constraints.rs | 5 ++++- 4 files changed, 36 insertions(+), 5 deletions(-) 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 7b02dc2cea3f4a..8b8a6b56a3e504 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/builder.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/builder.rs @@ -370,8 +370,8 @@ impl<'db> SemanticIndexBuilder<'db> { /// Records that all remaining statements in the current block are unreachable, and therefore /// not visible. - fn record_unreachable_visibility(&mut self) -> ScopedVisibilityConstraintId { - self.record_negated_visibility_constraint(ScopedVisibilityConstraintId::ALWAYS_TRUE) + fn mark_unreachable(&mut self) { + self.current_use_def_map_mut().mark_unreachable(); } /// Records a [`VisibilityConstraint::Ambiguous`] constraint. @@ -1275,7 +1275,7 @@ where 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.record_unreachable_visibility(); + self.mark_unreachable(); } ast::Stmt::Break(_) => { @@ -1283,7 +1283,7 @@ where self.loop_break_states.push(self.flow_snapshot()); } // Everything in the current block after a terminal statement is unreachable. - self.record_unreachable_visibility(); + self.mark_unreachable(); } _ => { 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 7ad1703bfa3a1a..efe3d1857e32a9 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 5a5a06c5b0bffd..ccba2d47adb578 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 f19144fb5e072c..73e7cabbdc71de 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), + ]), } } } From 60eccc9b25d1fd50e231645c6beebf23bad04a6a Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Fri, 24 Jan 2025 14:32:23 -0500 Subject: [PATCH 07/33] Apply suggestions from code review Co-authored-by: Carl Meyer --- .../resources/mdtest/terminal_statements.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 61b914588930e9..426b1edd840eed 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -40,7 +40,7 @@ def f(cond: bool) -> str: x = "test" else: return "early" - return x + return x # no possibly-unresolved-reference diagnostic! def g(cond: bool): if cond: @@ -104,7 +104,7 @@ def g(cond: bool, i: int): reveal_type(x) # revealed: Literal["before", "loop", "break"] ``` -## `return` is terminal in nested scopes +## `return` is terminal in nested conditionals ```py def f(cond1: bool, cond2: bool) -> str: From 4190b5b30f3a44ce41eda9f02b2ef56e12c3d54b Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Fri, 24 Jan 2025 12:22:53 -0500 Subject: [PATCH 08/33] Add (failing) eager subscope example --- .../resources/mdtest/terminal_statements.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 426b1edd840eed..4916156e09bd22 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -132,3 +132,16 @@ def g(cond1: bool, cond2: bool): reveal_type(x) # revealed: Literal["test2"] reveal_type(x) # revealed: Literal["test1", "test2"] ``` + +## Terminal statement after a list comprehension + +This currently gives the wrong result because list comprehensions introduce an “eager” scope, which +we don't handle correctly yet. (The use in the list comprehension resolves relative to the _end_ of +the containing function scope, not relative to the point in the scope where the comprehension +appears.) + +```py +def f(x: str) -> int: + y = [x for i in range(len(x))] # error: [unresolved-reference] + return 4 +``` From 641ccb8c0b23167e2152554eb7d72846e28182c8 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Fri, 24 Jan 2025 13:54:01 -0500 Subject: [PATCH 09/33] Also skip merging when lhs is unreachable --- .../resources/mdtest/terminal_statements.md | 7 +----- .../src/semantic_index/use_def.rs | 25 +++++++++++-------- .../semantic_index/use_def/symbol_state.rs | 6 ----- .../src/visibility_constraints.rs | 5 +--- 4 files changed, 16 insertions(+), 27 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 4916156e09bd22..94c67285a09004 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -135,13 +135,8 @@ def g(cond1: bool, cond2: bool): ## Terminal statement after a list comprehension -This currently gives the wrong result because list comprehensions introduce an “eager” scope, which -we don't handle correctly yet. (The use in the list comprehension resolves relative to the _end_ of -the containing function scope, not relative to the point in the scope where the comprehension -appears.) - ```py def f(x: str) -> int: - y = [x for i in range(len(x))] # error: [unresolved-reference] + y = [x for i in range(len(x))] return 4 ``` 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 efe3d1857e32a9..489db14e3c29b3 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 @@ -526,12 +526,6 @@ impl Default for UseDefMapBuilder<'_> { 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) { @@ -701,16 +695,22 @@ impl<'db> UseDefMapBuilder<'db> { /// path to get here. The new state for each symbol should include definitions from both the /// prior state and the snapshot. pub(super) fn merge(&mut self, snapshot: FlowSnapshot) { + // Unreachable snapshots should not be merged: If the current snapshot is unreachable, it + // should be completely overwritten by the snapshot we're merging in. If the other snapshot + // is unreachable, we should return without merging. + if !self.reachable { + self.restore(snapshot); + return; + } + if !snapshot.reachable { + return; + } + // We never remove symbols from `symbol_states` (it's an IndexVec, and the symbol // IDs must line up), so the current number of known symbols must always be equal to or // 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() { @@ -727,6 +727,9 @@ impl<'db> UseDefMapBuilder<'db> { self.scope_start_visibility = self .visibility_constraints .add_or_constraint(self.scope_start_visibility, snapshot.scope_start_visibility); + + // At least one of the two snapshots was reachable, so the merged result is too. + self.reachable = true; } pub(super) fn finish(mut self) -> UseDefMap<'db> { 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 ccba2d47adb578..5a5a06c5b0bffd 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,12 +108,6 @@ 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 73e7cabbdc71de..f19144fb5e072c 100644 --- a/crates/red_knot_python_semantic/src/visibility_constraints.rs +++ b/crates/red_knot_python_semantic/src/visibility_constraints.rs @@ -186,10 +186,7 @@ pub(crate) struct VisibilityConstraints<'db> { impl Default for VisibilityConstraints<'_> { fn default() -> Self { Self { - constraints: IndexVec::from_iter([ - VisibilityConstraint::AlwaysTrue, - VisibilityConstraint::VisibleIfNot(ScopedVisibilityConstraintId::ALWAYS_TRUE), - ]), + constraints: IndexVec::from_iter([VisibilityConstraint::AlwaysTrue]), } } } From efdf6c3e75d17a7d1e988847cfcf76208f992766 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Fri, 24 Jan 2025 14:24:48 -0500 Subject: [PATCH 10/33] We support this one too --- crates/ruff_benchmark/benches/red_knot.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/ruff_benchmark/benches/red_knot.rs b/crates/ruff_benchmark/benches/red_knot.rs index b791b642102db2..6fc53f64f0822b 100644 --- a/crates/ruff_benchmark/benches/red_knot.rs +++ b/crates/ruff_benchmark/benches/red_knot.rs @@ -28,7 +28,6 @@ static EXPECTED_DIAGNOSTICS: &[&str] = &[ "error[lint:unresolved-import] /src/tomllib/_parser.py:7:29 Module `collections.abc` has no member `Iterable`", // 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", "error[lint:invalid-argument-type] /src/tomllib/_parser.py:632:58 Object of type `@Todo & ~AlwaysFalsy` cannot be assigned to parameter 1 (`match`) of function `match_to_localtime`; expected type `Match`", "error[lint:invalid-argument-type] /src/tomllib/_parser.py:639:52 Object of type `@Todo & ~AlwaysFalsy` cannot be assigned to parameter 1 (`match`) of function `match_to_number`; expected type `Match`", "warning[lint:unused-ignore-comment] /src/tomllib/_parser.py:682:31 Unused blanket `type: ignore` directive", From 57ea7cac0d630a9c2354ce33a8babbd8157244f0 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Fri, 24 Jan 2025 14:30:58 -0500 Subject: [PATCH 11/33] Add failing try/finally example --- .../resources/mdtest/terminal_statements.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 94c67285a09004..5096887a642d6d 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -133,6 +133,22 @@ def g(cond1: bool, cond2: bool): reveal_type(x) # revealed: Literal["test1", "test2"] ``` +## Terminal in a `finally` block + +Control-flow through finally isn't working right yet: + +```py +def f(): + x = 1 + while True: + try: + break + finally: + x = 2 + # TODO: should be Literal[2] + reveal_type(x) # revealed: Literal[1] +``` + ## Terminal statement after a list comprehension ```py From db989266d68cfada6f8d7c102566f7de520ddeac Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Fri, 24 Jan 2025 14:39:57 -0500 Subject: [PATCH 12/33] Use for loops --- .../resources/mdtest/terminal_statements.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 5096887a642d6d..a35a7625d48d5c 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -66,7 +66,7 @@ def f(cond: bool) -> str: def g(cond: bool, i: int): x = "before" - while i < 5: + for _ in range(i): if cond: x = "loop" reveal_type(x) # revealed: Literal["loop"] @@ -92,7 +92,7 @@ def f(cond: bool) -> str: def g(cond: bool, i: int): x = "before" - while i < 5: + for _ in range(i): if cond: x = "loop" reveal_type(x) # revealed: Literal["loop"] From a4347625c858cc9d74088d68082401444272bf92 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Fri, 24 Jan 2025 14:40:03 -0500 Subject: [PATCH 13/33] Add Carl's suggested return error --- .../resources/mdtest/terminal_statements.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index a35a7625d48d5c..29a38fc49d9d99 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -88,7 +88,7 @@ def f(cond: bool) -> str: else: break return x - return "late" + return x # error: [unresolved-reference] def g(cond: bool, i: int): x = "before" From 79de3ebb4accc9e6cac92da67308f3bc41c9c41f Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Mon, 27 Jan 2025 13:35:42 -0500 Subject: [PATCH 14/33] Add failing early return example --- .../resources/mdtest/terminal_statements.md | 53 ++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 29a38fc49d9d99..fc7b3e6e7545e7 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -149,7 +149,58 @@ def f(): reveal_type(x) # revealed: Literal[1] ``` -## Terminal statement after a list comprehension +## Early returns and nested functions + +Free references inside of a function body refer to variables defined in the containing scope. +Function bodies are _lazy scopes_: at runtime, these references are not resolved immediately at the +point of the function definition. Instead, they are resolved _at the time of the call_, which means +that their values (and types) can be different for different invocations. For simplicity, we instead +resolve free references _at the end of the containing scope_. That means that in the examples below, +all of the `x` bindings should be visible to the `reveal_type`, regardless of where we place the +`return` statements. + +```py +def top_level_return(cond1: bool, cond2: bool): + x = 1 + + def g(): + reveal_type(x) # revealed: Unknown | Literal[1, 2, 3] + + if cond1: + if cond2: + x = 2 + else: + x = 3 + return + +def return_from_if(cond1: bool, cond2: bool): + x = 1 + + def g(): + reveal_type(x) # revealed: Unknown | Literal[1] + + if cond1: + if cond2: + x = 2 + else: + x = 3 + return + +def return_from_nested_if(cond1: bool, cond2: bool): + x = 1 + + def g(): + reveal_type(x) # revealed: Unknown | Literal[1, 3] + + if cond1: + if cond2: + x = 2 + return + else: + x = 3 +``` + +## Early returns and list comprehensions ```py def f(x: str) -> int: From c4d15994bf3c8ef62ec906048e077a50788904f6 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Mon, 27 Jan 2025 16:17:22 -0500 Subject: [PATCH 15/33] Add TODOs for function return example --- .../resources/mdtest/terminal_statements.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index fc7b3e6e7545e7..3626f384d30cc6 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -159,6 +159,9 @@ resolve free references _at the end of the containing scope_. That means that in all of the `x` bindings should be visible to the `reveal_type`, regardless of where we place the `return` statements. +TODO: These currently produce the wrong results, but not because of our terminal statement support. +See [ruff#15777](https://github.com/astral-sh/ruff/issues/15777) for more details. + ```py def top_level_return(cond1: bool, cond2: bool): x = 1 @@ -177,6 +180,7 @@ def return_from_if(cond1: bool, cond2: bool): x = 1 def g(): + # TODO: Unknown | Literal[1, 2, 3] reveal_type(x) # revealed: Unknown | Literal[1] if cond1: @@ -190,6 +194,7 @@ def return_from_nested_if(cond1: bool, cond2: bool): x = 1 def g(): + # TODO: Unknown | Literal[1, 2, 3] reveal_type(x) # revealed: Unknown | Literal[1, 3] if cond1: From 82e6e674ae777d807e7a4f7c28082acb6fe87edf Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Mon, 27 Jan 2025 16:19:47 -0500 Subject: [PATCH 16/33] Add TODO link for continue false positives --- .../resources/mdtest/terminal_statements.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 3626f384d30cc6..278fa8f320a83b 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -55,6 +55,10 @@ def g(cond: bool): ## `continue` is terminal within its loop scope +TODO: We are not currently modeling the cyclic control flow for loops, pending fixpoint support in +Salsa. The false positives in this section are because of that, and not our terminal statement +support. See [ruff#14160](https://github.com/astral-sh/ruff/issues/14160) for more details. + ```py def f(cond: bool) -> str: while True: @@ -75,6 +79,7 @@ def g(cond: bool, i: int): reveal_type(x) # revealed: Literal["continue"] continue reveal_type(x) # revealed: Literal["loop"] + # TODO: Should be Literal["before", "loop", "continue"] reveal_type(x) # revealed: Literal["before", "loop"] ``` @@ -168,7 +173,6 @@ def top_level_return(cond1: bool, cond2: bool): def g(): reveal_type(x) # revealed: Unknown | Literal[1, 2, 3] - if cond1: if cond2: x = 2 @@ -182,7 +186,6 @@ def return_from_if(cond1: bool, cond2: bool): def g(): # TODO: Unknown | Literal[1, 2, 3] reveal_type(x) # revealed: Unknown | Literal[1] - if cond1: if cond2: x = 2 @@ -196,7 +199,6 @@ def return_from_nested_if(cond1: bool, cond2: bool): def g(): # TODO: Unknown | Literal[1, 2, 3] reveal_type(x) # revealed: Unknown | Literal[1, 3] - if cond1: if cond2: x = 2 From bfe76dafbb08242a1891ad3a932972abf2138f60 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Mon, 27 Jan 2025 16:24:34 -0500 Subject: [PATCH 17/33] Prefer the faster option first --- .../red_knot_python_semantic/src/semantic_index/use_def.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 489db14e3c29b3..c6f8359b3562e4 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 @@ -698,11 +698,11 @@ impl<'db> UseDefMapBuilder<'db> { // Unreachable snapshots should not be merged: If the current snapshot is unreachable, it // should be completely overwritten by the snapshot we're merging in. If the other snapshot // is unreachable, we should return without merging. - if !self.reachable { - self.restore(snapshot); + if !snapshot.reachable { return; } - if !snapshot.reachable { + if !self.reachable { + self.restore(snapshot); return; } From 0b7da665825cd36207b51ac8b67b57c3d877dbcc Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Tue, 28 Jan 2025 09:54:48 -0500 Subject: [PATCH 18/33] Apply suggestions from code review Co-authored-by: Carl Meyer --- .../resources/mdtest/terminal_statements.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 278fa8f320a83b..5c48499e2b43ee 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -172,6 +172,7 @@ def top_level_return(cond1: bool, cond2: bool): x = 1 def g(): + # TODO eliminate Unknown reveal_type(x) # revealed: Unknown | Literal[1, 2, 3] if cond1: if cond2: @@ -184,7 +185,7 @@ def return_from_if(cond1: bool, cond2: bool): x = 1 def g(): - # TODO: Unknown | Literal[1, 2, 3] + # TODO: Literal[1, 2, 3] reveal_type(x) # revealed: Unknown | Literal[1] if cond1: if cond2: @@ -197,7 +198,7 @@ def return_from_nested_if(cond1: bool, cond2: bool): x = 1 def g(): - # TODO: Unknown | Literal[1, 2, 3] + # TODO: Literal[1, 2, 3] reveal_type(x) # revealed: Unknown | Literal[1, 3] if cond1: if cond2: From e4771bff895dd546a381af8632c845aa050079f6 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Tue, 28 Jan 2025 09:53:42 -0500 Subject: [PATCH 19/33] Better descriptions of continue and break; add nested tests --- .../resources/mdtest/terminal_statements.md | 100 +++++++++++++++--- 1 file changed, 83 insertions(+), 17 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 5c48499e2b43ee..06f62716f78883 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -19,8 +19,8 @@ def g(cond: bool): x = "test" reveal_type(x) # revealed: Literal["test"] else: - x = "unreachable" - reveal_type(x) # revealed: Literal["unreachable"] + x = "terminal" + reveal_type(x) # revealed: Literal["terminal"] raise ValueError reveal_type(x) # revealed: Literal["test"] ``` @@ -29,38 +29,61 @@ In `f`, we should be able to determine that the `else` branch ends in a terminal 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`. +Similarly, in `g`, we should see that the assignment of the value `"terminal"` can never be seen by +the final `reveal_type`. -## `return` is terminal +## `return` + +A `return` statement is terminal; bindings that occur before it are not visible after it. ```py -def f(cond: bool) -> str: +def resolved_reference(cond: bool) -> str: if cond: x = "test" else: return "early" return x # no possibly-unresolved-reference diagnostic! -def g(cond: bool): +def return_in_if(cond: bool): if cond: x = "test" reveal_type(x) # revealed: Literal["test"] else: - x = "unreachable" - reveal_type(x) # revealed: Literal["unreachable"] + x = "terminal" + reveal_type(x) # revealed: Literal["terminal"] return reveal_type(x) # revealed: Literal["test"] + +def return_in_nested_if(cond1: bool, cond2: bool): + if cond1: + x = "test1" + reveal_type(x) # revealed: Literal["test1"] + else: + if cond2: + x = "test2" + reveal_type(x) # revealed: Literal["test2"] + else: + x = "terminal" + reveal_type(x) # revealed: Literal["terminal"] + return + reveal_type(x) # revealed: Literal["test2"] + reveal_type(x) # revealed: Literal["test1", "test2"] ``` -## `continue` is terminal within its loop scope +## `continue` + +A `continue` statement jumps back to the top of the innermost loop. This makes it terminal within +the loop body: definitions before it are not visible after it within the rest of the loop body. They +are likely to visible after the loop body, since loops do not introduce new scopes. (Statically +known infinite loops are one exception — if control never leaves the loop body, bindings inside of +the loop are not visible outside of it.) TODO: We are not currently modeling the cyclic control flow for loops, pending fixpoint support in Salsa. The false positives in this section are because of that, and not our terminal statement support. See [ruff#14160](https://github.com/astral-sh/ruff/issues/14160) for more details. ```py -def f(cond: bool) -> str: +def resolved_reference(cond: bool) -> str: while True: if cond: x = "test" @@ -68,7 +91,7 @@ def f(cond: bool) -> str: continue return x -def g(cond: bool, i: int): +def continue_in_if(cond: bool, i: int): x = "before" for _ in range(i): if cond: @@ -81,12 +104,37 @@ def g(cond: bool, i: int): reveal_type(x) # revealed: Literal["loop"] # TODO: Should be Literal["before", "loop", "continue"] reveal_type(x) # revealed: Literal["before", "loop"] + +def continue_in_nested_if(cond1: bool, cond2: bool, i: int): + x = "before" + for _ in range(i): + if cond1: + x = "loop1" + reveal_type(x) # revealed: Literal["loop1"] + else: + if cond2: + x = "loop2" + reveal_type(x) # revealed: Literal["loop2"] + else: + x = "continue" + reveal_type(x) # revealed: Literal["continue"] + continue + reveal_type(x) # revealed: Literal["loop2"] + reveal_type(x) # revealed: Literal["loop1", "loop2"] + # TODO: Should be Literal["before", "loop1", "loop2", "continue"] + reveal_type(x) # revealed: Literal["before", "loop1", "loop2"] ``` -## `break` is terminal within its loop scope +## `break` + +A `break` statement jumps to the end of the innermost loop. This makes it terminal within the loop +body: definitions before it are not visible after it within the rest of the loop body. They are +likely to visible after the loop body, since loops do not introduce new scopes. (Statically known +infinite loops are one exception — if control never leaves the loop body, bindings inside of the +loop are not visible outside of it.) ```py -def f(cond: bool) -> str: +def resolved_reference(cond: bool) -> str: while True: if cond: x = "test" @@ -95,7 +143,7 @@ def f(cond: bool) -> str: return x return x # error: [unresolved-reference] -def g(cond: bool, i: int): +def break_in_if(cond: bool, i: int): x = "before" for _ in range(i): if cond: @@ -107,6 +155,24 @@ def g(cond: bool, i: int): break reveal_type(x) # revealed: Literal["loop"] reveal_type(x) # revealed: Literal["before", "loop", "break"] + +def break_in_nested_if(cond1: bool, cond2: bool, i: int): + x = "before" + for _ in range(i): + if cond1: + x = "loop1" + reveal_type(x) # revealed: Literal["loop1"] + else: + if cond2: + x = "loop2" + reveal_type(x) # revealed: Literal["loop2"] + else: + x = "break" + reveal_type(x) # revealed: Literal["break"] + break + reveal_type(x) # revealed: Literal["loop2"] + reveal_type(x) # revealed: Literal["loop1", "loop2"] + reveal_type(x) # revealed: Literal["before", "loop1", "loop2", "break"] ``` ## `return` is terminal in nested conditionals @@ -128,8 +194,8 @@ def g(cond1: bool, cond2: bool): x = "test1" reveal_type(x) # revealed: Literal["test1"] else: - x = "unreachable" - reveal_type(x) # revealed: Literal["unreachable"] + x = "terminal" + reveal_type(x) # revealed: Literal["terminal"] return reveal_type(x) # revealed: Literal["test1"] else: From 9431fb93661e666f7213dab78415a19b68df53d0 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Tue, 28 Jan 2025 09:53:57 -0500 Subject: [PATCH 20/33] Remove tomllib benchmark error reproduction --- .../resources/mdtest/terminal_statements.md | 8 -------- 1 file changed, 8 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 06f62716f78883..2da266814ef2bf 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -273,11 +273,3 @@ def return_from_nested_if(cond1: bool, cond2: bool): else: x = 3 ``` - -## Early returns and list comprehensions - -```py -def f(x: str) -> int: - y = [x for i in range(len(x))] - return 4 -``` From 08b05229622eedfd0b5686c9f92ef90df6d4e0ec Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Tue, 28 Jan 2025 09:59:32 -0500 Subject: [PATCH 21/33] Add "both nested branches" tests --- .../resources/mdtest/terminal_statements.md | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 2da266814ef2bf..b68164a14f9adb 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -54,6 +54,16 @@ def return_in_if(cond: bool): return reveal_type(x) # revealed: Literal["test"] +def return_in_both_branches(cond: bool): + if cond: + x = "terminal1" + reveal_type(x) # revealed: Literal["terminal1"] + return + else: + x = "terminal2" + reveal_type(x) # revealed: Literal["terminal2"] + return + def return_in_nested_if(cond1: bool, cond2: bool): if cond1: x = "test1" @@ -68,6 +78,21 @@ def return_in_nested_if(cond1: bool, cond2: bool): return reveal_type(x) # revealed: Literal["test2"] reveal_type(x) # revealed: Literal["test1", "test2"] + +def return_in_both_nested_branches(cond1: bool, cond2: bool): + if cond1: + x = "test" + reveal_type(x) # revealed: Literal["test"] + else: + if cond2: + x = "terminal1" + reveal_type(x) # revealed: Literal["terminal1"] + return + else: + x = "terminal2" + reveal_type(x) # revealed: Literal["terminal2"] + return + reveal_type(x) # revealed: Literal["test"] ``` ## `continue` @@ -105,6 +130,20 @@ def continue_in_if(cond: bool, i: int): # TODO: Should be Literal["before", "loop", "continue"] reveal_type(x) # revealed: Literal["before", "loop"] +def continue_in_both_branches(cond: bool, i: int): + x = "before" + for _ in range(i): + if cond: + x = "continue1" + reveal_type(x) # revealed: Literal["continue1"] + continue + else: + x = "continue2" + reveal_type(x) # revealed: Literal["continue2"] + continue + # TODO: Should be Literal["before", "continue1", "continue2"] + reveal_type(x) # revealed: Literal["before"] + def continue_in_nested_if(cond1: bool, cond2: bool, i: int): x = "before" for _ in range(i): @@ -123,6 +162,25 @@ def continue_in_nested_if(cond1: bool, cond2: bool, i: int): reveal_type(x) # revealed: Literal["loop1", "loop2"] # TODO: Should be Literal["before", "loop1", "loop2", "continue"] reveal_type(x) # revealed: Literal["before", "loop1", "loop2"] + +def continue_in_both_nested_branches(cond1: bool, cond2: bool, i: int): + x = "before" + for _ in range(i): + if cond1: + x = "loop" + reveal_type(x) # revealed: Literal["loop"] + else: + if cond2: + x = "continue1" + reveal_type(x) # revealed: Literal["continue1"] + continue + else: + x = "continue2" + reveal_type(x) # revealed: Literal["continue2"] + continue + reveal_type(x) # revealed: Literal["loop"] + # TODO: Should be Literal["before", "loop", "continue1", "continue2"] + reveal_type(x) # revealed: Literal["before", "loop"] ``` ## `break` @@ -156,6 +214,19 @@ def break_in_if(cond: bool, i: int): reveal_type(x) # revealed: Literal["loop"] reveal_type(x) # revealed: Literal["before", "loop", "break"] +def break_in_both_branches(cond: bool, i: int): + x = "before" + for _ in range(i): + if cond: + x = "break1" + reveal_type(x) # revealed: Literal["break1"] + break + else: + x = "break2" + reveal_type(x) # revealed: Literal["break2"] + break + reveal_type(x) # revealed: Literal["before", "break1", "break2"] + def break_in_nested_if(cond1: bool, cond2: bool, i: int): x = "before" for _ in range(i): @@ -173,6 +244,24 @@ def break_in_nested_if(cond1: bool, cond2: bool, i: int): reveal_type(x) # revealed: Literal["loop2"] reveal_type(x) # revealed: Literal["loop1", "loop2"] reveal_type(x) # revealed: Literal["before", "loop1", "loop2", "break"] + +def break_in_both_nested_branches(cond1: bool, cond2: bool, i: int): + x = "before" + for _ in range(i): + if cond1: + x = "loop" + reveal_type(x) # revealed: Literal["loop"] + else: + if cond2: + x = "break1" + reveal_type(x) # revealed: Literal["break1"] + break + else: + x = "break2" + reveal_type(x) # revealed: Literal["break2"] + break + reveal_type(x) # revealed: Literal["loop"] + reveal_type(x) # revealed: Literal["before", "loop", "break1", "break2"] ``` ## `return` is terminal in nested conditionals From ee3ac28c46d4ce460945c2155f2a1caebb36642a Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Tue, 28 Jan 2025 10:06:33 -0500 Subject: [PATCH 22/33] Add separate then/else branch tests --- .../resources/mdtest/terminal_statements.md | 101 ++++++++++++++++-- 1 file changed, 95 insertions(+), 6 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index b68164a14f9adb..9861ec0f8c6eb0 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -44,7 +44,17 @@ def resolved_reference(cond: bool) -> str: return "early" return x # no possibly-unresolved-reference diagnostic! -def return_in_if(cond: bool): +def return_in_then_branch(cond: bool): + if cond: + x = "terminal" + reveal_type(x) # revealed: Literal["terminal"] + return + else: + x = "test" + reveal_type(x) # revealed: Literal["test"] + reveal_type(x) # revealed: Literal["test"] + +def return_in_else_branch(cond: bool): if cond: x = "test" reveal_type(x) # revealed: Literal["test"] @@ -64,7 +74,22 @@ def return_in_both_branches(cond: bool): reveal_type(x) # revealed: Literal["terminal2"] return -def return_in_nested_if(cond1: bool, cond2: bool): +def return_in_nested_then_branch(cond1: bool, cond2: bool): + if cond1: + x = "test1" + reveal_type(x) # revealed: Literal["test1"] + else: + if cond2: + x = "terminal" + reveal_type(x) # revealed: Literal["terminal"] + return + else: + x = "test2" + reveal_type(x) # revealed: Literal["test2"] + reveal_type(x) # revealed: Literal["test2"] + reveal_type(x) # revealed: Literal["test1", "test2"] + +def return_in_nested_else_branch(cond1: bool, cond2: bool): if cond1: x = "test1" reveal_type(x) # revealed: Literal["test1"] @@ -116,7 +141,21 @@ def resolved_reference(cond: bool) -> str: continue return x -def continue_in_if(cond: bool, i: int): +def continue_in_then_branch(cond: bool, i: int): + x = "before" + for _ in range(i): + if cond: + x = "continue" + reveal_type(x) # revealed: Literal["continue"] + continue + else: + x = "loop" + reveal_type(x) # revealed: Literal["loop"] + reveal_type(x) # revealed: Literal["loop"] + # TODO: Should be Literal["before", "loop", "continue"] + reveal_type(x) # revealed: Literal["before", "loop"] + +def continue_in_else_branch(cond: bool, i: int): x = "before" for _ in range(i): if cond: @@ -144,7 +183,26 @@ def continue_in_both_branches(cond: bool, i: int): # TODO: Should be Literal["before", "continue1", "continue2"] reveal_type(x) # revealed: Literal["before"] -def continue_in_nested_if(cond1: bool, cond2: bool, i: int): +def continue_in_nested_then_branch(cond1: bool, cond2: bool, i: int): + x = "before" + for _ in range(i): + if cond1: + x = "loop1" + reveal_type(x) # revealed: Literal["loop1"] + else: + if cond2: + x = "continue" + reveal_type(x) # revealed: Literal["continue"] + continue + else: + x = "loop2" + reveal_type(x) # revealed: Literal["loop2"] + reveal_type(x) # revealed: Literal["loop2"] + reveal_type(x) # revealed: Literal["loop1", "loop2"] + # TODO: Should be Literal["before", "loop1", "loop2", "continue"] + reveal_type(x) # revealed: Literal["before", "loop1", "loop2"] + +def continue_in_nested_else_branch(cond1: bool, cond2: bool, i: int): x = "before" for _ in range(i): if cond1: @@ -201,7 +259,20 @@ def resolved_reference(cond: bool) -> str: return x return x # error: [unresolved-reference] -def break_in_if(cond: bool, i: int): +def break_in_then_branch(cond: bool, i: int): + x = "before" + for _ in range(i): + if cond: + x = "break" + reveal_type(x) # revealed: Literal["break"] + break + else: + x = "loop" + reveal_type(x) # revealed: Literal["loop"] + reveal_type(x) # revealed: Literal["loop"] + reveal_type(x) # revealed: Literal["before", "break", "loop"] + +def break_in_else_branch(cond: bool, i: int): x = "before" for _ in range(i): if cond: @@ -227,7 +298,25 @@ def break_in_both_branches(cond: bool, i: int): break reveal_type(x) # revealed: Literal["before", "break1", "break2"] -def break_in_nested_if(cond1: bool, cond2: bool, i: int): +def break_in_nested_then_branch(cond1: bool, cond2: bool, i: int): + x = "before" + for _ in range(i): + if cond1: + x = "loop1" + reveal_type(x) # revealed: Literal["loop1"] + else: + if cond2: + x = "break" + reveal_type(x) # revealed: Literal["break"] + break + else: + x = "loop2" + reveal_type(x) # revealed: Literal["loop2"] + reveal_type(x) # revealed: Literal["loop2"] + reveal_type(x) # revealed: Literal["loop1", "loop2"] + reveal_type(x) # revealed: Literal["before", "loop1", "break", "loop2"] + +def break_in_nested_else_branch(cond1: bool, cond2: bool, i: int): x = "before" for _ in range(i): if cond1: From add8a574d3cf65473244eac1dee795f8cba81188 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Tue, 28 Jan 2025 10:29:02 -0500 Subject: [PATCH 23/33] Fix comment --- crates/red_knot_python_semantic/src/semantic_index/use_def.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c6f8359b3562e4..04120f8a67fa9b 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 @@ -728,7 +728,7 @@ impl<'db> UseDefMapBuilder<'db> { .visibility_constraints .add_or_constraint(self.scope_start_visibility, snapshot.scope_start_visibility); - // At least one of the two snapshots was reachable, so the merged result is too. + // Both of the snapshots are reachable, so the merged result is too. self.reachable = true; } From a0e6980937e9bc2eee5c4e871c2e335374934d84 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Tue, 28 Jan 2025 12:44:42 -0500 Subject: [PATCH 24/33] Remove duplicate test --- .../resources/mdtest/terminal_statements.md | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 9861ec0f8c6eb0..30f959cc4c7cb2 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -353,35 +353,6 @@ def break_in_both_nested_branches(cond1: bool, cond2: bool, i: int): reveal_type(x) # revealed: Literal["before", "loop", "break1", "break2"] ``` -## `return` is terminal in nested conditionals - -```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 = "terminal" - reveal_type(x) # revealed: Literal["terminal"] - return - reveal_type(x) # revealed: Literal["test1"] - else: - x = "test2" - reveal_type(x) # revealed: Literal["test2"] - reveal_type(x) # revealed: Literal["test1", "test2"] -``` - ## Terminal in a `finally` block Control-flow through finally isn't working right yet: From 9fb8d2a6b1a3ddd531a480664c6bd0f7027aee27 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Tue, 28 Jan 2025 13:12:36 -0500 Subject: [PATCH 25/33] Fix header --- .../resources/mdtest/terminal_statements.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 30f959cc4c7cb2..84189504d50155 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -369,7 +369,7 @@ def f(): reveal_type(x) # revealed: Literal[1] ``` -## Early returns and nested functions +## Nested functions Free references inside of a function body refer to variables defined in the containing scope. Function bodies are _lazy scopes_: at runtime, these references are not resolved immediately at the From 36847396c97ff79d0c2855d9c1f1e78aac54dec2 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Tue, 28 Jan 2025 13:16:54 -0500 Subject: [PATCH 26/33] Add raise tests --- .../resources/mdtest/terminal_statements.md | 181 ++++++++++++++++++ 1 file changed, 181 insertions(+) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 84189504d50155..3887e1bcbe085c 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -353,6 +353,187 @@ def break_in_both_nested_branches(cond1: bool, cond2: bool, i: int): reveal_type(x) # revealed: Literal["before", "loop", "break1", "break2"] ``` +## `raise` + +A `raise` statement is terminal. If it occurs in a lexically containing `try` statement, it will +jump to one of the `except` clauses (if it matches the value being raised), or to the `finally` +clause (if none match). Currently, we assume definitions from before the `raise` are visible in all +`except` and `finally` clauses. (In the future, we might analyze the `except` clauses to see which +ones match the value being raised, and limit visibility to those clauses.) Definitions from before +the `raise` are not visible in any `else` clause, but are visible after the containing `try` +statement, since `try` does not introduce a new scope. + +TODO: We are not currently implementing the "jump" behavior correctly for `raise` statements. The +false positives in this section are because of that, and not our terminal statement support. + +```py +def raise_in_then_branch(cond: bool): + x = "before" + try: + if cond: + x = "raise" + reveal_type(x) # revealed: Literal["raise"] + raise ValueError + else: + x = "else" + reveal_type(x) # revealed: Literal["else"] + reveal_type(x) # revealed: Literal["else"] + except ValueError: + # TODO: Literal["raise"] + reveal_type(x) # revealed: Literal["before", "raise", "else"] + except: + # TODO: Literal["raise"] or Never + reveal_type(x) # revealed: Literal["before", "raise", "else"] + else: + reveal_type(x) # revealed: Literal["else"] + finally: + # This includes "before" because we assume that an exception might have occurred before the + # "raise" assignment. + reveal_type(x) # revealed: Literal["before", "raise", "else"] + reveal_type(x) # revealed: Literal["before", "raise", "else"] + +def raise_in_else_branch(cond: bool): + x = "before" + try: + if cond: + x = "else" + reveal_type(x) # revealed: Literal["else"] + else: + x = "raise" + reveal_type(x) # revealed: Literal["raise"] + raise ValueError + reveal_type(x) # revealed: Literal["else"] + except ValueError: + # TODO: Literal["raise"] + reveal_type(x) # revealed: Literal["before", "else", "raise"] + except: + # TODO: Literal["raise"] or Never + reveal_type(x) # revealed: Literal["before", "else", "raise"] + else: + reveal_type(x) # revealed: Literal["else"] + finally: + # This includes "before" because we assume that an exception might have occurred before the + # "raise" assignment. + reveal_type(x) # revealed: Literal["before", "else", "raise"] + reveal_type(x) # revealed: Literal["before", "else", "raise"] + +def raise_in_both_branches(cond: bool): + x = "before" + try: + if cond: + x = "raise1" + reveal_type(x) # revealed: Literal["raise1"] + raise ValueError + else: + x = "raise2" + reveal_type(x) # revealed: Literal["raise2"] + raise ValueError + except ValueError: + # TODO: Literal["raise1", "raise2"] + reveal_type(x) # revealed: Literal["before", "raise1", "raise2"] + except: + # TODO: Literal["raise1", "raise2"] or Never + reveal_type(x) # revealed: Literal["before", "raise1", "raise2"] + else: + # This is unreachable + pass + finally: + # This includes "before" because we assume that an exception might have occurred before the + # "raise" assignment. + reveal_type(x) # revealed: Literal["before", "raise1", "raise2"] + reveal_type(x) # revealed: Literal["before", "raise1", "raise2"] + +def raise_in_nested_then_branch(cond1: bool, cond2: bool): + x = "before" + try: + if cond1: + x = "else1" + reveal_type(x) # revealed: Literal["else1"] + else: + if cond2: + x = "raise" + reveal_type(x) # revealed: Literal["raise"] + raise ValueError + else: + x = "else2" + reveal_type(x) # revealed: Literal["else2"] + reveal_type(x) # revealed: Literal["else2"] + reveal_type(x) # revealed: Literal["else1", "else2"] + except ValueError: + # TODO: Literal["raise"] + reveal_type(x) # revealed: Literal["before", "else1", "raise", "else2"] + except: + # TODO: Literal["raise"] or Never + reveal_type(x) # revealed: Literal["before", "else1", "raise", "else2"] + else: + reveal_type(x) # revealed: Literal["else1", "else2"] + finally: + # This includes "before" because we assume that an exception might have occurred before the + # "raise" assignment. + reveal_type(x) # revealed: Literal["before", "else1", "raise", "else2"] + reveal_type(x) # revealed: Literal["before", "else1", "raise", "else2"] + +def raise_in_nested_else_branch(cond1: bool, cond2: bool): + x = "before" + try: + if cond1: + x = "else1" + reveal_type(x) # revealed: Literal["else1"] + else: + if cond2: + x = "else2" + reveal_type(x) # revealed: Literal["else2"] + else: + x = "raise" + reveal_type(x) # revealed: Literal["raise"] + raise ValueError + reveal_type(x) # revealed: Literal["else2"] + reveal_type(x) # revealed: Literal["else1", "else2"] + except ValueError: + # TODO: Literal["raise"] + reveal_type(x) # revealed: Literal["before", "else1", "else2", "raise"] + except: + # TODO: Literal["raise"] or Never + reveal_type(x) # revealed: Literal["before", "else1", "else2", "raise"] + else: + reveal_type(x) # revealed: Literal["else1", "else2"] + finally: + # This includes "before" because we assume that an exception might have occurred before the + # "raise" assignment. + reveal_type(x) # revealed: Literal["before", "else1", "else2", "raise"] + reveal_type(x) # revealed: Literal["before", "else1", "else2", "raise"] + +def raise_in_both_nested_branches(cond1: bool, cond2: bool): + x = "before" + try: + if cond1: + x = "else" + reveal_type(x) # revealed: Literal["else"] + else: + if cond2: + x = "raise1" + reveal_type(x) # revealed: Literal["raise1"] + raise ValueError + else: + x = "raise2" + reveal_type(x) # revealed: Literal["raise2"] + raise ValueError + reveal_type(x) # revealed: Literal["else"] + except ValueError: + # TODO: Literal["raise1", "raise2"] + reveal_type(x) # revealed: Literal["before", "else", "raise1", "raise2"] + except: + # TODO: Literal["raise1", "raise2"] or Never + reveal_type(x) # revealed: Literal["before", "else", "raise1", "raise2"] + else: + reveal_type(x) # revealed: Literal["else"] + finally: + # This includes "before" because we assume that an exception might have occurred before the + # "raise" assignment. + reveal_type(x) # revealed: Literal["before", "else", "raise1", "raise2"] + reveal_type(x) # revealed: Literal["before", "else", "raise1", "raise2"] +``` + ## Terminal in a `finally` block Control-flow through finally isn't working right yet: From 7fbec5f3dac0195181b5be7d35b9a10cb8a10ca7 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Tue, 28 Jan 2025 13:22:47 -0500 Subject: [PATCH 27/33] Add return-in-try tests --- .../resources/mdtest/terminal_statements.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 3887e1bcbe085c..7adb0a9690e7d7 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -74,6 +74,21 @@ def return_in_both_branches(cond: bool): reveal_type(x) # revealed: Literal["terminal2"] return +def return_in_try(cond: bool): + x = "before" + try: + if cond: + x = "test" + return + except: + # TODO: Literal["before"] + reveal_type(x) # revealed: Literal["before", "test"] + else: + reveal_type(x) # revealed: Literal["before"] + finally: + reveal_type(x) # revealed: Literal["before", "test"] + reveal_type(x) # revealed: Literal["before", "test"] + def return_in_nested_then_branch(cond1: bool, cond2: bool): if cond1: x = "test1" From 5c45e88b8fa26e88c8026d6d55be789e1be8a65c Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Tue, 28 Jan 2025 15:55:34 -0500 Subject: [PATCH 28/33] Fix comment --- .../resources/mdtest/terminal_statements.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 7adb0a9690e7d7..634f7e6d6c9196 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -403,7 +403,7 @@ def raise_in_then_branch(cond: bool): reveal_type(x) # revealed: Literal["else"] finally: # This includes "before" because we assume that an exception might have occurred before the - # "raise" assignment. + # `if` statement. reveal_type(x) # revealed: Literal["before", "raise", "else"] reveal_type(x) # revealed: Literal["before", "raise", "else"] @@ -428,7 +428,7 @@ def raise_in_else_branch(cond: bool): reveal_type(x) # revealed: Literal["else"] finally: # This includes "before" because we assume that an exception might have occurred before the - # "raise" assignment. + # `if` statement. reveal_type(x) # revealed: Literal["before", "else", "raise"] reveal_type(x) # revealed: Literal["before", "else", "raise"] @@ -454,7 +454,7 @@ def raise_in_both_branches(cond: bool): pass finally: # This includes "before" because we assume that an exception might have occurred before the - # "raise" assignment. + # `if` statement. reveal_type(x) # revealed: Literal["before", "raise1", "raise2"] reveal_type(x) # revealed: Literal["before", "raise1", "raise2"] @@ -484,7 +484,7 @@ def raise_in_nested_then_branch(cond1: bool, cond2: bool): reveal_type(x) # revealed: Literal["else1", "else2"] finally: # This includes "before" because we assume that an exception might have occurred before the - # "raise" assignment. + # `if` statement. reveal_type(x) # revealed: Literal["before", "else1", "raise", "else2"] reveal_type(x) # revealed: Literal["before", "else1", "raise", "else2"] @@ -514,7 +514,7 @@ def raise_in_nested_else_branch(cond1: bool, cond2: bool): reveal_type(x) # revealed: Literal["else1", "else2"] finally: # This includes "before" because we assume that an exception might have occurred before the - # "raise" assignment. + # `if` statement. reveal_type(x) # revealed: Literal["before", "else1", "else2", "raise"] reveal_type(x) # revealed: Literal["before", "else1", "else2", "raise"] @@ -544,7 +544,7 @@ def raise_in_both_nested_branches(cond1: bool, cond2: bool): reveal_type(x) # revealed: Literal["else"] finally: # This includes "before" because we assume that an exception might have occurred before the - # "raise" assignment. + # `if` statement. reveal_type(x) # revealed: Literal["before", "else", "raise1", "raise2"] reveal_type(x) # revealed: Literal["before", "else", "raise1", "raise2"] ``` From d2b21fcfaf9a2f2a628034db492e017462e527a1 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Tue, 28 Jan 2025 15:55:38 -0500 Subject: [PATCH 29/33] Add statically-known terminal test case --- .../resources/mdtest/terminal_statements.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 634f7e6d6c9196..5d5f5ef1d9b5f3 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -618,3 +618,21 @@ def return_from_nested_if(cond1: bool, cond2: bool): else: x = 3 ``` + +## Statically known terminal statements + +Terminal statements do not yet interact correctly with statically known bounds. In this example, we +should see that the `return` statement is always executed, and therefore that the `"b"` assignment +is not visible to the `reveal_type`. + +```py +def _(cond: bool): + x = "a" + if cond: + x = "b" + if True: + return + + # TODO: Literal["a"] + reveal_type(x) # revealed: Literal["a", "b"] +``` From 2f62eb02bd6612068ad9ffed431dc11f618104af Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Wed, 29 Jan 2025 09:32:23 -0500 Subject: [PATCH 30/33] Apply suggestions from code review Co-authored-by: Carl Meyer --- .../resources/mdtest/terminal_statements.md | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 5d5f5ef1d9b5f3..688d62dff31650 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -124,6 +124,7 @@ def return_in_both_nested_branches(cond1: bool, cond2: bool): x = "test" reveal_type(x) # revealed: Literal["test"] else: + x = "terminal0" if cond2: x = "terminal1" reveal_type(x) # revealed: Literal["terminal1"] @@ -139,7 +140,7 @@ def return_in_both_nested_branches(cond1: bool, cond2: bool): A `continue` statement jumps back to the top of the innermost loop. This makes it terminal within the loop body: definitions before it are not visible after it within the rest of the loop body. They -are likely to visible after the loop body, since loops do not introduce new scopes. (Statically +are likely visible after the loop body, since loops do not introduce new scopes. (Statically known infinite loops are one exception — if control never leaves the loop body, bindings inside of the loop are not visible outside of it.) @@ -260,7 +261,7 @@ def continue_in_both_nested_branches(cond1: bool, cond2: bool, i: int): A `break` statement jumps to the end of the innermost loop. This makes it terminal within the loop body: definitions before it are not visible after it within the rest of the loop body. They are -likely to visible after the loop body, since loops do not introduce new scopes. (Statically known +likely visible after the loop body, since loops do not introduce new scopes. (Statically known infinite loops are one exception — if control never leaves the loop body, bindings inside of the loop are not visible outside of it.) @@ -371,15 +372,16 @@ def break_in_both_nested_branches(cond1: bool, cond2: bool, i: int): ## `raise` A `raise` statement is terminal. If it occurs in a lexically containing `try` statement, it will -jump to one of the `except` clauses (if it matches the value being raised), or to the `finally` +jump to one of the `except` clauses (if it matches the value being raised), or to the `else` clause (if none match). Currently, we assume definitions from before the `raise` are visible in all -`except` and `finally` clauses. (In the future, we might analyze the `except` clauses to see which +`except` and `else` clauses. (In the future, we might analyze the `except` clauses to see which ones match the value being raised, and limit visibility to those clauses.) Definitions from before -the `raise` are not visible in any `else` clause, but are visible after the containing `try` -statement, since `try` does not introduce a new scope. +the `raise` are not visible in any `else` clause, but are visible in `except` clauses or after the +containing `try` statement (since control flow may have passed through an `except`). -TODO: We are not currently implementing the "jump" behavior correctly for `raise` statements. The -false positives in this section are because of that, and not our terminal statement support. +Currently we assume that an exception could be raised anywhere within a `try` block; the TODOs below reflect +cases where we could implement a more precise understanding of where exceptions (barring `KeyboardInterrupt` +and `MemoryError`) can and cannot actually be raised. ```py def raise_in_then_branch(cond: bool): @@ -447,7 +449,7 @@ def raise_in_both_branches(cond: bool): # TODO: Literal["raise1", "raise2"] reveal_type(x) # revealed: Literal["before", "raise1", "raise2"] except: - # TODO: Literal["raise1", "raise2"] or Never + # TODO: Literal["raise1", "raise2"] reveal_type(x) # revealed: Literal["before", "raise1", "raise2"] else: # This is unreachable @@ -549,9 +551,10 @@ def raise_in_both_nested_branches(cond1: bool, cond2: bool): reveal_type(x) # revealed: Literal["before", "else", "raise1", "raise2"] ``` -## Terminal in a `finally` block +## Terminal in `try` with `finally` clause -Control-flow through finally isn't working right yet: +TODO: we don't yet model that a `break` or `continue` in a `try` block will jump to a `finally` clause before it +jumps to end/start of the loop. ```py def f(): From 32e8131352f68080c3a15810d9452a4b49137fe1 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Wed, 29 Jan 2025 09:32:56 -0500 Subject: [PATCH 31/33] Update TODOs to not assume how we'll change exception tracking --- .../resources/mdtest/terminal_statements.md | 51 +++++++++---------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index 688d62dff31650..e369b35d8afbbd 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -396,17 +396,17 @@ def raise_in_then_branch(cond: bool): reveal_type(x) # revealed: Literal["else"] reveal_type(x) # revealed: Literal["else"] except ValueError: - # TODO: Literal["raise"] + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "raise", "else"] except: - # TODO: Literal["raise"] or Never + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "raise", "else"] else: reveal_type(x) # revealed: Literal["else"] finally: - # This includes "before" because we assume that an exception might have occurred before the - # `if` statement. + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "raise", "else"] + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "raise", "else"] def raise_in_else_branch(cond: bool): @@ -421,17 +421,17 @@ def raise_in_else_branch(cond: bool): raise ValueError reveal_type(x) # revealed: Literal["else"] except ValueError: - # TODO: Literal["raise"] + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "else", "raise"] except: - # TODO: Literal["raise"] or Never + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "else", "raise"] else: reveal_type(x) # revealed: Literal["else"] finally: - # This includes "before" because we assume that an exception might have occurred before the - # `if` statement. + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "else", "raise"] + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "else", "raise"] def raise_in_both_branches(cond: bool): @@ -446,18 +446,17 @@ def raise_in_both_branches(cond: bool): reveal_type(x) # revealed: Literal["raise2"] raise ValueError except ValueError: - # TODO: Literal["raise1", "raise2"] + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "raise1", "raise2"] except: - # TODO: Literal["raise1", "raise2"] + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "raise1", "raise2"] else: - # This is unreachable - pass + x = "unreachable" finally: - # This includes "before" because we assume that an exception might have occurred before the - # `if` statement. + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "raise1", "raise2"] + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "raise1", "raise2"] def raise_in_nested_then_branch(cond1: bool, cond2: bool): @@ -477,17 +476,17 @@ def raise_in_nested_then_branch(cond1: bool, cond2: bool): reveal_type(x) # revealed: Literal["else2"] reveal_type(x) # revealed: Literal["else1", "else2"] except ValueError: - # TODO: Literal["raise"] + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "else1", "raise", "else2"] except: - # TODO: Literal["raise"] or Never + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "else1", "raise", "else2"] else: reveal_type(x) # revealed: Literal["else1", "else2"] finally: - # This includes "before" because we assume that an exception might have occurred before the - # `if` statement. + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "else1", "raise", "else2"] + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "else1", "raise", "else2"] def raise_in_nested_else_branch(cond1: bool, cond2: bool): @@ -507,17 +506,17 @@ def raise_in_nested_else_branch(cond1: bool, cond2: bool): reveal_type(x) # revealed: Literal["else2"] reveal_type(x) # revealed: Literal["else1", "else2"] except ValueError: - # TODO: Literal["raise"] + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "else1", "else2", "raise"] except: - # TODO: Literal["raise"] or Never + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "else1", "else2", "raise"] else: reveal_type(x) # revealed: Literal["else1", "else2"] finally: - # This includes "before" because we assume that an exception might have occurred before the - # `if` statement. + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "else1", "else2", "raise"] + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "else1", "else2", "raise"] def raise_in_both_nested_branches(cond1: bool, cond2: bool): @@ -537,17 +536,17 @@ def raise_in_both_nested_branches(cond1: bool, cond2: bool): raise ValueError reveal_type(x) # revealed: Literal["else"] except ValueError: - # TODO: Literal["raise1", "raise2"] + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "else", "raise1", "raise2"] except: - # TODO: Literal["raise1", "raise2"] or Never + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "else", "raise1", "raise2"] else: reveal_type(x) # revealed: Literal["else"] finally: - # This includes "before" because we assume that an exception might have occurred before the - # `if` statement. + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "else", "raise1", "raise2"] + # Exceptions can occur anywhere, so "before" and "raise" are valid possibilities reveal_type(x) # revealed: Literal["before", "else", "raise1", "raise2"] ``` From 1cf65f508e8ba899d56f3b2d09b6e143899fb5fd Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Wed, 29 Jan 2025 09:34:43 -0500 Subject: [PATCH 32/33] Linter --- .../resources/mdtest/terminal_statements.md | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index e369b35d8afbbd..a8e5575f6cd4cd 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -140,9 +140,9 @@ def return_in_both_nested_branches(cond1: bool, cond2: bool): A `continue` statement jumps back to the top of the innermost loop. This makes it terminal within the loop body: definitions before it are not visible after it within the rest of the loop body. They -are likely visible after the loop body, since loops do not introduce new scopes. (Statically -known infinite loops are one exception — if control never leaves the loop body, bindings inside of -the loop are not visible outside of it.) +are likely visible after the loop body, since loops do not introduce new scopes. (Statically known +infinite loops are one exception — if control never leaves the loop body, bindings inside of the +loop are not visible outside of it.) TODO: We are not currently modeling the cyclic control flow for loops, pending fixpoint support in Salsa. The false positives in this section are because of that, and not our terminal statement @@ -372,16 +372,16 @@ def break_in_both_nested_branches(cond1: bool, cond2: bool, i: int): ## `raise` A `raise` statement is terminal. If it occurs in a lexically containing `try` statement, it will -jump to one of the `except` clauses (if it matches the value being raised), or to the `else` -clause (if none match). Currently, we assume definitions from before the `raise` are visible in all -`except` and `else` clauses. (In the future, we might analyze the `except` clauses to see which -ones match the value being raised, and limit visibility to those clauses.) Definitions from before -the `raise` are not visible in any `else` clause, but are visible in `except` clauses or after the +jump to one of the `except` clauses (if it matches the value being raised), or to the `else` clause +(if none match). Currently, we assume definitions from before the `raise` are visible in all +`except` and `else` clauses. (In the future, we might analyze the `except` clauses to see which ones +match the value being raised, and limit visibility to those clauses.) Definitions from before the +`raise` are not visible in any `else` clause, but are visible in `except` clauses or after the containing `try` statement (since control flow may have passed through an `except`). -Currently we assume that an exception could be raised anywhere within a `try` block; the TODOs below reflect -cases where we could implement a more precise understanding of where exceptions (barring `KeyboardInterrupt` -and `MemoryError`) can and cannot actually be raised. +Currently we assume that an exception could be raised anywhere within a `try` block; the TODOs below +reflect cases where we could implement a more precise understanding of where exceptions (barring +`KeyboardInterrupt` and `MemoryError`) can and cannot actually be raised. ```py def raise_in_then_branch(cond: bool): @@ -552,8 +552,8 @@ def raise_in_both_nested_branches(cond1: bool, cond2: bool): ## Terminal in `try` with `finally` clause -TODO: we don't yet model that a `break` or `continue` in a `try` block will jump to a `finally` clause before it -jumps to end/start of the loop. +TODO: we don't yet model that a `break` or `continue` in a `try` block will jump to a `finally` +clause before it jumps to end/start of the loop. ```py def f(): From 681fae2751ee939e787bea5f6f7bb8fb4231d924 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Wed, 29 Jan 2025 13:30:38 -0500 Subject: [PATCH 33/33] Update crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md Co-authored-by: Carl Meyer --- .../resources/mdtest/terminal_statements.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md index a8e5575f6cd4cd..6a3427e211c845 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/red_knot_python_semantic/resources/mdtest/terminal_statements.md @@ -379,9 +379,9 @@ match the value being raised, and limit visibility to those clauses.) Definition `raise` are not visible in any `else` clause, but are visible in `except` clauses or after the containing `try` statement (since control flow may have passed through an `except`). -Currently we assume that an exception could be raised anywhere within a `try` block; the TODOs below -reflect cases where we could implement a more precise understanding of where exceptions (barring -`KeyboardInterrupt` and `MemoryError`) can and cannot actually be raised. +Currently we assume that an exception could be raised anywhere within a `try` block. We may want to +implement a more precise understanding of where exceptions (barring `KeyboardInterrupt` and +`MemoryError`) can and cannot actually be raised. ```py def raise_in_then_branch(cond: bool):