language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-09-29 13:25:17 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[red-knot] Better handling of visibility constraint copies (#16276)

Browse source 
Two related changes.  For context:

1. We were maintaining two separate arenas of `Constraint`s in each
use-def map. One was used for narrowing constraints, and the other for
visibility constraints. The visibility constraint arena was interned,
ensuring that we always used the same ID for any particular
`Constraint`. The narrowing constraint arena was not interned.

2. The TDD code relies on _all_ TDD nodes being interned and reduced.
This is an important requirement for TDDs to be a canonical form, which
allows us to use a single int comparison to test for "always true/false"
and to compare two TDDs for equivalence. But we also need to support an
individual `Constraint` having multiple values in a TDD evaluation (e.g.
to handle a `while` condition having different values the first time
it's evaluated vs later times). Previously, we handled that by
introducing a "copy" number, which was only there as a disambiguator, to
allow an interned, deduplicated constraint ID to appear in the TDD
formula multiple times.

A better way to handle (2) is to not intern the constraints in the
visibility constraint arena! The caller now gets to decide: if they add
a `Constraint` to the arena more than once, they get distinct
`ScopedConstraintId`s — which the TDD code will treat as distinct
variables, allowing them to take on different values in the ternary
function.

With that in place, we can then consolidate on a single (non-interned)
arena, which is shared for both narrowing and visibility constraints.

---------

Co-authored-by: Carl Meyer <carl@astral.sh>
This commit is contained in:
Douglas Creager 2025-02-21 09:16:25 -05:00 • committed by GitHub
parent b9b094869a
commit 4dae09ecff
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 113 additions and 139 deletions
Download patch file Download diff file Expand all files Collapse all files

31
crates/red_knot_python_semantic/src/semantic_index/builder.rs
View file

@ -15,7 +15,7 @@ use crate::module_name::ModuleName;
use crate::semantic_index::ast_ids::node_key::ExpressionNodeKey;
use crate::semantic_index::ast_ids::AstIdsBuilder;
use crate::semantic_index::attribute_assignment::{AttributeAssignment, AttributeAssignments};
use crate::semantic_index::constraint::PatternConstraintKind;
use crate::semantic_index::constraint::{PatternConstraintKind, ScopedConstraintId};
use crate::semantic_index::definition::{
AssignmentDefinitionNodeRef, ComprehensionDefinitionNodeRef, Definition, DefinitionNodeKey,
DefinitionNodeRef, ForStmtDefinitionNodeRef, ImportFromDefinitionNodeRef,
@ -26,7 +26,7 @@ use crate::semantic_index::symbol::{
SymbolTableBuilder,
};
use crate::semantic_index::use_def::{
EagerBindingsKey, FlowSnapshot, ScopedConstraintId, ScopedEagerBindingsId, UseDefMapBuilder,
EagerBindingsKey, FlowSnapshot, ScopedEagerBindingsId, UseDefMapBuilder,
};
use crate::semantic_index::SemanticIndex;
use crate::unpack::{Unpack, UnpackValue};
@ -294,7 +294,7 @@ impl<'db> SemanticIndexBuilder<'db> {
&self.use_def_maps[scope_id]
}
fn current_visibility_constraints_mut(&mut self) -> &mut VisibilityConstraintsBuilder<'db> {
fn current_visibility_constraints_mut(&mut self) -> &mut VisibilityConstraintsBuilder {
let scope_id = self.current_scope();
&mut self.use_def_maps[scope_id].visibility_constraints
}
@ -406,16 +406,12 @@ impl<'db> SemanticIndexBuilder<'db> {
}
/// Negates a constraint and adds it to the list of all constraints, does not record it.
fn add_negated_constraint(
&mut self,
constraint: Constraint<'db>,
) -> (Constraint<'db>, ScopedConstraintId) {
fn add_negated_constraint(&mut self, constraint: Constraint<'db>) -> ScopedConstraintId {
let negated = Constraint {
node: constraint.node,
is_positive: false,
};
let id = self.current_use_def_map_mut().add_constraint(negated);
(negated, id)
self.current_use_def_map_mut().add_constraint(negated)
}
/// Records a previously added constraint by adding it to all live bindings.
@ -431,7 +427,7 @@ impl<'db> SemanticIndexBuilder<'db> {
/// Negates the given constraint and then adds it to all live bindings.
fn record_negated_constraint(&mut self, constraint: Constraint<'db>) -> ScopedConstraintId {
let (_, id) = self.add_negated_constraint(constraint);
let id = self.add_negated_constraint(constraint);
self.record_constraint_id(id);
id
}
@ -460,9 +456,10 @@ impl<'db> SemanticIndexBuilder<'db> {
&mut self,
constraint: Constraint<'db>,
) -> ScopedVisibilityConstraintId {
let constraint_id = self.current_use_def_map_mut().add_constraint(constraint);
let id = self
.current_visibility_constraints_mut()
.add_atom(constraint, 0);
.add_atom(constraint_id);
self.record_visibility_constraint_id(id);
id
}
@ -1192,12 +1189,14 @@ where
// We need multiple copies of the visibility constraint for the while condition,
// since we need to model situations where the first evaluation of the condition
// returns True, but a later evaluation returns False.
let first_constraint_id = self.current_use_def_map_mut().add_constraint(constraint);
let later_constraint_id = self.current_use_def_map_mut().add_constraint(constraint);
let first_vis_constraint_id = self
.current_visibility_constraints_mut()
.add_atom(constraint, 0);
.add_atom(first_constraint_id);
let later_vis_constraint_id = self
.current_visibility_constraints_mut()
.add_atom(constraint, 1);
.add_atom(later_constraint_id);
// Save aside any break states from an outer loop
let saved_break_states = std::mem::take(&mut self.loop_break_states);
@ -1778,13 +1777,13 @@ where
// anymore.
if index < values.len() - 1 {
let constraint = self.build_constraint(value);
let (constraint, constraint_id) = match op {
ast::BoolOp::And => (constraint, self.add_constraint(constraint)),
let constraint_id = match op {
ast::BoolOp::And => self.add_constraint(constraint),
ast::BoolOp::Or => self.add_negated_constraint(constraint),
};
let visibility_constraint = self
.current_visibility_constraints_mut()
.add_atom(constraint, 0);
.add_atom(constraint_id);
let after_expr = self.flow_snapshot();