[ty] Fix a panic when checking truthiness in a loop (#29759)

## Summary

Ty could panic on `assert predicate` inside a loop where `predicate`
starts as a lambda and a later iteration rebinds it to a class. When
cycle recovery added the class to the operand type, ty looked up its
metaclass to determine its truthiness. That lookup could introduce a new
inference cycle while Salsa was recovering the first one.

Reachability now evaluates the predicate from the operand types that
expression inference has already normalized. Outside loops, it caches
the result so repeated walks of the same condition can reuse it. Inside
a loop, a condition can depend on an assignment it guards, so the caller
reads the inferred operands directly. Expression inference no longer
stores predicate truthiness or recomputes it during recovery.

The 163-project ecosystem run produced identical diagnostics. Checking
isort took 291 ms, up from 248 ms on the base revision, which gives back
some of the speedup from #29706. The average timing factor across
projects was 1.02×. CodSpeed found no significant changes among the 214
benchmarks it compared. [Ecosystem
diff](https://dqhfo45wdv0ek.cloudfront.net/ruff/pull-requests/29759/ty-ecosystem/40467940782/1/diff.html)
·
[Timings](https://dqhfo45wdv0ek.cloudfront.net/ruff/pull-requests/29759/ty-ecosystem/40467940782/1/timing.html)
·
[CodSpeed](https://github.com/astral-sh/ruff/pull/29759#issuecomment-6435864937)

Closes https://github.com/astral-sh/ty/issues/4834.
This commit is contained in:
Charlie Marsh authored and GitHub committed 2026-10-27 07:30:58 -07:00
1 parent fbf83e2268
commit 691b60a3f3
6 files changed
+51 -54

No files matched your search

+1 -1
View File
@@ -91,7 +91,7 @@ pub enum ExpressionContext {
pub enum ExpressionKind {
/// An ordinary value expression, such as `1` in `self.x: int = 1`.
Normal,
/// A predicate whose truthiness is recorded in its evaluation context.
/// A predicate evaluated in value or condition context.
Predicate {
context: ExpressionContext,
in_loop: bool,
@@ -0,0 +1,11 @@
# Regression test for https://github.com/astral-sh/ty/issues/4834
class C("", (lambda: value, (predicate := lambda: value))):
pass
# Read the lambda before the loop adds the class as another possible value of predicate.
predicate
while True:
assert predicate
value = 0
predicate = C
@@ -1943,6 +1943,21 @@ for _ in range(1_000_000):
x = 2
```
The condition can depend on an earlier iteration. Here `y` starts at `2` and only ever receives `1`
or `2` from `x`, so the loop never decrements `x`.
```py
def f(n: int):
x = 1
y = 2
for _ in range(n):
if not y:
x, y = x - 1, x
else:
x, y = 2, x
reveal_type(x) # revealed: Literal[1, 2]
```
### Large reachability constraint graphs fall back to `Unknown`
```py
+24 -51
View File
@@ -45,7 +45,7 @@
use crate::ProgramEnvironment;
use itertools::Either;
use ruff_db::parsed::{ParsedModuleRef, parsed_module};
use ruff_db::parsed::parsed_module;
use ruff_python_ast as ast;
use ruff_text_size::{Ranged, TextRange};
use rustc_hash::FxHashMap;
@@ -579,7 +579,7 @@ pub(crate) fn infer_expression_types<'db>(
}
/// A loop predicate can depend on bindings it guards in the previous iteration.
/// Reading inference directly keeps the extra truthiness query out of that cycle.
/// Evaluate it in the caller's query to keep an extra query out of that cycle.
pub(crate) fn infer_predicate_truthiness<'db>(
db: &'db dyn Db,
expression: Expression<'db>,
@@ -588,24 +588,38 @@ pub(crate) fn infer_predicate_truthiness<'db>(
expression.kind(db),
ExpressionKind::Predicate { in_loop: true, .. }
) {
recorded_predicate_truthiness(db, expression)
evaluate_predicate_truthiness(db, expression)
} else {
cached_predicate_truthiness(db, expression)
}
}
/// Keep reachability queries cached when an operand's type changes but its truthiness stays the same.
/// Reachability walks can revisit the same predicate for many different bindings.
#[salsa::tracked(
returns(copy),
cycle_initial = |_, _, _| Truthiness::Ambiguous,
heap_size = get_size2::GetSize::get_heap_size
)]
fn cached_predicate_truthiness<'db>(db: &'db dyn Db, expression: Expression<'db>) -> Truthiness {
recorded_predicate_truthiness(db, expression)
evaluate_predicate_truthiness(db, expression)
}
fn recorded_predicate_truthiness<'db>(db: &'db dyn Db, expression: Expression<'db>) -> Truthiness {
infer_expression_types_impl(db, InferExpression::Bare(expression)).predicate_truthiness
/// A widened operand can require a new metaclass lookup, so evaluate truthiness after
/// expression inference has normalized the operand types.
fn evaluate_predicate_truthiness<'db>(db: &'db dyn Db, expression: Expression<'db>) -> Truthiness {
let ExpressionKind::Predicate { context, .. } = expression.kind(db) else {
return Truthiness::Ambiguous;
};
let inference = infer_expression_types(db, expression, TypeContext::default());
let env = ProgramEnvironment::from_scope(expression.scope(db));
let module = parsed_module(db, expression.python_file(db)).load(db);
TruthinessAnalyzer::new(
db,
&env,
|node| inference.expression_type(node),
|node| inference.comparison_truthiness(node),
)
.truthiness(expression.node_ref(db).node(&module), context)
}
#[salsa::tracked(
@@ -614,7 +628,7 @@ fn recorded_predicate_truthiness<'db>(db: &'db dyn Db, expression: Expression<'d
cycle_fn=|db, cycle, previous: &ExpressionInference<'db>, inference: ExpressionInference<'db>, input: InferExpression<'db>| {
let (expression, _) = input.into_inner(db);
let env = ProgramEnvironment::from_scope(expression.scope(db));
inference.cycle_normalized(db, &env, previous, cycle, input)
inference.cycle_normalized(db, &env, previous, cycle)
},
heap_size=ruff_memory_usage::heap_size
)]
@@ -639,7 +653,7 @@ pub(super) fn infer_expression_types_impl<'db>(
let env = ProgramEnvironment::from_file(program_file);
let mut inference = TypeInferenceBuilder::new(
TypeInferenceBuilder::new(
db,
&env,
InferenceRegion::Expression(expression, tcx),
@@ -648,9 +662,7 @@ pub(super) fn infer_expression_types_impl<'db>(
index,
&module,
)
.finish_expression();
inference.record_predicate_truthiness(db, &env, input, Some(module));
inference
.finish_expression()
}
fn expression_cycle_initial<'db>(
@@ -2027,9 +2039,6 @@ pub(crate) struct ExpressionInference<'db> {
/// The types of every expression in this region.
expressions: FrozenMap<ExpressionNodeKey, Type<'db>>,
/// The predicate's possible outcomes in its evaluation context.
predicate_truthiness: Truthiness,
extra: Option<Box<ExpressionInferenceExtra<'db>>>,
/// The scope this region is part of.
@@ -2098,31 +2107,6 @@ struct ExpressionInferenceExtra<'db> {
}
impl<'db> ExpressionInference<'db> {
/// Reachability always asks for a predicate's truthiness without an expected type.
fn record_predicate_truthiness(
&mut self,
db: &'db dyn Db,
env: &ProgramEnvironment<'db>,
input: InferExpression<'db>,
module: Option<ParsedModuleRef>,
) {
let InferExpression::Bare(expression) = input else {
return;
};
let ExpressionKind::Predicate { context, .. } = expression.kind(db) else {
return;
};
let module =
module.unwrap_or_else(|| parsed_module(db, expression.python_file(db)).load(db));
self.predicate_truthiness = TruthinessAnalyzer::new(
db,
env,
|node| self.expression_type(node),
|node| self.comparison_truthiness(node),
)
.truthiness(expression.node_ref(db).node(&module), context);
}
fn cycle_initial(scope: ScopeId<'db>, cycle_recovery: Type<'db>) -> Self {
let _ = scope;
Self {
@@ -2130,7 +2114,6 @@ impl<'db> ExpressionInference<'db> {
cycle_recovery: Some(cycle_recovery),
..ExpressionInferenceExtra::default()
})),
predicate_truthiness: Truthiness::Ambiguous,
expressions: FrozenMap::default(),
#[cfg(debug_assertions)]
scope,
@@ -2143,7 +2126,6 @@ impl<'db> ExpressionInference<'db> {
env: &ProgramEnvironment<'db>,
previous: &ExpressionInference<'db>,
cycle: &salsa::Cycle,
input: InferExpression<'db>,
) -> ExpressionInference<'db> {
if let Some(extra) = self.extra.as_mut() {
for (binding, binding_ty) in &mut extra.bindings {
@@ -2169,15 +2151,6 @@ impl<'db> ExpressionInference<'db> {
*ty = ty.cycle_normalized(db, env, previous_ty, cycle);
}
// Normalizing the operands can change whether the predicate completes. Once the first
// provisional results have cleared, retain earlier outcomes so the predicate converges.
self.record_predicate_truthiness(db, env, input, None);
if cycle.iteration() > crate::TAINTED_CYCLES {
self.predicate_truthiness = previous
.predicate_truthiness
.union(self.predicate_truthiness);
}
if cycle.iteration() > crate::TAINTED_CYCLES
&& let Some(previous_extra) = previous.extra.as_deref()
&& !previous_extra.collection_use_constraints.is_empty()
@@ -12949,7 +12949,6 @@ impl<'db> FullExpressionCacheEntry<'db> {
});
ExpressionInference {
predicate_truthiness: Truthiness::Ambiguous,
expressions: FrozenMap::from(self.expressions),
extra,
#[cfg(debug_assertions)]
@@ -574,7 +574,6 @@ fn comparison_truthiness_widens_across_sparse_cycle_results() -> anyhow::Result<
let inference = |ty, truthiness: Option<Truthiness>| {
(
ExpressionInference {
predicate_truthiness: Truthiness::Ambiguous,
expressions: [(expression, ty)].into_iter().collect(),
extra: truthiness.map(|truthiness| {
Box::new(ExpressionInferenceExtra {