Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transition to salsa coarse-grained tracked structs #15763

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,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 = "88a1d7774d78f048fbd77d40abca9ebd729fd1f0" }
salsa = { git = "https://github.com/ibraheemdev/salsa.git", rev = "5afc02d69487976d700ad74e954f51606e8376ff" }
schemars = { version = "0.8.16" }
seahash = { version = "4.1.0" }
serde = { version = "1.0.197", features = ["derive"] }
Expand Down
20 changes: 7 additions & 13 deletions crates/red_knot_python_semantic/src/ast_node_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use ruff_db::parsed::ParsedModule;
/// Holding on to any [`AstNodeRef`] prevents the [`ParsedModule`] from being released.
///
/// ## Equality
/// Two `AstNodeRef` are considered equal if their wrapped nodes are equal.
/// Two `AstNodeRef` are considered equal if their pointer addresses are equal.
#[derive(Clone)]
pub struct AstNodeRef<T> {
/// Owned reference to the node's [`ParsedModule`].
Expand Down Expand Up @@ -67,23 +67,17 @@ where
}
}

impl<T> PartialEq for AstNodeRef<T>
where
T: PartialEq,
{
impl<T> PartialEq for AstNodeRef<T> {
fn eq(&self, other: &Self) -> bool {
self.node().eq(other.node())
self.node.eq(&other.node)
}
}

impl<T> Eq for AstNodeRef<T> where T: Eq {}
impl<T> Eq for AstNodeRef<T> {}

impl<T> Hash for AstNodeRef<T>
where
T: Hash,
{
impl<T> Hash for AstNodeRef<T> {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.node().hash(state);
self.node.hash(state);
}
}

Expand Down Expand Up @@ -117,7 +111,7 @@ mod tests {
let stmt_cloned = &cloned.syntax().body[0];
let cloned_node = unsafe { AstNodeRef::new(cloned.clone(), stmt_cloned) };

assert_eq!(node1, cloned_node);
assert_ne!(node1, cloned_node);

let other_raw = parse_unchecked_source("2 + 2", PySourceType::Python);
let other = ParsedModule::new(other_raw);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use super::path::{ModulePath, SearchPath, SearchPathValidationError};

/// Resolves a module name to a module.
pub fn resolve_module(db: &dyn Db, module_name: &ModuleName) -> Option<Module> {
let interned_name = ModuleNameIngredient::new(db, module_name);
let interned_name = ModuleNameIngredient::new(db, module_name.clone());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was caused by the new bounds introduced in salsa-rs/salsa@f428a94. I'm not sure why there's no blanket implementation for &T anymore.


resolve_module_query(db, interned_name)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub(crate) enum ConstraintNode<'db> {
}

/// Pattern kinds for which we support type narrowing and/or static visibility analysis.
#[derive(Debug, Clone, PartialEq)]
#[derive(Debug, Clone, PartialEq, Hash)]
pub(crate) enum PatternConstraintKind<'db> {
Singleton(Singleton, Option<Expression<'db>>),
Value(Expression<'db>, Option<Expression<'db>>),
Expand All @@ -28,10 +28,8 @@ pub(crate) enum PatternConstraintKind<'db> {

#[salsa::tracked]
pub(crate) struct PatternConstraint<'db> {
#[id]
pub(crate) file: File,

#[id]
pub(crate) file_scope: FileScopeId,

#[no_eq]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,12 @@ use crate::Db;
#[salsa::tracked]
pub struct Definition<'db> {
/// The file in which the definition occurs.
#[id]
pub(crate) file: File,

/// The scope in which the definition occurs.
#[id]
pub(crate) file_scope: FileScopeId,

/// The symbol defined.
#[id]
pub(crate) symbol: ScopedSymbolId,

#[no_eq]
Expand Down Expand Up @@ -435,7 +432,7 @@ impl DefinitionCategory {
}
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug, Hash)]
pub enum DefinitionKind<'db> {
Import(AstNodeRef<ast::Alias>),
ImportFrom(ImportFromDefinitionKind),
Expand Down Expand Up @@ -540,7 +537,7 @@ impl DefinitionKind<'_> {
}
}

#[derive(Copy, Clone, Debug, PartialEq)]
#[derive(Copy, Clone, Debug, PartialEq, Hash)]
pub(crate) enum TargetKind<'db> {
Sequence(Unpack<'db>),
Name,
Expand All @@ -555,7 +552,7 @@ impl<'db> From<Option<Unpack<'db>>> for TargetKind<'db> {
}
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug, Hash)]
#[allow(dead_code)]
pub struct MatchPatternDefinitionKind {
pattern: AstNodeRef<ast::Pattern>,
Expand All @@ -573,7 +570,7 @@ impl MatchPatternDefinitionKind {
}
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug, Hash)]
pub struct ComprehensionDefinitionKind {
iterable: AstNodeRef<ast::Expr>,
target: AstNodeRef<ast::ExprName>,
Expand All @@ -599,7 +596,7 @@ impl ComprehensionDefinitionKind {
}
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug, Hash)]
pub struct ImportFromDefinitionKind {
node: AstNodeRef<ast::StmtImportFrom>,
alias_index: usize,
Expand All @@ -615,7 +612,7 @@ impl ImportFromDefinitionKind {
}
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug, Hash)]
pub struct AssignmentDefinitionKind<'db> {
target: TargetKind<'db>,
value: AstNodeRef<ast::Expr>,
Expand All @@ -641,7 +638,7 @@ impl<'db> AssignmentDefinitionKind<'db> {
}
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug, Hash)]
pub struct WithItemDefinitionKind {
node: AstNodeRef<ast::WithItem>,
target: AstNodeRef<ast::ExprName>,
Expand All @@ -662,7 +659,7 @@ impl WithItemDefinitionKind {
}
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug, Hash)]
pub struct ForStmtDefinitionKind<'db> {
target: TargetKind<'db>,
iterable: AstNodeRef<ast::Expr>,
Expand Down Expand Up @@ -693,7 +690,7 @@ impl<'db> ForStmtDefinitionKind<'db> {
}
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug, Hash)]
pub struct ExceptHandlerDefinitionKind {
handler: AstNodeRef<ast::ExceptHandlerExceptHandler>,
is_star: bool,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,9 @@ use salsa;
#[salsa::tracked]
pub(crate) struct Expression<'db> {
/// The file in which the expression occurs.
#[id]
pub(crate) file: File,

/// The scope in which the expression occurs.
#[id]
pub(crate) file_scope: FileScopeId,

/// The expression node.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,8 @@ pub struct ScopedSymbolId;
/// A cross-module identifier of a scope that can be used as a salsa query parameter.
#[salsa::tracked]
pub struct ScopeId<'db> {
#[id]
pub file: File,

#[id]
pub file_scope_id: FileScopeId,

#[no_eq]
Expand Down
6 changes: 3 additions & 3 deletions crates/red_knot_python_semantic/src/types/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1134,7 +1134,7 @@ impl<'db> TypeInferenceBuilder<'db> {

let function_ty = Type::FunctionLiteral(FunctionType::new(
self.db(),
&name.id,
name.id.clone(),
function_kind,
body_scope,
decorator_tys.into_boxed_slice(),
Expand Down Expand Up @@ -1362,7 +1362,7 @@ impl<'db> TypeInferenceBuilder<'db> {

let maybe_known_class = KnownClass::try_from_file_and_name(self.db(), self.file(), name);

let class = Class::new(self.db(), &name.id, body_scope, maybe_known_class);
let class = Class::new(self.db(), name.id.clone(), body_scope, maybe_known_class);
let class_ty = Type::class_literal(class);

self.add_declaration_with_binding(
Expand Down Expand Up @@ -1419,7 +1419,7 @@ impl<'db> TypeInferenceBuilder<'db> {
let type_alias_ty =
Type::KnownInstance(KnownInstanceType::TypeAliasType(TypeAliasType::new(
self.db(),
&type_alias.name.as_name_expr().unwrap().id,
type_alias.name.as_name_expr().unwrap().id.clone(),
rhs_scope,
)));

Expand Down
4 changes: 1 addition & 3 deletions crates/red_knot_python_semantic/src/unpack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,8 @@ use crate::Db;
/// * an argument of a cross-module query
#[salsa::tracked]
pub(crate) struct Unpack<'db> {
#[id]
pub(crate) file: File,

#[id]
pub(crate) file_scope: FileScopeId,

/// The target expression that is being unpacked. For example, in `(a, b) = (1, 2)`, the target
Expand Down Expand Up @@ -62,7 +60,7 @@ impl<'db> Unpack<'db> {
}

/// The expression that is being unpacked.
#[derive(Clone, Copy, Debug)]
#[derive(Clone, Copy, Debug, Hash)]
pub(crate) enum UnpackValue<'db> {
/// An iterable expression like the one in a `for` loop or a comprehension.
Iterable(Expression<'db>),
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_python_ast/src/nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3426,7 +3426,7 @@ impl From<Identifier> for Name {
}
}

#[derive(Clone, Copy, Debug, PartialEq)]
#[derive(Clone, Copy, Debug, PartialEq, Hash)]
pub enum Singleton {
None,
True,
Expand Down
2 changes: 1 addition & 1 deletion fuzz/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "88a1d7774d78f048fbd77d40abca9ebd729fd1f0" }
salsa = { git = "https://github.com/ibraheemdev/salsa.git", rev = "5afc02d69487976d700ad74e954f51606e8376ff" }
similar = { version = "2.5.0" }
tracing = { version = "0.1.40" }

Expand Down
Loading