Try checking if reachability contains AlwaysFalse in syntax tree

astral-sh · Jan 30, 2025 · 999188a · 999188a
1 parent 46b1ec2
commit 999188a
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 11 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
@@ -477,7 +477,6 @@ impl std::iter::FusedIterator for DeclarationsIterator<'_, '_> {}
 pub(super) struct FlowSnapshot {
     symbol_states: IndexVec<ScopedSymbolId, SymbolState>,
     scope_start_visibility: ScopedVisibilityConstraintId,
-    always_reachable: bool,
 }
 
 pub(super) struct UseDefMapBuilder<'db> {
@@ -506,8 +505,6 @@ pub(super) struct UseDefMapBuilder<'db> {
 
     /// Currently live bindings and declarations for each symbol.
     symbol_states: IndexVec<ScopedSymbolId, SymbolState>,
-
-    always_reachable: bool,
 }
 
 impl<'db> UseDefMapBuilder<'db> {
@@ -521,13 +518,11 @@ impl<'db> UseDefMapBuilder<'db> {
             bindings_by_use: IndexVec::new(),
             definitions_by_definition: FxHashMap::default(),
             symbol_states: IndexVec::new(),
-            always_reachable: true,
         }
     }
 
     pub(super) fn mark_unreachable(&mut self) {
         self.record_visibility_constraint_id(ScopedVisibilityConstraintId::ALWAYS_FALSE);
-        self.always_reachable = false;
     }
 
     pub(super) fn add_symbol(&mut self, symbol: ScopedSymbolId) {
@@ -622,7 +617,10 @@ impl<'db> UseDefMapBuilder<'db> {
     pub(super) fn simplify_visibility_constraints(&mut self, snapshot: FlowSnapshot) {
         debug_assert!(self.symbol_states.len() >= snapshot.symbol_states.len());
 
-        if !self.always_reachable {
+        if self
+            .visibility_constraints
+            .contains_always_false(self.scope_start_visibility)
+        {
             return;
         }
 
@@ -679,7 +677,6 @@ impl<'db> UseDefMapBuilder<'db> {
         FlowSnapshot {
             symbol_states: self.symbol_states.clone(),
             scope_start_visibility: self.scope_start_visibility,
-            always_reachable: self.always_reachable,
         }
     }
 
@@ -694,7 +691,6 @@ impl<'db> UseDefMapBuilder<'db> {
         // Restore the current visible-definitions state to the given snapshot.
         self.symbol_states = snapshot.symbol_states;
         self.scope_start_visibility = snapshot.scope_start_visibility;
-        self.always_reachable = snapshot.always_reachable;
 
         // If the snapshot we are restoring is missing some symbols we've recorded since, we need
         // to fill them in so the symbol IDs continue to line up. Since they don't exist in the
@@ -721,7 +717,6 @@ impl<'db> UseDefMapBuilder<'db> {
             .evaluate_without_inference(self.db, snapshot.scope_start_visibility)
             .is_always_false()
         {
-            self.always_reachable = false;
             return;
         }
         if self
@@ -730,7 +725,6 @@ impl<'db> UseDefMapBuilder<'db> {
             .is_always_false()
         {
             self.restore(snapshot);
-            self.always_reachable = false;
             return;
         }
 
@@ -755,7 +749,6 @@ impl<'db> UseDefMapBuilder<'db> {
         self.scope_start_visibility = self
             .visibility_constraints
             .add_or_constraint(self.scope_start_visibility, snapshot.scope_start_visibility);
-        self.always_reachable &= snapshot.always_reachable;
     }
 
     pub(super) fn finish(mut self) -> UseDefMap<'db> {

diff --git a/crates/red_knot_python_semantic/src/visibility_constraints.rs b/crates/red_knot_python_semantic/src/visibility_constraints.rs
@@ -273,6 +273,40 @@ impl<'db> VisibilityConstraints<'db> {
         self.evaluate_impl::<false>(db, id, MAX_RECURSION_DEPTH)
     }
 
+    pub(crate) fn contains_always_false(&self, id: ScopedVisibilityConstraintId) -> bool {
+        let visibility_constraint = &self.constraints[id];
+        match visibility_constraint {
+            VisibilityConstraint::AlwaysTrue => false,
+            VisibilityConstraint::AlwaysFalse => true,
+            VisibilityConstraint::Ambiguous => false,
+            VisibilityConstraint::VisibleIf(_) => false,
+            VisibilityConstraint::VisibleIfNot(negated) => self.contains_always_true(*negated),
+            VisibilityConstraint::KleeneAnd(lhs, rhs) => {
+                self.contains_always_false(*lhs) || self.contains_always_false(*rhs)
+            }
+            VisibilityConstraint::KleeneOr(lhs, rhs) => {
+                self.contains_always_false(*lhs) || self.contains_always_false(*rhs)
+            }
+        }
+    }
+
+    pub(crate) fn contains_always_true(&self, id: ScopedVisibilityConstraintId) -> bool {
+        let visibility_constraint = &self.constraints[id];
+        match visibility_constraint {
+            VisibilityConstraint::AlwaysTrue => true,
+            VisibilityConstraint::AlwaysFalse => false,
+            VisibilityConstraint::Ambiguous => false,
+            VisibilityConstraint::VisibleIf(_) => false,
+            VisibilityConstraint::VisibleIfNot(negated) => self.contains_always_false(*negated),
+            VisibilityConstraint::KleeneAnd(lhs, rhs) => {
+                self.contains_always_true(*lhs) || self.contains_always_true(*rhs)
+            }
+            VisibilityConstraint::KleeneOr(lhs, rhs) => {
+                self.contains_always_true(*lhs) || self.contains_always_true(*rhs)
+            }
+        }
+    }
+
     /// Analyze the statically known visibility for a given visibility constraint.
     pub(crate) fn evaluate(&self, db: &'db dyn Db, id: ScopedVisibilityConstraintId) -> Truthiness {
         self.evaluate_impl::<true>(db, id, MAX_RECURSION_DEPTH)