[red-knot] Include salsa backtrace in check and mdtest panic messages (#17732)

Co-authored-by: David Peter <sharkdp@users.noreply.github.com>
This commit is contained in:
Micha Reiser 2025-04-30 10:26:40 +02:00 committed by GitHub
parent 8a6787b39e
commit d94be0e780
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 83 additions and 19 deletions

6
Cargo.lock generated
View File

@ -3445,7 +3445,7 @@ checksum = "28d3b2b1366ec20994f1fd18c3c594f05c5dd4bc44d8bb0c1c632c8d6829481f"
[[package]]
name = "salsa"
version = "0.21.0"
source = "git+https://github.com/salsa-rs/salsa.git?rev=af811a34cef26932859b09bbefe50fbd8e8db06a#af811a34cef26932859b09bbefe50fbd8e8db06a"
source = "git+https://github.com/salsa-rs/salsa.git?rev=79afd59ed5a5edb4dac63cf5b6cf4a6aa9514bdf#79afd59ed5a5edb4dac63cf5b6cf4a6aa9514bdf"
dependencies = [
"boxcar",
"compact_str",
@ -3468,12 +3468,12 @@ dependencies = [
[[package]]
name = "salsa-macro-rules"
version = "0.21.0"
source = "git+https://github.com/salsa-rs/salsa.git?rev=af811a34cef26932859b09bbefe50fbd8e8db06a#af811a34cef26932859b09bbefe50fbd8e8db06a"
source = "git+https://github.com/salsa-rs/salsa.git?rev=79afd59ed5a5edb4dac63cf5b6cf4a6aa9514bdf#79afd59ed5a5edb4dac63cf5b6cf4a6aa9514bdf"
[[package]]
name = "salsa-macros"
version = "0.21.0"
source = "git+https://github.com/salsa-rs/salsa.git?rev=af811a34cef26932859b09bbefe50fbd8e8db06a#af811a34cef26932859b09bbefe50fbd8e8db06a"
source = "git+https://github.com/salsa-rs/salsa.git?rev=79afd59ed5a5edb4dac63cf5b6cf4a6aa9514bdf#79afd59ed5a5edb4dac63cf5b6cf4a6aa9514bdf"
dependencies = [
"heck",
"proc-macro2",

View File

@ -124,7 +124,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 = "af811a34cef26932859b09bbefe50fbd8e8db06a" }
salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "79afd59ed5a5edb4dac63cf5b6cf4a6aa9514bdf" }
schemars = { version = "0.8.16" }
seahash = { version = "4.1.0" }
serde = { version = "1.0.197", features = ["derive"] }

View File

@ -20,6 +20,7 @@ use ruff_db::system::{SystemPath, SystemPathBuf};
use rustc_hash::FxHashSet;
use salsa::Durability;
use salsa::Setter;
use std::backtrace::BacktraceStatus;
use std::panic::{AssertUnwindSafe, UnwindSafe};
use std::sync::Arc;
use thiserror::Error;
@ -626,10 +627,27 @@ where
));
if let Some(backtrace) = error.backtrace {
diagnostic.sub(SubDiagnostic::new(
Severity::Info,
format!("Backtrace:\n{backtrace}"),
));
match backtrace.status() {
BacktraceStatus::Disabled => {
diagnostic.sub(SubDiagnostic::new(
Severity::Info,
"run with `RUST_BACKTRACE=1` environment variable to show the full backtrace information",
));
}
BacktraceStatus::Captured => {
diagnostic.sub(SubDiagnostic::new(
Severity::Info,
format!("Backtrace:\n{backtrace}"),
));
}
_ => {}
}
}
if let Some(backtrace) = error.salsa_backtrace {
salsa::attach(db, || {
diagnostic.sub(SubDiagnostic::new(Severity::Info, backtrace.to_string()));
});
}
Err(diagnostic)

View File

@ -8483,7 +8483,7 @@ mod tests {
db.clear_salsa_events();
assert_file_diagnostics(&db, "src/a.py", &[]);
let events = db.take_salsa_events();
let cycles = salsa::plumbing::attach(&db, || {
let cycles = salsa::attach(&db, || {
events
.iter()
.filter_map(|event| {

View File

@ -18,6 +18,7 @@ use ruff_db::parsed::parsed_module;
use ruff_db::system::{DbWithWritableSystem as _, SystemPath, SystemPathBuf};
use ruff_db::testing::{setup_logging, setup_logging_with_filter};
use ruff_source_file::{LineIndex, OneIndexed};
use std::backtrace::BacktraceStatus;
use std::fmt::Write;
mod assertion;
@ -331,10 +332,24 @@ fn run_test(
None => messages.push("Box<dyn Any>".to_string()),
}
if let Some(backtrace) = info.backtrace {
if std::env::var("RUST_BACKTRACE").is_ok() {
messages.extend(backtrace.to_string().split('\n').map(String::from));
match backtrace.status() {
BacktraceStatus::Disabled => {
let msg = "run with `RUST_BACKTRACE=1` environment variable to display a backtrace";
messages.push(msg.to_string());
}
BacktraceStatus::Captured => {
messages.extend(backtrace.to_string().split('\n').map(String::from));
}
_ => {}
}
}
if let Some(backtrace) = info.salsa_backtrace {
salsa::attach(db, || {
messages.extend(backtrace.to_string().split('\n').map(String::from));
});
}
by_line.push(OneIndexed::from_zero_indexed(0), messages);
return Some(FileFailures {
backtick_offsets: test_file.backtick_offsets,

View File

@ -160,7 +160,7 @@ pub(crate) fn format(
}),
Err(error) => Err(FormatCommandError::Panic(
Some(resolved_file.path().to_path_buf()),
error,
Box::new(error),
)),
},
)
@ -635,7 +635,7 @@ impl<'a> FormatResults<'a> {
pub(crate) enum FormatCommandError {
Ignore(#[from] ignore::Error),
Parse(#[from] DisplayParseError),
Panic(Option<PathBuf>, PanicError),
Panic(Option<PathBuf>, Box<PanicError>),
Read(Option<PathBuf>, SourceError),
Format(Option<PathBuf>, FormatModuleError),
Write(Option<PathBuf>, SourceError),

View File

@ -1,3 +1,4 @@
use std::backtrace::BacktraceStatus;
use std::cell::Cell;
use std::panic::Location;
use std::sync::OnceLock;
@ -7,6 +8,7 @@ pub struct PanicError {
pub location: Option<String>,
pub payload: Payload,
pub backtrace: Option<std::backtrace::Backtrace>,
pub salsa_backtrace: Option<salsa::Backtrace>,
}
#[derive(Debug)]
@ -34,15 +36,35 @@ impl std::fmt::Display for PanicError {
write!(f, ":\n{payload}")?;
}
if let Some(backtrace) = &self.backtrace {
writeln!(f, "\nBacktrace: {backtrace}")?;
match backtrace.status() {
BacktraceStatus::Disabled => {
writeln!(
f,
"\nrun with `RUST_BACKTRACE=1` environment variable to display a backtrace"
)?;
}
BacktraceStatus::Captured => {
writeln!(f, "\nBacktrace: {backtrace}")?;
}
_ => {}
}
}
Ok(())
}
}
#[derive(Default)]
struct CapturedPanicInfo {
backtrace: Option<std::backtrace::Backtrace>,
location: Option<String>,
salsa_backtrace: Option<salsa::Backtrace>,
}
thread_local! {
static CAPTURE_PANIC_INFO: Cell<bool> = const { Cell::new(false) };
static LAST_BACKTRACE: Cell<(Option<std::backtrace::Backtrace>, Option<String>)> = const { Cell::new((None, None)) };
static LAST_BACKTRACE: Cell<CapturedPanicInfo> = const {
Cell::new(CapturedPanicInfo { backtrace: None, location: None, salsa_backtrace: None })
};
}
fn install_hook() {
@ -56,9 +78,13 @@ fn install_hook() {
}
let location = info.location().map(Location::to_string);
let backtrace = Some(std::backtrace::Backtrace::force_capture());
let backtrace = Some(std::backtrace::Backtrace::capture());
LAST_BACKTRACE.set((backtrace, location));
LAST_BACKTRACE.set(CapturedPanicInfo {
backtrace,
location,
salsa_backtrace: salsa::Backtrace::capture(),
});
}));
});
}
@ -92,12 +118,17 @@ where
// Because of that, always take the payload from `catch_unwind` because it may have been transformed
// by an inner `std::panic::catch_unwind` handlers and only use the information
// from the custom handler to enrich the error with the backtrace and location.
let (backtrace, location) = LAST_BACKTRACE.with(Cell::take);
let CapturedPanicInfo {
location,
backtrace,
salsa_backtrace,
} = LAST_BACKTRACE.with(Cell::take);
PanicError {
location,
payload: Payload(payload),
backtrace,
salsa_backtrace,
}
});
CAPTURE_PANIC_INFO.set(prev_should_capture);

View File

@ -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 = "af811a34cef26932859b09bbefe50fbd8e8db06a" }
salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "79afd59ed5a5edb4dac63cf5b6cf4a6aa9514bdf" }
similar = { version = "2.5.0" }
tracing = { version = "0.1.40" }