mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-29 13:24:57 +00:00
[ty] no more diverging query cycles in type expressions (#20359)
## Summary Use `Type::Divergent` to short-circuit diverging types in type expressions. This avoids panicking in a wide variety of cases of recursive type expressions. Avoids many panics (but not yet all -- I'll be tracking down the rest) from https://github.com/astral-sh/ty/issues/256 by falling back to Divergent. For many of these recursive type aliases, we'd like to support them properly (i.e. really understand the recursive nature of the type, not just fall back to Divergent) but that will be future work. This switches `Type::has_divergent_type` from using `any_over_type` to a custom set of visit methods, because `any_over_type` visits more than we need to visit, and exercises some lazy attributes of type, causing significantly more work. This change means this diff doesn't regress perf; it even reclaims some of the perf regression from https://github.com/astral-sh/ruff/pull/20333. ## Test Plan Added mdtest for recursive type alias that panics on main. Verified that we can now type-check `packaging` (and projects depending on it) without panic; this will allow moving a number of mypy-primer projects from `bad.txt` to `good.txt` in a subsequent PR.
This commit is contained in:
parent
c3f2187fda
commit
d121a76aef
9 changed files with 173 additions and 45 deletions
|
@ -6496,10 +6496,7 @@ impl<'db> Type<'db> {
|
|||
}
|
||||
|
||||
pub(super) fn has_divergent_type(self, db: &'db dyn Db, div: Type<'db>) -> bool {
|
||||
any_over_type(db, self, &|ty| match ty {
|
||||
Type::Dynamic(DynamicType::Divergent(_)) => ty == div,
|
||||
_ => false,
|
||||
})
|
||||
any_over_type(db, self, &|ty| ty == div, false)
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -7445,10 +7442,27 @@ fn walk_type_var_type<'db, V: visitor::TypeVisitor<'db> + ?Sized>(
|
|||
typevar: TypeVarInstance<'db>,
|
||||
visitor: &V,
|
||||
) {
|
||||
if let Some(bounds) = typevar.bound_or_constraints(db) {
|
||||
walk_type_var_bounds(db, bounds, visitor);
|
||||
if let Some(bound_or_constraints) = if visitor.should_visit_lazy_type_attributes() {
|
||||
typevar.bound_or_constraints(db)
|
||||
} else {
|
||||
match typevar._bound_or_constraints(db) {
|
||||
_ if visitor.should_visit_lazy_type_attributes() => typevar.bound_or_constraints(db),
|
||||
Some(TypeVarBoundOrConstraintsEvaluation::Eager(bound_or_constraints)) => {
|
||||
Some(bound_or_constraints)
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
} {
|
||||
walk_type_var_bounds(db, bound_or_constraints, visitor);
|
||||
}
|
||||
if let Some(default_type) = typevar.default_type(db) {
|
||||
if let Some(default_type) = if visitor.should_visit_lazy_type_attributes() {
|
||||
typevar.default_type(db)
|
||||
} else {
|
||||
match typevar._default(db) {
|
||||
Some(TypeVarDefaultEvaluation::Eager(default_type)) => Some(default_type),
|
||||
_ => None,
|
||||
}
|
||||
} {
|
||||
visitor.visit_type(db, default_type);
|
||||
}
|
||||
}
|
||||
|
@ -9877,6 +9891,9 @@ fn walk_type_alias_type<'db, V: visitor::TypeVisitor<'db> + ?Sized>(
|
|||
type_alias: TypeAliasType<'db>,
|
||||
visitor: &V,
|
||||
) {
|
||||
if !visitor.should_visit_lazy_type_attributes() {
|
||||
return;
|
||||
}
|
||||
match type_alias {
|
||||
TypeAliasType::PEP695(type_alias) => {
|
||||
walk_pep_695_type_alias(db, type_alias, visitor);
|
||||
|
|
|
@ -1499,8 +1499,8 @@ impl KnownFunction {
|
|||
let contains_unknown_or_todo =
|
||||
|ty| matches!(ty, Type::Dynamic(dynamic) if dynamic != DynamicType::Any);
|
||||
if source_type.is_equivalent_to(db, *casted_type)
|
||||
&& !any_over_type(db, *source_type, &contains_unknown_or_todo)
|
||||
&& !any_over_type(db, *casted_type, &contains_unknown_or_todo)
|
||||
&& !any_over_type(db, *source_type, &contains_unknown_or_todo, true)
|
||||
&& !any_over_type(db, *casted_type, &contains_unknown_or_todo, true)
|
||||
{
|
||||
if let Some(builder) = context.report_lint(&REDUNDANT_CAST, call_expression) {
|
||||
builder.into_diagnostic(format_args!(
|
||||
|
|
|
@ -58,6 +58,9 @@ mod builder;
|
|||
#[cfg(test)]
|
||||
mod tests;
|
||||
|
||||
/// How many fixpoint iterations to allow before falling back to Divergent type.
|
||||
const ITERATIONS_BEFORE_FALLBACK: u32 = 10;
|
||||
|
||||
/// Infer all types for a [`ScopeId`], including all definitions and expressions in that scope.
|
||||
/// Use when checking a scope, or needing to provide a type for an arbitrary expression in the
|
||||
/// scope.
|
||||
|
@ -111,12 +114,18 @@ pub(crate) fn infer_definition_types<'db>(
|
|||
}
|
||||
|
||||
fn definition_cycle_recover<'db>(
|
||||
_db: &'db dyn Db,
|
||||
db: &'db dyn Db,
|
||||
_value: &DefinitionInference<'db>,
|
||||
_count: u32,
|
||||
_definition: Definition<'db>,
|
||||
count: u32,
|
||||
definition: Definition<'db>,
|
||||
) -> salsa::CycleRecoveryAction<DefinitionInference<'db>> {
|
||||
salsa::CycleRecoveryAction::Iterate
|
||||
if count == ITERATIONS_BEFORE_FALLBACK {
|
||||
salsa::CycleRecoveryAction::Fallback(DefinitionInference::cycle_fallback(
|
||||
definition.scope(db),
|
||||
))
|
||||
} else {
|
||||
salsa::CycleRecoveryAction::Iterate
|
||||
}
|
||||
}
|
||||
|
||||
fn definition_cycle_initial<'db>(
|
||||
|
@ -207,9 +216,6 @@ fn infer_expression_types_impl<'db>(
|
|||
.finish_expression()
|
||||
}
|
||||
|
||||
/// How many fixpoint iterations to allow before falling back to Divergent type.
|
||||
const ITERATIONS_BEFORE_FALLBACK: u32 = 10;
|
||||
|
||||
fn expression_cycle_recover<'db>(
|
||||
db: &'db dyn Db,
|
||||
_value: &ExpressionInference<'db>,
|
||||
|
@ -623,6 +629,22 @@ impl<'db> DefinitionInference<'db> {
|
|||
}
|
||||
}
|
||||
|
||||
fn cycle_fallback(scope: ScopeId<'db>) -> Self {
|
||||
let _ = scope;
|
||||
|
||||
Self {
|
||||
expressions: FxHashMap::default(),
|
||||
bindings: Box::default(),
|
||||
declarations: Box::default(),
|
||||
#[cfg(debug_assertions)]
|
||||
scope,
|
||||
extra: Some(Box::new(DefinitionInferenceExtra {
|
||||
cycle_recovery: Some(CycleRecovery::Divergent(scope)),
|
||||
..DefinitionInferenceExtra::default()
|
||||
})),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn expression_type(&self, expression: impl Into<ExpressionNodeKey>) -> Type<'db> {
|
||||
self.try_expression_type(expression)
|
||||
.unwrap_or_else(Type::unknown)
|
||||
|
|
|
@ -8785,14 +8785,19 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
|||
*typevar,
|
||||
)
|
||||
.ok_or(GenericContextError::InvalidArgument)
|
||||
} else if any_over_type(self.db(), *typevar, &|ty| match ty {
|
||||
Type::Dynamic(DynamicType::TodoUnpack) => true,
|
||||
Type::NominalInstance(nominal) => matches!(
|
||||
nominal.known_class(self.db()),
|
||||
Some(KnownClass::TypeVarTuple | KnownClass::ParamSpec)
|
||||
),
|
||||
_ => false,
|
||||
}) {
|
||||
} else if any_over_type(
|
||||
self.db(),
|
||||
*typevar,
|
||||
&|ty| match ty {
|
||||
Type::Dynamic(DynamicType::TodoUnpack) => true,
|
||||
Type::NominalInstance(nominal) => matches!(
|
||||
nominal.known_class(self.db()),
|
||||
Some(KnownClass::TypeVarTuple | KnownClass::ParamSpec)
|
||||
),
|
||||
_ => false,
|
||||
},
|
||||
true,
|
||||
) {
|
||||
Err(GenericContextError::NotYetSupported)
|
||||
} else {
|
||||
if let Some(builder) =
|
||||
|
|
|
@ -21,7 +21,11 @@ use crate::types::{
|
|||
impl<'db> TypeInferenceBuilder<'db, '_> {
|
||||
/// Infer the type of a type expression.
|
||||
pub(super) fn infer_type_expression(&mut self, expression: &ast::Expr) -> Type<'db> {
|
||||
let ty = self.infer_type_expression_no_store(expression);
|
||||
let mut ty = self.infer_type_expression_no_store(expression);
|
||||
let divergent = Type::divergent(self.scope());
|
||||
if ty.has_divergent_type(self.db(), divergent) {
|
||||
ty = divergent;
|
||||
}
|
||||
self.store_expression_type(expression, ty);
|
||||
ty
|
||||
}
|
||||
|
@ -713,7 +717,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
|
|||
// `infer_expression` (instead of `infer_type_expression`) here to avoid
|
||||
// false-positive `invalid-type-form` diagnostics (`1` is not a valid type
|
||||
// expression).
|
||||
self.infer_expression(&subscript.slice, TypeContext::default());
|
||||
self.infer_expression(slice, TypeContext::default());
|
||||
Type::unknown()
|
||||
}
|
||||
Type::SpecialForm(special_form) => {
|
||||
|
@ -721,7 +725,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
|
|||
}
|
||||
Type::KnownInstance(known_instance) => match known_instance {
|
||||
KnownInstanceType::SubscriptedProtocol(_) => {
|
||||
self.infer_type_expression(&subscript.slice);
|
||||
self.infer_type_expression(slice);
|
||||
if let Some(builder) = self.context.report_lint(&INVALID_TYPE_FORM, subscript) {
|
||||
builder.into_diagnostic(format_args!(
|
||||
"`typing.Protocol` is not allowed in type expressions",
|
||||
|
@ -730,7 +734,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
|
|||
Type::unknown()
|
||||
}
|
||||
KnownInstanceType::SubscriptedGeneric(_) => {
|
||||
self.infer_type_expression(&subscript.slice);
|
||||
self.infer_type_expression(slice);
|
||||
if let Some(builder) = self.context.report_lint(&INVALID_TYPE_FORM, subscript) {
|
||||
builder.into_diagnostic(format_args!(
|
||||
"`typing.Generic` is not allowed in type expressions",
|
||||
|
@ -739,7 +743,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
|
|||
Type::unknown()
|
||||
}
|
||||
KnownInstanceType::Deprecated(_) => {
|
||||
self.infer_type_expression(&subscript.slice);
|
||||
self.infer_type_expression(slice);
|
||||
if let Some(builder) = self.context.report_lint(&INVALID_TYPE_FORM, subscript) {
|
||||
builder.into_diagnostic(format_args!(
|
||||
"`warnings.deprecated` is not allowed in type expressions",
|
||||
|
@ -748,7 +752,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
|
|||
Type::unknown()
|
||||
}
|
||||
KnownInstanceType::Field(_) => {
|
||||
self.infer_type_expression(&subscript.slice);
|
||||
self.infer_type_expression(slice);
|
||||
if let Some(builder) = self.context.report_lint(&INVALID_TYPE_FORM, subscript) {
|
||||
builder.into_diagnostic(format_args!(
|
||||
"`dataclasses.Field` is not allowed in type expressions",
|
||||
|
@ -757,7 +761,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
|
|||
Type::unknown()
|
||||
}
|
||||
KnownInstanceType::ConstraintSet(_) => {
|
||||
self.infer_type_expression(&subscript.slice);
|
||||
self.infer_type_expression(slice);
|
||||
if let Some(builder) = self.context.report_lint(&INVALID_TYPE_FORM, subscript) {
|
||||
builder.into_diagnostic(format_args!(
|
||||
"`ty_extensions.ConstraintSet` is not allowed in type expressions",
|
||||
|
@ -766,7 +770,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
|
|||
Type::unknown()
|
||||
}
|
||||
KnownInstanceType::TypeVar(_) => {
|
||||
self.infer_type_expression(&subscript.slice);
|
||||
self.infer_type_expression(slice);
|
||||
todo_type!("TypeVar annotations")
|
||||
}
|
||||
KnownInstanceType::TypeAliasType(type_alias @ TypeAliasType::PEP695(_)) => {
|
||||
|
@ -831,8 +835,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
|
|||
.unwrap_or(Type::unknown())
|
||||
}
|
||||
None => {
|
||||
// TODO: Once we know that e.g. `list` is generic, emit a diagnostic if you try to
|
||||
// specialize a non-generic class.
|
||||
// TODO: emit a diagnostic if you try to specialize a non-generic class.
|
||||
self.infer_type_expression(slice);
|
||||
todo_type!("specialized non-generic class")
|
||||
}
|
||||
|
@ -1519,13 +1522,18 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
|
|||
// `Callable[]`.
|
||||
return None;
|
||||
}
|
||||
if any_over_type(self.db(), self.infer_name_load(name), &|ty| match ty {
|
||||
Type::Dynamic(DynamicType::TodoPEP695ParamSpec) => true,
|
||||
Type::NominalInstance(nominal) => {
|
||||
nominal.has_known_class(self.db(), KnownClass::ParamSpec)
|
||||
}
|
||||
_ => false,
|
||||
}) {
|
||||
if any_over_type(
|
||||
self.db(),
|
||||
self.infer_name_load(name),
|
||||
&|ty| match ty {
|
||||
Type::Dynamic(DynamicType::TodoPEP695ParamSpec) => true,
|
||||
Type::NominalInstance(nominal) => {
|
||||
nominal.has_known_class(self.db(), KnownClass::ParamSpec)
|
||||
}
|
||||
_ => false,
|
||||
},
|
||||
true,
|
||||
) {
|
||||
return Some(Parameters::todo());
|
||||
}
|
||||
}
|
||||
|
|
|
@ -9,6 +9,7 @@ use crate::place::PlaceAndQualifiers;
|
|||
use crate::semantic_index::definition::Definition;
|
||||
use crate::types::constraints::{ConstraintSet, IteratorConstraintsExtension};
|
||||
use crate::types::enums::is_single_member_enum;
|
||||
use crate::types::generics::walk_specialization;
|
||||
use crate::types::protocol_class::walk_protocol_interface;
|
||||
use crate::types::tuple::{TupleSpec, TupleType};
|
||||
use crate::types::{
|
||||
|
@ -528,7 +529,20 @@ pub(super) fn walk_protocol_instance_type<'db, V: super::visitor::TypeVisitor<'d
|
|||
protocol: ProtocolInstanceType<'db>,
|
||||
visitor: &V,
|
||||
) {
|
||||
walk_protocol_interface(db, protocol.inner.interface(db), visitor);
|
||||
if visitor.should_visit_lazy_type_attributes() {
|
||||
walk_protocol_interface(db, protocol.inner.interface(db), visitor);
|
||||
} else {
|
||||
match protocol.inner {
|
||||
Protocol::FromClass(class) => {
|
||||
if let Some(specialization) = class.class_literal(db).1 {
|
||||
walk_specialization(db, specialization, visitor);
|
||||
}
|
||||
}
|
||||
Protocol::Synthesized(synthesized) => {
|
||||
walk_protocol_interface(db, synthesized.interface(), visitor);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl<'db> ProtocolInstanceType<'db> {
|
||||
|
|
|
@ -573,9 +573,12 @@ impl<'a, 'db> ProtocolMember<'a, 'db> {
|
|||
|
||||
let proto_member_as_bound_method = method.bind_self(db);
|
||||
|
||||
if any_over_type(db, proto_member_as_bound_method, &|t| {
|
||||
matches!(t, Type::TypeVar(_))
|
||||
}) {
|
||||
if any_over_type(
|
||||
db,
|
||||
proto_member_as_bound_method,
|
||||
&|t| matches!(t, Type::TypeVar(_)),
|
||||
true,
|
||||
) {
|
||||
// TODO: proper validation for generic methods on protocols
|
||||
return ConstraintSet::from(true);
|
||||
}
|
||||
|
|
|
@ -23,6 +23,9 @@ use std::cell::{Cell, RefCell};
|
|||
/// but it makes it easy for implementors of the trait to do so.
|
||||
/// See [`any_over_type`] for an example of how to do this.
|
||||
pub(crate) trait TypeVisitor<'db> {
|
||||
/// Should the visitor trigger inference of and visit lazily-inferred type attributes?
|
||||
fn should_visit_lazy_type_attributes(&self) -> bool;
|
||||
|
||||
fn visit_type(&self, db: &'db dyn Db, ty: Type<'db>);
|
||||
|
||||
fn visit_union_type(&self, db: &'db dyn Db, union: UnionType<'db>) {
|
||||
|
@ -244,18 +247,28 @@ fn walk_non_atomic_type<'db, V: TypeVisitor<'db> + ?Sized>(
|
|||
///
|
||||
/// The function guards against infinite recursion
|
||||
/// by keeping track of the non-atomic types it has already seen.
|
||||
///
|
||||
/// The `should_visit_lazy_type_attributes` parameter controls whether deferred type attributes
|
||||
/// (value of a type alias, attributes of a class-based protocol, bounds/constraints of a typevar)
|
||||
/// are visited or not.
|
||||
pub(super) fn any_over_type<'db>(
|
||||
db: &'db dyn Db,
|
||||
ty: Type<'db>,
|
||||
query: &dyn Fn(Type<'db>) -> bool,
|
||||
should_visit_lazy_type_attributes: bool,
|
||||
) -> bool {
|
||||
struct AnyOverTypeVisitor<'db, 'a> {
|
||||
query: &'a dyn Fn(Type<'db>) -> bool,
|
||||
seen_types: RefCell<FxIndexSet<NonAtomicType<'db>>>,
|
||||
found_matching_type: Cell<bool>,
|
||||
should_visit_lazy_type_attributes: bool,
|
||||
}
|
||||
|
||||
impl<'db> TypeVisitor<'db> for AnyOverTypeVisitor<'db, '_> {
|
||||
fn should_visit_lazy_type_attributes(&self) -> bool {
|
||||
self.should_visit_lazy_type_attributes
|
||||
}
|
||||
|
||||
fn visit_type(&self, db: &'db dyn Db, ty: Type<'db>) {
|
||||
let already_found = self.found_matching_type.get();
|
||||
if already_found {
|
||||
|
@ -283,6 +296,7 @@ pub(super) fn any_over_type<'db>(
|
|||
query,
|
||||
seen_types: RefCell::new(FxIndexSet::default()),
|
||||
found_matching_type: Cell::new(false),
|
||||
should_visit_lazy_type_attributes,
|
||||
};
|
||||
visitor.visit_type(db, ty);
|
||||
visitor.found_matching_type.get()
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue