Skip to content

Commit d9aaacd

Browse files
Glyphacksharkdp
andauthored
[ty] Evaluate reachability of non-definitely-bound to Ambiguous (#19579)
## Summary closes astral-sh/ty#692 If the expression (or any child expressions) is not definitely bound the reachability constraint evaluation is determined as ambiguous. This fixes the infinite cycles panic in the following code: ```py from typing import Literal class Toggle: def __init__(self: "Toggle"): if not self.x: self.x: Literal[True] = True ``` Credit of this solution is for David. ## Test Plan - Added a test case with too many cycle iterations panic. - Previous tests. --------- Co-authored-by: David Peter <[email protected]>
1 parent 18eaa65 commit d9aaacd

File tree

8 files changed

+153
-27
lines changed

8 files changed

+153
-27
lines changed

crates/ty_python_semantic/resources/mdtest/attributes.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2288,6 +2288,26 @@ class H:
22882288
self.x = other.x or self.x
22892289
```
22902290

2291+
An attribute definition can be guarded by a condition involving that attribute. This is a regression
2292+
test for <https://github.com/astral-sh/ty/issues/692>:
2293+
2294+
```py
2295+
from typing import Literal
2296+
2297+
def check(x) -> Literal[False]:
2298+
return False
2299+
2300+
class Toggle:
2301+
def __init__(self: "Toggle"):
2302+
if not self.x:
2303+
self.x: Literal[True] = True
2304+
if check(self.y):
2305+
self.y = True
2306+
2307+
reveal_type(Toggle().x) # revealed: Literal[True]
2308+
reveal_type(Toggle().y) # revealed: Unknown | Literal[True]
2309+
```
2310+
22912311
### Builtin types attributes
22922312

22932313
This test can probably be removed eventually, but we currently include it because we do not yet

crates/ty_python_semantic/resources/mdtest/statically_known_branches.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1564,6 +1564,24 @@ if True:
15641564
from module import symbol
15651565
```
15661566

1567+
## Non-definitely bound symbols in conditions
1568+
1569+
When a non-definitely bound symbol is used as a (part of a) condition, we always infer an ambiguous
1570+
truthiness. If we didn't do that, `x` would be considered definitely bound in the following example:
1571+
1572+
```py
1573+
def _(flag: bool):
1574+
if flag:
1575+
ALWAYS_TRUE_IF_BOUND = True
1576+
1577+
# error: [possibly-unresolved-reference] "Name `ALWAYS_TRUE_IF_BOUND` used when possibly not defined"
1578+
if True and ALWAYS_TRUE_IF_BOUND:
1579+
x = 1
1580+
1581+
# error: [possibly-unresolved-reference] "Name `x` used when possibly not defined"
1582+
x
1583+
```
1584+
15671585
## Unreachable code
15681586

15691587
A closely related feature is the ability to detect unreachable code. For example, we do not emit a

crates/ty_python_semantic/src/place.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,10 @@ impl<'db> Place<'db> {
135135
Place::Unbound => Place::Unbound,
136136
}
137137
}
138+
139+
pub(crate) const fn is_definitely_bound(&self) -> bool {
140+
matches!(self, Place::Type(_, Boundness::Bound))
141+
}
138142
}
139143

140144
impl<'db> From<LookupResult<'db>> for PlaceAndQualifiers<'db> {

crates/ty_python_semantic/src/semantic_index/reachability_constraints.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ use crate::semantic_index::predicate::{
209209
};
210210
use crate::types::{
211211
IntersectionBuilder, Truthiness, Type, UnionBuilder, UnionType, infer_expression_type,
212+
static_expression_truthiness,
212213
};
213214

214215
/// A ternary formula that defines under what conditions a binding is visible. (A ternary formula
@@ -821,8 +822,7 @@ impl ReachabilityConstraints {
821822
fn analyze_single(db: &dyn Db, predicate: &Predicate) -> Truthiness {
822823
match predicate.node {
823824
PredicateNode::Expression(test_expr) => {
824-
let ty = infer_expression_type(db, test_expr);
825-
ty.bool(db).negate_if(!predicate.is_positive)
825+
static_expression_truthiness(db, test_expr).negate_if(!predicate.is_positive)
826826
}
827827
PredicateNode::ReturnsNever(CallableAndCallExpr {
828828
callable,

crates/ty_python_semantic/src/semantic_index/use_def.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -598,7 +598,7 @@ impl<'db> UseDefMap<'db> {
598598
.is_always_false()
599599
}
600600

601-
pub(crate) fn is_declaration_reachable(
601+
pub(crate) fn declaration_reachability(
602602
&self,
603603
db: &dyn crate::Db,
604604
declaration: &DeclarationWithConstraint<'db>,
@@ -610,7 +610,7 @@ impl<'db> UseDefMap<'db> {
610610
)
611611
}
612612

613-
pub(crate) fn is_binding_reachable(
613+
pub(crate) fn binding_reachability(
614614
&self,
615615
db: &dyn crate::Db,
616616
binding: &BindingWithConstraints<'_, 'db>,

crates/ty_python_semantic/src/types.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ pub use self::diagnostic::TypeCheckDiagnostics;
2424
pub(crate) use self::diagnostic::register_lints;
2525
pub(crate) use self::infer::{
2626
infer_deferred_types, infer_definition_types, infer_expression_type, infer_expression_types,
27-
infer_scope_types,
27+
infer_scope_types, static_expression_truthiness,
2828
};
2929
pub(crate) use self::signatures::{CallableSignature, Signature};
3030
pub(crate) use self::subclass_of::{SubclassOfInner, SubclassOfType};
@@ -6910,6 +6910,10 @@ bitflags! {
69106910
const NOT_REQUIRED = 1 << 4;
69116911
/// `typing_extensions.ReadOnly`
69126912
const READ_ONLY = 1 << 5;
6913+
/// An implicit instance attribute which is possibly unbound according
6914+
/// to local control flow within the method it is defined in. This flag
6915+
/// overrules the `Boundness` information on `PlaceAndQualifiers`.
6916+
const POSSIBLY_UNBOUND_IMPLICIT_ATTRIBUTE = 1 << 6;
69136917
}
69146918
}
69156919

@@ -8620,7 +8624,7 @@ impl TypeRelation {
86208624
}
86218625
}
86228626

8623-
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
8627+
#[derive(Debug, Copy, Clone, PartialEq, Eq, get_size2::GetSize)]
86248628
pub enum Truthiness {
86258629
/// For an object `x`, `bool(x)` will always return `True`
86268630
AlwaysTrue,

crates/ty_python_semantic/src/types/class.rs

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2736,17 +2736,20 @@ impl<'db> ClassLiteral<'db> {
27362736
// self.name: <annotation>
27372737
// self.name: <annotation> = …
27382738

2739-
if use_def_map(db, method_scope)
2740-
.is_declaration_reachable(db, &attribute_declaration)
2741-
.is_always_false()
2742-
{
2739+
let reachability = use_def_map(db, method_scope)
2740+
.declaration_reachability(db, &attribute_declaration);
2741+
2742+
if reachability.is_always_false() {
27432743
continue;
27442744
}
27452745

27462746
let annotation = declaration_type(db, declaration);
2747-
let annotation =
2747+
let mut annotation =
27482748
Place::bound(annotation.inner).with_qualifiers(annotation.qualifiers);
27492749

2750+
if reachability.is_ambiguous() {
2751+
annotation.qualifiers |= TypeQualifiers::POSSIBLY_UNBOUND_IMPLICIT_ATTRIBUTE;
2752+
}
27502753
if let Some(all_qualifiers) = annotation.is_bare_final() {
27512754
if let Some(value) = assignment.value(&module) {
27522755
// If we see an annotated assignment with a bare `Final` as in
@@ -2789,7 +2792,7 @@ impl<'db> ClassLiteral<'db> {
27892792
.all_reachable_symbol_bindings(method_place)
27902793
.find_map(|bind| {
27912794
(bind.binding.is_defined_and(|def| def == method))
2792-
.then(|| class_map.is_binding_reachable(db, &bind))
2795+
.then(|| class_map.binding_reachability(db, &bind))
27932796
})
27942797
.unwrap_or(Truthiness::AlwaysFalse)
27952798
} else {
@@ -2818,11 +2821,15 @@ impl<'db> ClassLiteral<'db> {
28182821
continue;
28192822
};
28202823
match method_map
2821-
.is_binding_reachable(db, &attribute_assignment)
2824+
.binding_reachability(db, &attribute_assignment)
28222825
.and(is_method_reachable)
28232826
{
2824-
Truthiness::AlwaysTrue | Truthiness::Ambiguous => {
2827+
Truthiness::AlwaysTrue => {
2828+
is_attribute_bound = true;
2829+
}
2830+
Truthiness::Ambiguous => {
28252831
is_attribute_bound = true;
2832+
qualifiers |= TypeQualifiers::POSSIBLY_UNBOUND_IMPLICIT_ATTRIBUTE;
28262833
}
28272834
Truthiness::AlwaysFalse => {
28282835
continue;
@@ -2834,7 +2841,7 @@ impl<'db> ClassLiteral<'db> {
28342841
// TODO: this is incomplete logic since the attributes bound after termination are considered reachable.
28352842
let unbound_reachability = unbound_binding
28362843
.as_ref()
2837-
.map(|binding| method_map.is_binding_reachable(db, binding))
2844+
.map(|binding| method_map.binding_reachability(db, binding))
28382845
.unwrap_or(Truthiness::AlwaysFalse);
28392846

28402847
if unbound_reachability

crates/ty_python_semantic/src/types/infer.rs

Lines changed: 85 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,45 @@ fn single_expression_cycle_initial<'db>(
337337
Type::Never
338338
}
339339

340+
/// Returns the statically-known truthiness of a given expression.
341+
///
342+
/// Returns [`Truthiness::Ambiguous`] in case any non-definitely bound places
343+
/// were encountered while inferring the type of the expression.
344+
#[salsa::tracked(cycle_fn=static_expression_truthiness_cycle_recover, cycle_initial=static_expression_truthiness_cycle_initial, heap_size=get_size2::GetSize::get_heap_size)]
345+
pub(crate) fn static_expression_truthiness<'db>(
346+
db: &'db dyn Db,
347+
expression: Expression<'db>,
348+
) -> Truthiness {
349+
let inference = infer_expression_types(db, expression);
350+
351+
if !inference.all_places_definitely_bound() {
352+
return Truthiness::Ambiguous;
353+
}
354+
355+
let file = expression.file(db);
356+
let module = parsed_module(db, file).load(db);
357+
let node = expression.node_ref(db, &module);
358+
359+
inference.expression_type(node).bool(db)
360+
}
361+
362+
#[expect(clippy::trivially_copy_pass_by_ref)]
363+
fn static_expression_truthiness_cycle_recover<'db>(
364+
_db: &'db dyn Db,
365+
_value: &Truthiness,
366+
_count: u32,
367+
_expression: Expression<'db>,
368+
) -> salsa::CycleRecoveryAction<Truthiness> {
369+
salsa::CycleRecoveryAction::Iterate
370+
}
371+
372+
fn static_expression_truthiness_cycle_initial<'db>(
373+
_db: &'db dyn Db,
374+
_expression: Expression<'db>,
375+
) -> Truthiness {
376+
Truthiness::Ambiguous
377+
}
378+
340379
/// Infer the types for an [`Unpack`] operation.
341380
///
342381
/// This infers the expression type and performs structural match against the target expression
@@ -657,6 +696,9 @@ struct ExpressionInferenceExtra<'db> {
657696
///
658697
/// Falls back to `Type::Never` if an expression is missing.
659698
cycle_fallback: bool,
699+
700+
/// `true` if all places in this expression are definitely bound
701+
all_definitely_bound: bool,
660702
}
661703

662704
impl<'db> ExpressionInference<'db> {
@@ -665,6 +707,7 @@ impl<'db> ExpressionInference<'db> {
665707
Self {
666708
extra: Some(Box::new(ExpressionInferenceExtra {
667709
cycle_fallback: true,
710+
all_definitely_bound: true,
668711
..ExpressionInferenceExtra::default()
669712
})),
670713
expressions: FxHashMap::default(),
@@ -698,6 +741,14 @@ impl<'db> ExpressionInference<'db> {
698741
fn fallback_type(&self) -> Option<Type<'db>> {
699742
self.is_cycle_callback().then_some(Type::Never)
700743
}
744+
745+
/// Returns true if all places in this expression are definitely bound.
746+
pub(crate) fn all_places_definitely_bound(&self) -> bool {
747+
self.extra
748+
.as_ref()
749+
.map(|e| e.all_definitely_bound)
750+
.unwrap_or(true)
751+
}
701752
}
702753

703754
/// Whether the intersection type is on the left or right side of the comparison.
@@ -847,6 +898,9 @@ pub(super) struct TypeInferenceBuilder<'db, 'ast> {
847898
///
848899
/// This is used only when constructing a cycle-recovery `TypeInference`.
849900
cycle_fallback: bool,
901+
902+
/// `true` if all places in this expression are definitely bound
903+
all_definitely_bound: bool,
850904
}
851905

852906
impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
@@ -880,6 +934,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
880934
deferred: VecSet::default(),
881935
undecorated_type: None,
882936
cycle_fallback: false,
937+
all_definitely_bound: true,
883938
}
884939
}
885940

@@ -6614,7 +6669,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
66146669
let (resolved, constraint_keys) =
66156670
self.infer_place_load(PlaceExprRef::from(&expr), ast::ExprRef::Name(name_node));
66166671

6617-
resolved
6672+
let resolved_after_fallback = resolved
66186673
// Not found in the module's explicitly declared global symbols?
66196674
// Check the "implicit globals" such as `__doc__`, `__file__`, `__name__`, etc.
66206675
// These are looked up as attributes on `types.ModuleType`.
@@ -6650,8 +6705,14 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
66506705
} else {
66516706
Place::Unbound.into()
66526707
}
6653-
})
6654-
.unwrap_with_diagnostic(|lookup_error| match lookup_error {
6708+
});
6709+
6710+
if !resolved_after_fallback.place.is_definitely_bound() {
6711+
self.all_definitely_bound = false;
6712+
}
6713+
6714+
let ty =
6715+
resolved_after_fallback.unwrap_with_diagnostic(|lookup_error| match lookup_error {
66556716
LookupError::Unbound(qualifiers) => {
66566717
self.report_unresolved_reference(name_node);
66576718
TypeAndQualifiers::new(Type::unknown(), qualifiers)
@@ -6662,8 +6723,9 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
66626723
}
66636724
type_when_bound
66646725
}
6665-
})
6666-
.inner_type()
6726+
});
6727+
6728+
ty.inner_type()
66676729
}
66686730

66696731
fn infer_local_place_load(
@@ -7093,7 +7155,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
70937155
}
70947156

70957157
fn narrow_expr_with_applicable_constraints<'r>(
7096-
&self,
7158+
&mut self,
70977159
target: impl Into<ast::ExprRef<'r>>,
70987160
target_ty: Type<'db>,
70997161
constraint_keys: &[(FileScopeId, ConstraintKey)],
@@ -7136,11 +7198,19 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
71367198
assigned_type = Some(ty);
71377199
}
71387200
}
7201+
let fallback_place = value_type.member(db, &attr.id);
7202+
if !fallback_place.place.is_definitely_bound()
7203+
|| fallback_place
7204+
.qualifiers
7205+
.contains(TypeQualifiers::POSSIBLY_UNBOUND_IMPLICIT_ATTRIBUTE)
7206+
{
7207+
self.all_definitely_bound = false;
7208+
}
71397209

7140-
let resolved_type = value_type
7141-
.member(db, &attr.id)
7142-
.map_type(|ty| self.narrow_expr_with_applicable_constraints(attribute, ty, &constraint_keys))
7143-
.unwrap_with_diagnostic(|lookup_error| match lookup_error {
7210+
let resolved_type =
7211+
fallback_place.map_type(|ty| {
7212+
self.narrow_expr_with_applicable_constraints(attribute, ty, &constraint_keys)
7213+
}).unwrap_with_diagnostic(|lookup_error| match lookup_error {
71447214
LookupError::Unbound(_) => {
71457215
let report_unresolved_attribute = self.is_reachable(attribute);
71467216

@@ -9248,6 +9318,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
92489318
declarations,
92499319
deferred,
92509320
cycle_fallback,
9321+
all_definitely_bound,
92519322

92529323
// Ignored; only relevant to definition regions
92539324
undecorated_type: _,
@@ -9274,7 +9345,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
92749345
);
92759346

92769347
let extra =
9277-
(cycle_fallback || !bindings.is_empty() || !diagnostics.is_empty()).then(|| {
9348+
(cycle_fallback || !bindings.is_empty() || !diagnostics.is_empty() || !all_definitely_bound).then(|| {
92789349
if bindings.len() > 20 {
92799350
tracing::debug!(
92809351
"Inferred expression region `{:?}` contains {} bindings. Lookups by linear scan might be slow.",
@@ -9287,6 +9358,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
92879358
bindings: bindings.into_boxed_slice(),
92889359
diagnostics,
92899360
cycle_fallback,
9361+
all_definitely_bound,
92909362
})
92919363
});
92929364

@@ -9312,7 +9384,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
93129384
deferred,
93139385
cycle_fallback,
93149386
undecorated_type,
9315-
9387+
all_definitely_bound: _,
93169388
// builder only state
93179389
typevar_binding_context: _,
93189390
deferred_state: _,
@@ -9379,6 +9451,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
93799451
deferred: _,
93809452
bindings: _,
93819453
declarations: _,
9454+
all_definitely_bound: _,
93829455

93839456
// Ignored; only relevant to definition regions
93849457
undecorated_type: _,

0 commit comments

Comments
 (0)