diff --git a/Cargo.lock b/Cargo.lock index cdc790fb13..ba9030af5f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3327,8 +3327,8 @@ checksum = "6ea1a2d0a644769cc99faa24c3ad26b379b786fe7c36fd3c546254801650e6dd" [[package]] name = "salsa" -version = "0.18.0" -source = "git+https://github.com/salsa-rs/salsa.git?rev=99be5d9917c3dd88e19735a82ef6bf39ba84bd7e#99be5d9917c3dd88e19735a82ef6bf39ba84bd7e" +version = "0.19.0" +source = "git+https://github.com/salsa-rs/salsa.git?rev=095d8b2b8115c3cf8bf31914dd9ea74648bb7cf9#095d8b2b8115c3cf8bf31914dd9ea74648bb7cf9" dependencies = [ "boxcar", "compact_str", @@ -3338,6 +3338,7 @@ dependencies = [ "hashlink", "indexmap", "parking_lot", + "portable-atomic", "rayon", "rustc-hash 2.1.1", "salsa-macro-rules", @@ -3348,13 +3349,13 @@ dependencies = [ [[package]] name = "salsa-macro-rules" -version = "0.1.0" -source = "git+https://github.com/salsa-rs/salsa.git?rev=99be5d9917c3dd88e19735a82ef6bf39ba84bd7e#99be5d9917c3dd88e19735a82ef6bf39ba84bd7e" +version = "0.19.0" +source = "git+https://github.com/salsa-rs/salsa.git?rev=095d8b2b8115c3cf8bf31914dd9ea74648bb7cf9#095d8b2b8115c3cf8bf31914dd9ea74648bb7cf9" [[package]] name = "salsa-macros" -version = "0.18.0" -source = "git+https://github.com/salsa-rs/salsa.git?rev=99be5d9917c3dd88e19735a82ef6bf39ba84bd7e#99be5d9917c3dd88e19735a82ef6bf39ba84bd7e" +version = "0.19.0" +source = "git+https://github.com/salsa-rs/salsa.git?rev=095d8b2b8115c3cf8bf31914dd9ea74648bb7cf9#095d8b2b8115c3cf8bf31914dd9ea74648bb7cf9" dependencies = [ "heck", "proc-macro2", diff --git a/Cargo.toml b/Cargo.toml index af94be8bd3..f3782c5f14 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -123,7 +123,7 @@ rayon = { version = "1.10.0" } regex = { version = "1.10.2" } rustc-hash = { version = "2.0.0" } # When updating salsa, make sure to also update the revision in `fuzz/Cargo.toml` -salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "99be5d9917c3dd88e19735a82ef6bf39ba84bd7e" } +salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "095d8b2b8115c3cf8bf31914dd9ea74648bb7cf9" } schemars = { version = "0.8.16" } seahash = { version = "4.1.0" } serde = { version = "1.0.197", features = ["derive"] } diff --git a/crates/red_knot_project/tests/check.rs b/crates/red_knot_project/tests/check.rs index 2f3d9b5172..091908cf0c 100644 --- a/crates/red_knot_project/tests/check.rs +++ b/crates/red_knot_project/tests/check.rs @@ -279,23 +279,4 @@ impl SourceOrderVisitor<'_> for PullTypesVisitor<'_> { /// Whether or not the .py/.pyi version of this file is expected to fail #[rustfmt::skip] -const KNOWN_FAILURES: &[(&str, bool, bool)] = &[ - // related to circular references in nested functions - ("crates/ruff_linter/resources/test/fixtures/flake8_return/RET503.py", false, true), - // related to circular references in class definitions - ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_26.py", true, true), - ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_27.py", true, true), - ("crates/ruff_linter/resources/test/fixtures/pyflakes/F811_19.py", true, false), - ("crates/ruff_linter/resources/test/fixtures/pyupgrade/UP039.py", true, false), - // related to circular references in type aliases (salsa cycle panic): - ("crates/ruff_python_parser/resources/inline/err/type_alias_invalid_value_expr.py", true, true), - ("crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TC008.py", true, true), - // related to circular references in f-string annotations (invalid syntax) - ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_15.py", true, true), - ("crates/ruff_linter/resources/test/fixtures/pyflakes/F821_14.py", false, true), - // related to circular references in stub type annotations (salsa cycle panic): - ("crates/ruff_linter/resources/test/fixtures/pycodestyle/E501_4.py", false, true), - ("crates/ruff_linter/resources/test/fixtures/pyflakes/F401_0.py", false, true), - ("crates/ruff_linter/resources/test/fixtures/pyflakes/F401_12.py", false, true), - ("crates/ruff_linter/resources/test/fixtures/pyflakes/F401_14.py", false, true), -]; +const KNOWN_FAILURES: &[(&str, bool, bool)] = &[]; diff --git a/crates/red_knot_python_semantic/resources/mdtest/generics/classes.md b/crates/red_knot_python_semantic/resources/mdtest/generics/classes.md index 3ae0095c57..da9626323b 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/generics/classes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/generics/classes.md @@ -180,6 +180,7 @@ reveal_type(Sub) # revealed: Literal[Sub] class Base[T]: ... # TODO: error: [unresolved-reference] +# error: [non-subscriptable] class Sub(Base[Sub]): ... ``` diff --git a/crates/red_knot_python_semantic/src/symbol.rs b/crates/red_knot_python_semantic/src/symbol.rs index 7459e37f28..3ba9bc3692 100644 --- a/crates/red_knot_python_semantic/src/symbol.rs +++ b/crates/red_knot_python_semantic/src/symbol.rs @@ -488,7 +488,27 @@ impl<'db> From> for SymbolAndQualifiers<'db> { } } -#[salsa::tracked] +fn symbol_cycle_recover<'db>( + _db: &'db dyn Db, + _value: &SymbolAndQualifiers<'db>, + _count: u32, + _scope: ScopeId<'db>, + _symbol_id: ScopedSymbolId, + _requires_explicit_reexport: RequiresExplicitReExport, +) -> salsa::CycleRecoveryAction> { + salsa::CycleRecoveryAction::Iterate +} + +fn symbol_cycle_initial<'db>( + _db: &'db dyn Db, + _scope: ScopeId<'db>, + _symbol_id: ScopedSymbolId, + _requires_explicit_reexport: RequiresExplicitReExport, +) -> SymbolAndQualifiers<'db> { + Symbol::bound(Type::Never).into() +} + +#[salsa::tracked(cycle_fn=symbol_cycle_recover, cycle_initial=symbol_cycle_initial)] fn symbol_by_id<'db>( db: &'db dyn Db, scope: ScopeId<'db>, diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 8a64d0e053..5bcac004df 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -26,6 +26,13 @@ //! stringified annotations. We have a fourth Salsa query for inferring the deferred types //! associated with a particular definition. Scope-level inference infers deferred types for all //! definitions once the rest of the types in the scope have been inferred. +//! +//! Many of our type inference Salsa queries implement cycle recovery via fixed-point iteration. In +//! general, they initiate fixed-point iteration by returning a `TypeInference` that returns +//! `Type::Never` for all expressions, bindings, and declarations, and then they continue iterating +//! the query cycle until a fixed-point is reached. Salsa has a built-in fixed limit on the number +//! of iterations, so if we fail to converge, Salsa will eventually panic. (This should of course +//! be considered a bug.) use std::num::NonZeroU32; use itertools::{Either, Itertools}; @@ -100,7 +107,7 @@ use super::{CallDunderError, ParameterExpectation, ParameterExpectations}; /// 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. -#[salsa::tracked(return_ref)] +#[salsa::tracked(return_ref, cycle_fn=scope_cycle_recover, cycle_initial=scope_cycle_initial)] pub(crate) fn infer_scope_types<'db>(db: &'db dyn Db, scope: ScopeId<'db>) -> TypeInference<'db> { let file = scope.file(db); let _span = @@ -114,26 +121,22 @@ pub(crate) fn infer_scope_types<'db>(db: &'db dyn Db, scope: ScopeId<'db>) -> Ty TypeInferenceBuilder::new(db, InferenceRegion::Scope(scope), index).finish() } -/// TODO temporary cycle recovery for [`infer_definition_types()`], pending fixpoint iteration -fn infer_definition_types_cycle_recovery<'db>( - db: &'db dyn Db, - _cycle: &salsa::Cycle, - input: Definition<'db>, -) -> TypeInference<'db> { - let file = input.file(db); - let _span = tracing::trace_span!( - "infer_definition_types_cycle_recovery", - range = ?input.kind(db).target_range(), - file = %file.path(db) - ) - .entered(); +fn scope_cycle_recover<'db>( + _db: &'db dyn Db, + _value: &TypeInference<'db>, + _count: u32, + _scope: ScopeId<'db>, +) -> salsa::CycleRecoveryAction> { + salsa::CycleRecoveryAction::Iterate +} - TypeInference::cycle_fallback(input.scope(db), todo_type!("cycle recovery")) +fn scope_cycle_initial<'db>(_db: &'db dyn Db, scope: ScopeId<'db>) -> TypeInference<'db> { + TypeInference::cycle_fallback(scope, Type::Never) } /// Infer all types for a [`Definition`] (including sub-expressions). /// Use when resolving a symbol name use or public type of a symbol. -#[salsa::tracked(return_ref, recovery_fn=infer_definition_types_cycle_recovery)] +#[salsa::tracked(return_ref, cycle_fn=definition_cycle_recover, cycle_initial=definition_cycle_initial)] pub(crate) fn infer_definition_types<'db>( db: &'db dyn Db, definition: Definition<'db>, @@ -151,11 +154,27 @@ pub(crate) fn infer_definition_types<'db>( TypeInferenceBuilder::new(db, InferenceRegion::Definition(definition), index).finish() } +fn definition_cycle_recover<'db>( + _db: &'db dyn Db, + _value: &TypeInference<'db>, + _count: u32, + _definition: Definition<'db>, +) -> salsa::CycleRecoveryAction> { + salsa::CycleRecoveryAction::Iterate +} + +fn definition_cycle_initial<'db>( + db: &'db dyn Db, + definition: Definition<'db>, +) -> TypeInference<'db> { + TypeInference::cycle_fallback(definition.scope(db), Type::Never) +} + /// Infer types for all deferred type expressions in a [`Definition`]. /// /// Deferred expressions are type expressions (annotations, base classes, aliases...) in a stub /// file, or in a file with `from __future__ import annotations`, or stringified annotations. -#[salsa::tracked(return_ref)] +#[salsa::tracked(return_ref, cycle_fn=deferred_cycle_recover, cycle_initial=deferred_cycle_initial)] pub(crate) fn infer_deferred_types<'db>( db: &'db dyn Db, definition: Definition<'db>, @@ -174,11 +193,24 @@ pub(crate) fn infer_deferred_types<'db>( TypeInferenceBuilder::new(db, InferenceRegion::Deferred(definition), index).finish() } +fn deferred_cycle_recover<'db>( + _db: &'db dyn Db, + _value: &TypeInference<'db>, + _count: u32, + _definition: Definition<'db>, +) -> salsa::CycleRecoveryAction> { + salsa::CycleRecoveryAction::Iterate +} + +fn deferred_cycle_initial<'db>(db: &'db dyn Db, definition: Definition<'db>) -> TypeInference<'db> { + TypeInference::cycle_fallback(definition.scope(db), Type::Never) +} + /// Infer all types for an [`Expression`] (including sub-expressions). /// Use rarely; only for cases where we'd otherwise risk double-inferring an expression: RHS of an /// assignment, which might be unpacking/multi-target and thus part of multiple definitions, or a /// type narrowing guard expression (e.g. if statement test node). -#[salsa::tracked(return_ref)] +#[salsa::tracked(return_ref, cycle_fn=expression_cycle_recover, cycle_initial=expression_cycle_initial)] pub(crate) fn infer_expression_types<'db>( db: &'db dyn Db, expression: Expression<'db>, @@ -197,6 +229,22 @@ pub(crate) fn infer_expression_types<'db>( TypeInferenceBuilder::new(db, InferenceRegion::Expression(expression), index).finish() } +fn expression_cycle_recover<'db>( + _db: &'db dyn Db, + _value: &TypeInference<'db>, + _count: u32, + _expression: Expression<'db>, +) -> salsa::CycleRecoveryAction> { + salsa::CycleRecoveryAction::Iterate +} + +fn expression_cycle_initial<'db>( + db: &'db dyn Db, + expression: Expression<'db>, +) -> TypeInference<'db> { + TypeInference::cycle_fallback(expression.scope(db), Type::Never) +} + /// Infers the type of an `expression` that is guaranteed to be in the same file as the calling query. /// /// This is a small helper around [`infer_expression_types()`] to reduce the boilerplate. @@ -218,7 +266,7 @@ pub(super) fn infer_same_file_expression_type<'db>( /// /// Use [`infer_same_file_expression_type`] if it is guaranteed that `expression` is in the same /// to avoid unnecessary salsa ingredients. This is normally the case inside the `TypeInferenceBuilder`. -#[salsa::tracked] +#[salsa::tracked(cycle_fn=single_expression_cycle_recover, cycle_initial=single_expression_cycle_initial)] pub(crate) fn infer_expression_type<'db>( db: &'db dyn Db, expression: Expression<'db>, @@ -227,6 +275,22 @@ pub(crate) fn infer_expression_type<'db>( infer_same_file_expression_type(db, expression) } +fn single_expression_cycle_recover<'db>( + _db: &'db dyn Db, + _value: &Type<'db>, + _count: u32, + _expression: Expression<'db>, +) -> salsa::CycleRecoveryAction> { + salsa::CycleRecoveryAction::Iterate +} + +fn single_expression_cycle_initial<'db>( + _db: &'db dyn Db, + _expression: Expression<'db>, +) -> Type<'db> { + Type::Never +} + /// Infer the types for an [`Unpack`] operation. /// /// This infers the expression type and performs structural match against the target expression diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 3e27db0c5f..64e8dc0f89 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -29,7 +29,7 @@ ruff_python_formatter = { path = "../crates/ruff_python_formatter" } ruff_text_size = { path = "../crates/ruff_text_size" } libfuzzer-sys = { git = "https://github.com/rust-fuzz/libfuzzer", default-features = false } -salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "99be5d9917c3dd88e19735a82ef6bf39ba84bd7e" } +salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "095d8b2b8115c3cf8bf31914dd9ea74648bb7cf9" } similar = { version = "2.5.0" } tracing = { version = "0.1.40" }