But that doesn't work either

(because OR-ing reachability on merge throws away the AlwaysFalse)
astral-sh · Jan 30, 2025 · 0f278da · 0f278da
1 parent 999188a
commit 0f278da
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 38 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,6 +477,7 @@ 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> {
@@ -505,6 +506,8 @@ 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> {
@@ -518,11 +521,13 @@ 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) {
@@ -617,10 +622,7 @@ 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
-            .visibility_constraints
-            .contains_always_false(self.scope_start_visibility)
-        {
+        if !self.always_reachable {
             return;
         }
 
@@ -677,6 +679,7 @@ impl<'db> UseDefMapBuilder<'db> {
         FlowSnapshot {
             symbol_states: self.symbol_states.clone(),
             scope_start_visibility: self.scope_start_visibility,
+            always_reachable: self.always_reachable,
         }
     }
 
@@ -691,6 +694,7 @@ 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
@@ -717,6 +721,7 @@ impl<'db> UseDefMapBuilder<'db> {
             .evaluate_without_inference(self.db, snapshot.scope_start_visibility)
             .is_always_false()
         {
+            self.always_reachable = false;
             return;
         }
         if self
@@ -725,6 +730,7 @@ impl<'db> UseDefMapBuilder<'db> {
             .is_always_false()
         {
             self.restore(snapshot);
+            self.always_reachable = false;
             return;
         }
 
@@ -749,6 +755,7 @@ 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,40 +273,6 @@ 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)