Skip to content

Commit

Permalink
Use salsa accumulators for diagnostics
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Dec 3, 2024
1 parent 81bfcc9 commit 1f6247c
Show file tree
Hide file tree
Showing 26 changed files with 662 additions and 586 deletions.
8 changes: 4 additions & 4 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 @@ -118,7 +118,7 @@ rand = { version = "0.8.5" }
rayon = { version = "1.10.0" }
regex = { version = "1.10.2" }
rustc-hash = { version = "2.0.0" }
salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "254c749b02cde2fd29852a7463a33e800b771758" }
salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "e68679b3a9c2b5cfd8eab92de89edf4073b03601" }
schemars = { version = "0.8.16" }
seahash = { version = "4.1.0" }
serde = { version = "1.0.197", features = ["derive"] }
Expand Down
6 changes: 3 additions & 3 deletions crates/red_knot/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use red_knot_workspace::watch;
use red_knot_workspace::watch::WorkspaceWatcher;
use red_knot_workspace::workspace::settings::Configuration;
use red_knot_workspace::workspace::WorkspaceMetadata;
use ruff_db::diagnostic::Diagnostic;
use ruff_db::diagnostic::CompileDiagnostic;
use ruff_db::system::{OsSystem, System, SystemPath, SystemPathBuf};
use salsa::plumbing::ZalsaDatabase;
use target_version::TargetVersion;
Expand Down Expand Up @@ -297,7 +297,7 @@ impl MainLoop {
while let Ok(message) = self.receiver.recv() {
match message {
MainLoopMessage::CheckWorkspace => {
let db = db.snapshot();
let db = db.clone();
let sender = self.sender.clone();

// Spawn a new task that checks the workspace. This needs to be done in a separate thread
Expand Down Expand Up @@ -380,7 +380,7 @@ impl MainLoopCancellationToken {
enum MainLoopMessage {
CheckWorkspace,
CheckCompleted {
result: Vec<Box<dyn Diagnostic>>,
result: Vec<CompileDiagnostic>,
revision: u64,
},
ApplyChanges(Vec<watch::ChangeEvent>),
Expand Down
1 change: 1 addition & 0 deletions crates/red_knot_python_semantic/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub(crate) mod tests {
use super::Db;

#[salsa::db]
#[derive(Clone)]
pub(crate) struct TestDb {
storage: salsa::Storage<Self>,
files: Files,
Expand Down
27 changes: 5 additions & 22 deletions crates/red_knot_python_semantic/src/semantic_index/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,10 +314,6 @@ impl<'db> SemanticIndexBuilder<'db> {
self.current_assignments.last().copied()
}

fn current_assignment_mut(&mut self) -> Option<&mut CurrentAssignment<'db>> {
self.current_assignments.last_mut()
}

fn add_pattern_constraint(
&mut self,
subject: &ast::Expr,
Expand Down Expand Up @@ -686,7 +682,7 @@ where
ast::Expr::List(_) | ast::Expr::Tuple(_) => {
Some(CurrentAssignment::Assign {
node,
first: true,

unpack: Some(Unpack::new(
self.db,
self.file,
Expand All @@ -700,11 +696,9 @@ where
)),
})
}
ast::Expr::Name(_) => Some(CurrentAssignment::Assign {
node,
unpack: None,
first: false,
}),
ast::Expr::Name(_) => {
Some(CurrentAssignment::Assign { node, unpack: None })
}
_ => None,
};

Expand Down Expand Up @@ -1082,18 +1076,13 @@ where

if is_definition {
match self.current_assignment() {
Some(CurrentAssignment::Assign {
node,
first,
unpack,
}) => {
Some(CurrentAssignment::Assign { node, unpack }) => {
self.add_definition(
symbol,
AssignmentDefinitionNodeRef {
unpack,
value: &node.value,
name: name_node,
first,
},
);
}
Expand Down Expand Up @@ -1144,11 +1133,6 @@ where
}
}

if let Some(CurrentAssignment::Assign { first, .. }) = self.current_assignment_mut()
{
*first = false;
}

walk_expr(self, expr);
}
ast::Expr::Named(node) => {
Expand Down Expand Up @@ -1355,7 +1339,6 @@ where
enum CurrentAssignment<'a> {
Assign {
node: &'a ast::StmtAssign,
first: bool,
unpack: Option<Unpack<'a>>,
},
AnnAssign(&'a ast::StmtAnnAssign),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,6 @@ pub(crate) struct AssignmentDefinitionNodeRef<'a> {
pub(crate) unpack: Option<Unpack<'a>>,
pub(crate) value: &'a ast::Expr,
pub(crate) name: &'a ast::ExprName,
pub(crate) first: bool,
}

#[derive(Copy, Clone, Debug)]
Expand Down Expand Up @@ -282,12 +281,10 @@ impl<'db> DefinitionNodeRef<'db> {
unpack,
value,
name,
first,
}) => DefinitionKind::Assignment(AssignmentDefinitionKind {
target: TargetKind::from(unpack),
value: AstNodeRef::new(parsed.clone(), value),
name: AstNodeRef::new(parsed, name),
first,
}),
DefinitionNodeRef::AnnotatedAssignment(assign) => {
DefinitionKind::AnnotatedAssignment(AstNodeRef::new(parsed, assign))
Expand Down Expand Up @@ -374,7 +371,6 @@ impl<'db> DefinitionNodeRef<'db> {
value: _,
unpack: _,
name,
first: _,
}) => name.into(),
Self::AnnotatedAssignment(node) => node.into(),
Self::AugmentedAssignment(node) => node.into(),
Expand Down Expand Up @@ -591,7 +587,6 @@ pub struct AssignmentDefinitionKind<'db> {
target: TargetKind<'db>,
value: AstNodeRef<ast::Expr>,
name: AstNodeRef<ast::ExprName>,
first: bool,
}

impl<'db> AssignmentDefinitionKind<'db> {
Expand All @@ -606,10 +601,6 @@ impl<'db> AssignmentDefinitionKind<'db> {
pub(crate) fn name(&self) -> &ast::ExprName {
self.name.node()
}

pub(crate) fn is_first(&self) -> bool {
self.first
}
}

#[derive(Clone, Debug)]
Expand Down
56 changes: 32 additions & 24 deletions crates/red_knot_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use ruff_db::files::File;
use ruff_python_ast as ast;

pub(crate) use self::builder::{IntersectionBuilder, UnionBuilder};
pub use self::diagnostic::{TypeCheckDiagnostic, TypeCheckDiagnostics};
pub(crate) use self::display::TypeArrayDisplay;
pub(crate) use self::infer::{
infer_deferred_types, infer_definition_types, infer_expression_types, infer_scope_types,
Expand All @@ -25,7 +24,9 @@ use crate::stdlib::{
builtins_symbol, core_module_symbol, typing_extensions_symbol, CoreStdlibModule,
};
use crate::symbol::{Boundness, Symbol};
use crate::types::diagnostic::TypeCheckDiagnosticsBuilder;
use crate::types::diagnostic::{
report_not_iterable, report_not_iterable_possibly_unbound, report_type_diagnostic,
};
use crate::types::mro::{ClassBase, Mro, MroError, MroIterator};
use crate::types::narrow::narrowing_constraint;
use crate::{Db, FxOrderSet, Module, Program};
Expand All @@ -43,21 +44,17 @@ mod unpacker;
#[cfg(test)]
mod property_tests;

#[salsa::tracked(return_ref)]
pub fn check_types(db: &dyn Db, file: File) -> TypeCheckDiagnostics {
#[salsa::tracked]
pub fn check_types(db: &dyn Db, file: File) {
let _span = tracing::trace_span!("check_types", file=?file.path(db)).entered();

tracing::debug!("Checking file '{path}'", path = file.path(db));

let index = semantic_index(db, file);
let mut diagnostics = TypeCheckDiagnostics::default();

for scope_id in index.scope_ids() {
let result = infer_scope_types(db, scope_id);
diagnostics.extend(result.diagnostics());
let _ = infer_scope_types(db, scope_id);
}

diagnostics
}

/// Infer the public type of a symbol (its type as seen from outside its scope).
Expand Down Expand Up @@ -2131,19 +2128,21 @@ impl<'db> CallOutcome<'db> {
}

/// Get the return type of the call, emitting default diagnostics if needed.
fn unwrap_with_diagnostic<'a>(
fn unwrap_with_diagnostic(
&self,
db: &'db dyn Db,
file: File,
node: ast::AnyNodeRef,
diagnostics: &'a mut TypeCheckDiagnosticsBuilder<'db>,
) -> Type<'db> {
match self.return_ty_result(db, node, diagnostics) {
match self.return_ty_result(db, file, node) {
Ok(return_ty) => return_ty,
Err(NotCallableError::Type {
not_callable_ty,
return_ty,
}) => {
diagnostics.add(
report_type_diagnostic(
db,
file,
node,
"call-non-callable",
format_args!(
Expand All @@ -2158,7 +2157,9 @@ impl<'db> CallOutcome<'db> {
called_ty,
return_ty,
}) => {
diagnostics.add(
report_type_diagnostic(
db,
file,
node,
"call-non-callable",
format_args!(
Expand All @@ -2174,7 +2175,9 @@ impl<'db> CallOutcome<'db> {
called_ty,
return_ty,
}) => {
diagnostics.add(
report_type_diagnostic(
db,
file,
node,
"call-non-callable",
format_args!(
Expand All @@ -2189,7 +2192,9 @@ impl<'db> CallOutcome<'db> {
callable_ty: called_ty,
return_ty,
}) => {
diagnostics.add(
report_type_diagnostic(
db,
file,
node,
"call-non-callable",
format_args!(
Expand All @@ -2203,19 +2208,21 @@ impl<'db> CallOutcome<'db> {
}

/// Get the return type of the call as a result.
fn return_ty_result<'a>(
fn return_ty_result(
&self,
db: &'db dyn Db,
file: File,
node: ast::AnyNodeRef,
diagnostics: &'a mut TypeCheckDiagnosticsBuilder<'db>,
) -> Result<Type<'db>, NotCallableError<'db>> {
match self {
Self::Callable { return_ty } => Ok(*return_ty),
Self::RevealType {
return_ty,
revealed_ty,
} => {
diagnostics.add(
report_type_diagnostic(
db,
file,
node,
"revealed-type",
format_args!("Revealed type is `{}`", revealed_ty.display(db)),
Expand Down Expand Up @@ -2254,10 +2261,10 @@ impl<'db> CallOutcome<'db> {
*return_ty
} else {
revealed = true;
outcome.unwrap_with_diagnostic(db, node, diagnostics)
outcome.unwrap_with_diagnostic(db, file, node)
}
}
_ => outcome.unwrap_with_diagnostic(db, node, diagnostics),
_ => outcome.unwrap_with_diagnostic(db, file, node),
};
union_builder = union_builder.add(return_ty);
}
Expand Down Expand Up @@ -2372,20 +2379,21 @@ enum IterationOutcome<'db> {
impl<'db> IterationOutcome<'db> {
fn unwrap_with_diagnostic(
self,
db: &dyn Db,
file: File,
iterable_node: ast::AnyNodeRef,
diagnostics: &mut TypeCheckDiagnosticsBuilder<'db>,
) -> Type<'db> {
match self {
Self::Iterable { element_ty } => element_ty,
Self::NotIterable { not_iterable_ty } => {
diagnostics.add_not_iterable(iterable_node, not_iterable_ty);
report_not_iterable(db, file, iterable_node, not_iterable_ty);
Type::Unknown
}
Self::PossiblyUnboundDunderIter {
iterable_ty,
element_ty,
} => {
diagnostics.add_not_iterable_possibly_unbound(iterable_node, iterable_ty);
report_not_iterable_possibly_unbound(db, file, iterable_node, iterable_ty);
element_ty
}
}
Expand Down
Loading

0 comments on commit 1f6247c

Please sign in to comment.