Skip to content

Commit 7e6d383

Browse files
[red-knot] Add cycle handling to narrow constraints queries (#17209)
## Summary This PR fixes the cycle issue that was causing problems in the `support super` PR. ### Affected queries - `all_narrowing_constraints_for_expression` - `all_negative_narrowing_constraints_for_expression` -- Additionally, `bidict` and `werkzeug` have been added to the project-selection list in `mypy_primer`. This PR also addresses the panics that occurred while analyzing those packages: - `bidict`: panic triggered by `all_narrowing_constraints_for_expression` - `werkzeug`: panic triggered by `all_negative_narrowing_constraints_for_expression` I think the mypy-primer results for this PR can serve as sufficient test :)
1 parent 1a6a10b commit 7e6d383

File tree

2 files changed

+45
-3
lines changed

2 files changed

+45
-3
lines changed

.github/workflows/mypy_primer.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ jobs:
6868
--type-checker knot \
6969
--old base_commit \
7070
--new "$GITHUB_SHA" \
71-
--project-selector '/(mypy_primer|black|pyp|git-revise|zipp|arrow|isort|itsdangerous|rich|packaging|pybind11|pyinstrument|typeshed-stats|scrapy)$' \
71+
--project-selector '/(mypy_primer|black|pyp|git-revise|zipp|arrow|isort|itsdangerous|rich|packaging|pybind11|pyinstrument|typeshed-stats|scrapy|werkzeug|bidict)$' \
7272
--output concise \
7373
--debug > mypy_primer.diff || [ $? -eq 1 ]
7474

crates/red_knot_python_semantic/src/types/narrow.rs

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,11 @@ fn all_narrowing_constraints_for_pattern<'db>(
6969
}
7070

7171
#[allow(clippy::ref_option)]
72-
#[salsa::tracked(return_ref)]
72+
#[salsa::tracked(
73+
return_ref,
74+
cycle_fn=constraints_for_expression_cycle_recover,
75+
cycle_initial=constraints_for_expression_cycle_initial,
76+
)]
7377
fn all_narrowing_constraints_for_expression<'db>(
7478
db: &'db dyn Db,
7579
expression: Expression<'db>,
@@ -78,14 +82,52 @@ fn all_narrowing_constraints_for_expression<'db>(
7882
}
7983

8084
#[allow(clippy::ref_option)]
81-
#[salsa::tracked(return_ref)]
85+
#[salsa::tracked(
86+
return_ref,
87+
cycle_fn=negative_constraints_for_expression_cycle_recover,
88+
cycle_initial=negative_constraints_for_expression_cycle_initial,
89+
)]
8290
fn all_negative_narrowing_constraints_for_expression<'db>(
8391
db: &'db dyn Db,
8492
expression: Expression<'db>,
8593
) -> Option<NarrowingConstraints<'db>> {
8694
NarrowingConstraintsBuilder::new(db, PredicateNode::Expression(expression), false).finish()
8795
}
8896

97+
#[allow(clippy::ref_option)]
98+
fn constraints_for_expression_cycle_recover<'db>(
99+
_db: &'db dyn Db,
100+
_value: &Option<NarrowingConstraints<'db>>,
101+
_count: u32,
102+
_expression: Expression<'db>,
103+
) -> salsa::CycleRecoveryAction<Option<NarrowingConstraints<'db>>> {
104+
salsa::CycleRecoveryAction::Iterate
105+
}
106+
107+
fn constraints_for_expression_cycle_initial<'db>(
108+
_db: &'db dyn Db,
109+
_expression: Expression<'db>,
110+
) -> Option<NarrowingConstraints<'db>> {
111+
None
112+
}
113+
114+
#[allow(clippy::ref_option)]
115+
fn negative_constraints_for_expression_cycle_recover<'db>(
116+
_db: &'db dyn Db,
117+
_value: &Option<NarrowingConstraints<'db>>,
118+
_count: u32,
119+
_expression: Expression<'db>,
120+
) -> salsa::CycleRecoveryAction<Option<NarrowingConstraints<'db>>> {
121+
salsa::CycleRecoveryAction::Iterate
122+
}
123+
124+
fn negative_constraints_for_expression_cycle_initial<'db>(
125+
_db: &'db dyn Db,
126+
_expression: Expression<'db>,
127+
) -> Option<NarrowingConstraints<'db>> {
128+
None
129+
}
130+
89131
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
90132
pub enum KnownConstraintFunction {
91133
/// `builtins.isinstance`

0 commit comments

Comments
 (0)