diff --git a/crates/ruff_db/src/panic.rs b/crates/ruff_db/src/panic.rs index d91c1f7fe7a84..f5c176d5f44df 100644 --- a/crates/ruff_db/src/panic.rs +++ b/crates/ruff_db/src/panic.rs @@ -32,6 +32,10 @@ impl Payload { } impl PanicError { + pub fn resume_unwind(self) -> ! { + std::panic::resume_unwind(self.payload.0) + } + pub fn to_diagnostic_message(&self, path: Option) -> String { use std::fmt::Write; diff --git a/crates/ty_project/src/lib.rs b/crates/ty_project/src/lib.rs index d62af21b6c569..a5362100602d3 100644 --- a/crates/ty_project/src/lib.rs +++ b/crates/ty_project/src/lib.rs @@ -20,8 +20,7 @@ use ruff_db::parsed::parsed_module; use ruff_db::source::{SourceTextError, source_text}; use ruff_db::system::{SystemPath, SystemPathBuf}; use rustc_hash::FxHashSet; -use salsa::Durability; -use salsa::Setter; +use salsa::{Database, Durability, Setter}; use std::backtrace::BacktraceStatus; use std::collections::hash_set; use std::iter::FusedIterator; @@ -319,6 +318,9 @@ impl Project { for file in &files { let db = db.clone(); let reporter = &*reporter; + + db.unwind_if_revision_cancelled(); + scope.spawn(move |_| { let check_file_span = tracing::debug_span!(parent: project_span, "check_file", ?file); @@ -646,10 +648,9 @@ pub(crate) fn check_file_impl(db: &dyn Db, file: File) -> Result { + Ok(type_check_diagnostics) => { diagnostics.extend(type_check_diagnostics); } - Ok(None) => {} Err(diagnostic) => diagnostics.push(diagnostic), } } @@ -749,16 +750,37 @@ enum IOErrorKind { SourceText(#[from] SourceTextError), } -fn catch(db: &dyn Db, file: File, f: F) -> Result, Diagnostic> +fn catch(db: &dyn Db, file: File, f: F) -> Result where F: FnOnce() -> R + UnwindSafe, { - match ruff_db::panic::catch_unwind(|| { - // Ignore salsa errors - salsa::Cancelled::catch(f).ok() - }) { + match ruff_db::panic::catch_unwind(f) { Ok(result) => Ok(result), Err(error) => { + match error.payload.downcast_ref::() { + None => { + // Add a diagnostic (by not early returning) for + // any non Salsa panic (a bug in ty) + } + Some(salsa::Cancelled::PropagatedPanic) => { + // Add a diagnostic for propagated Salsa panics. That is, query `A` + // running on thread `a` depends on query `B` running on thread `b` + // and query `B` panics. However, avoid adding such a diagnostic + // if query `B` panicked because of a cancellation by calling + // `unwind_if_revision_cancelled`. + // + // The propagated Salsa panic isn't very actionable for users, + // but it can be useful to know that file A failed to type check + // because file B panicked (both files will have a panic-diagnostic). + db.unwind_if_revision_cancelled(); + } + + // For any pending write or local cancellation, resume the panic to abort the outer query. + Some(_) => { + error.resume_unwind(); + } + } + let message = error.to_diagnostic_message(Some(file.path(db))); let mut diagnostic = Diagnostic::new(DiagnosticId::Panic, Severity::Fatal, message); diagnostic.add_bug_sub_diagnostics("%5Bpanic%5D"); @@ -790,6 +812,10 @@ where }); } + // Report an untracked read because Salsa didn't carry over + // the dependencies of any query called by `f` because it panicked. + db.report_untracked_read(); + Err(diagnostic) } }