Skip to content

Commit c8703cb

Browse files
[ty] Conservative narrowing places optimization (#22734)
## Summary Add helpers to compute a conservative upper bound on the set of places a predicate might actually narrow, and use this to save work when computing narrowing constraints. ## Test Plan Existing tests pass --------- Co-authored-by: David Peter <mail@david-peter.de>
1 parent 789399c commit c8703cb

5 files changed

Lines changed: 289 additions & 24 deletions

File tree

crates/ty_python_semantic/src/semantic_index/builder.rs

Lines changed: 58 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ use crate::semantic_index::use_def::{
5151
};
5252
use crate::semantic_index::{ExpressionsScopeMap, SemanticIndex, VisibleAncestorsIter};
5353
use crate::semantic_model::HasTrackedScope;
54+
use crate::types::PossiblyNarrowedPlaces;
5455
use crate::unpack::{EvaluationMode, Unpack, UnpackKind, UnpackPosition, UnpackValue};
5556
use crate::{Db, Program};
5657

@@ -857,7 +858,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> {
857858

858859
/// Adds a new predicate to the list of all predicates, but does not record it. Returns the
859860
/// predicate ID for later recording using
860-
/// [`SemanticIndexBuilder::record_narrowing_constraint_id`].
861+
/// [`SemanticIndexBuilder::record_narrowing_constraint_id_for_places`].
861862
fn add_predicate(&mut self, predicate: PredicateOrLiteral<'db>) -> ScopedPredicateId {
862863
self.current_use_def_map_mut().add_predicate(predicate)
863864
}
@@ -868,27 +869,70 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> {
868869
.add_predicate(predicate.negated())
869870
}
870871

871-
/// Records a previously added narrowing constraint by adding it to all live bindings.
872-
fn record_narrowing_constraint_id(&mut self, predicate: ScopedPredicateId) {
872+
/// Records a previously added narrowing constraint by adding it to the live bindings
873+
/// of the specified places.
874+
fn record_narrowing_constraint_id_for_places(
875+
&mut self,
876+
predicate: ScopedPredicateId,
877+
places: &PossiblyNarrowedPlaces,
878+
) {
873879
self.current_use_def_map_mut()
874-
.record_narrowing_constraint(predicate);
880+
.record_narrowing_constraint_for_places(predicate, places);
875881
}
876882

877-
/// Adds and records a narrowing constraint, i.e. adds it to all live bindings.
883+
/// Adds and records a narrowing constraint for only the places that could possibly be narrowed.
878884
fn record_narrowing_constraint(&mut self, predicate: PredicateOrLiteral<'db>) {
885+
let possibly_narrowed = self.compute_possibly_narrowed_places(&predicate);
879886
let use_def = self.current_use_def_map_mut();
880887
let predicate_id = use_def.add_predicate(predicate);
881-
use_def.record_narrowing_constraint(predicate_id);
888+
use_def.record_narrowing_constraint_for_places(predicate_id, &possibly_narrowed);
882889
}
883890

884-
/// Negates the given predicate and then adds it as a narrowing constraint to all live
885-
/// bindings.
891+
/// Computes the conservative set of places that could possibly be narrowed by a predicate.
892+
///
893+
/// This uses the closure-based approach to avoid calling Salsa queries that depend on
894+
/// the semantic index (which is still being built).
895+
fn compute_possibly_narrowed_places(
896+
&self,
897+
predicate: &PredicateOrLiteral<'db>,
898+
) -> PossiblyNarrowedPlaces {
899+
use crate::types::PossiblyNarrowedPlacesBuilder;
900+
901+
match predicate {
902+
PredicateOrLiteral::Literal(_) => PossiblyNarrowedPlaces::default(),
903+
PredicateOrLiteral::Predicate(pred) => {
904+
let place_table = self.current_place_table();
905+
906+
match pred.node {
907+
PredicateNode::Expression(expression) => {
908+
let module = self.module;
909+
let expression_node = expression.node_ref(self.db, module);
910+
PossiblyNarrowedPlacesBuilder::new(self.db, place_table)
911+
.expression(expression_node)
912+
}
913+
PredicateNode::Pattern(pattern) => {
914+
let module = self.module;
915+
PossiblyNarrowedPlacesBuilder::new(self.db, place_table)
916+
.pattern(pattern, module)
917+
}
918+
PredicateNode::ReturnsNever(_) | PredicateNode::StarImportPlaceholder(_) => {
919+
// These predicates don't narrow any places
920+
PossiblyNarrowedPlaces::default()
921+
}
922+
}
923+
}
924+
}
925+
}
926+
927+
/// Negates the given predicate and then adds it as a narrowing constraint to the places
928+
/// that could possibly be narrowed.
886929
fn record_negated_narrowing_constraint(
887930
&mut self,
888931
predicate: PredicateOrLiteral<'db>,
889932
) -> ScopedPredicateId {
933+
let possibly_narrowed = self.compute_possibly_narrowed_places(&predicate);
890934
let id = self.add_negated_predicate(predicate);
891-
self.record_narrowing_constraint_id(id);
935+
self.record_narrowing_constraint_id_for_places(id, &possibly_narrowed);
892936
id
893937
}
894938

@@ -2767,6 +2811,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
27672811
// anymore.
27682812
if index < values.len() - 1 {
27692813
let predicate = self.build_predicate(value);
2814+
let possibly_narrowed = self.compute_possibly_narrowed_places(&predicate);
27702815
let predicate_id = match op {
27712816
ast::BoolOp::And => self.add_predicate(predicate),
27722817
ast::BoolOp::Or => self.add_negated_predicate(predicate),
@@ -2789,7 +2834,10 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
27892834
// the application of the reachability constraint until after the expression
27902835
// has been evaluated, so we only push it onto the stack here.
27912836
self.flow_restore(after_expr);
2792-
self.record_narrowing_constraint_id(predicate_id);
2837+
self.record_narrowing_constraint_id_for_places(
2838+
predicate_id,
2839+
&possibly_narrowed,
2840+
);
27932841
reachability_constraints.push(reachability_constraint);
27942842
}
27952843
}

crates/ty_python_semantic/src/semantic_index/place.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ pub(crate) struct PlaceTableBuilder {
261261

262262
impl PlaceTableBuilder {
263263
/// Looks up a place ID by its expression.
264-
pub(super) fn place_id(&self, expression: PlaceExprRef) -> Option<ScopedPlaceId> {
264+
pub(crate) fn place_id(&self, expression: PlaceExprRef) -> Option<ScopedPlaceId> {
265265
match expression {
266266
PlaceExprRef::Symbol(symbol) => self.symbols.symbol_id(symbol.name()).map(Into::into),
267267
PlaceExprRef::Member(member) => {

crates/ty_python_semantic/src/semantic_index/use_def.rs

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,9 @@ use crate::semantic_index::use_def::place_state::{
266266
LiveDeclarationsIterator, PlaceState, PreviousDefinitions, ScopedDefinitionId,
267267
};
268268
use crate::semantic_index::{EnclosingSnapshotResult, SemanticIndex};
269-
use crate::types::{NarrowingConstraint, Truthiness, Type, infer_narrowing_constraint};
269+
use crate::types::{
270+
NarrowingConstraint, PossiblyNarrowedPlaces, Truthiness, Type, infer_narrowing_constraint,
271+
};
270272

271273
mod place_state;
272274

@@ -1002,7 +1004,12 @@ impl<'db> UseDefMapBuilder<'db> {
10021004
}
10031005
}
10041006

1005-
pub(super) fn record_narrowing_constraint(&mut self, predicate: ScopedPredicateId) {
1007+
/// Records a narrowing constraint for only the specified places.
1008+
pub(super) fn record_narrowing_constraint_for_places(
1009+
&mut self,
1010+
predicate: ScopedPredicateId,
1011+
places: &PossiblyNarrowedPlaces,
1012+
) {
10061013
if predicate == ScopedPredicateId::ALWAYS_TRUE
10071014
|| predicate == ScopedPredicateId::ALWAYS_FALSE
10081015
{
@@ -1011,14 +1018,25 @@ impl<'db> UseDefMapBuilder<'db> {
10111018
}
10121019

10131020
let narrowing_constraint = predicate.into();
1014-
for state in &mut self.symbol_states {
1015-
state
1016-
.record_narrowing_constraint(&mut self.narrowing_constraints, narrowing_constraint);
1017-
}
1018-
1019-
for state in &mut self.member_states {
1020-
state
1021-
.record_narrowing_constraint(&mut self.narrowing_constraints, narrowing_constraint);
1021+
for place in places {
1022+
match place {
1023+
ScopedPlaceId::Symbol(symbol_id) => {
1024+
if let Some(state) = self.symbol_states.get_mut(*symbol_id) {
1025+
state.record_narrowing_constraint(
1026+
&mut self.narrowing_constraints,
1027+
narrowing_constraint,
1028+
);
1029+
}
1030+
}
1031+
ScopedPlaceId::Member(member_id) => {
1032+
if let Some(state) = self.member_states.get_mut(*member_id) {
1033+
state.record_narrowing_constraint(
1034+
&mut self.narrowing_constraints,
1035+
narrowing_constraint,
1036+
);
1037+
}
1038+
}
1039+
}
10221040
}
10231041
}
10241042

crates/ty_python_semantic/src/types.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,10 @@ use crate::types::generics::{
6868
typing_self, walk_generic_context,
6969
};
7070
use crate::types::mro::{Mro, MroIterator, StaticMroError};
71-
pub(crate) use crate::types::narrow::{NarrowingConstraint, infer_narrowing_constraint};
71+
pub(crate) use crate::types::narrow::{
72+
NarrowingConstraint, PossiblyNarrowedPlaces, PossiblyNarrowedPlacesBuilder,
73+
infer_narrowing_constraint,
74+
};
7275
use crate::types::newtype::NewType;
7376
pub(crate) use crate::types::signatures::{Parameter, Parameters};
7477
use crate::types::signatures::{ParameterForm, walk_signature};

0 commit comments

Comments
 (0)