From aed0fec1a98a6716fb23c6b9d89bb56bf32b5295 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 16 Aug 2025 16:26:30 +0200 Subject: [PATCH] Auto-attach database in `Analysis` calls --- crates/hir-ty/src/next_solver/generic_arg.rs | 2 +- crates/ide-assists/src/lib.rs | 9 +-- crates/ide-assists/src/tests.rs | 17 ++++- crates/ide-completion/src/lib.rs | 5 +- crates/ide-completion/src/tests.rs | 6 +- crates/ide-diagnostics/src/lib.rs | 12 ++-- crates/ide-diagnostics/src/tests.rs | 75 ++++++++++++-------- crates/ide/src/goto_definition.rs | 4 +- crates/ide/src/hover.rs | 43 +++++------ crates/ide/src/hover/render.rs | 5 +- crates/ide/src/lib.rs | 57 ++++++++++----- crates/parser/src/lexed_str.rs | 3 +- 12 files changed, 137 insertions(+), 101 deletions(-) diff --git a/crates/hir-ty/src/next_solver/generic_arg.rs b/crates/hir-ty/src/next_solver/generic_arg.rs index 834f4e3765..76186e3746 100644 --- a/crates/hir-ty/src/next_solver/generic_arg.rs +++ b/crates/hir-ty/src/next_solver/generic_arg.rs @@ -46,7 +46,7 @@ impl<'db> GenericArg<'db> { pub fn expect_ty(self) -> Ty<'db> { match self.kind() { GenericArgKind::Type(ty) => ty, - _ => panic!("Expected ty, got {:?}", self), + _ => panic!("Expected ty, got {self:?}"), } } diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs index 5008f97447..4682c04732 100644 --- a/crates/ide-assists/src/lib.rs +++ b/crates/ide-assists/src/lib.rs @@ -67,7 +67,7 @@ mod tests; pub mod utils; use hir::Semantics; -use ide_db::{EditionedFileId, RootDatabase, base_db::salsa}; +use ide_db::{EditionedFileId, RootDatabase}; use syntax::{Edition, TextRange}; pub(crate) use crate::assist_context::{AssistContext, Assists}; @@ -93,11 +93,8 @@ pub fn assists( .unwrap_or_else(|| EditionedFileId::new(db, range.file_id, Edition::CURRENT)); let ctx = AssistContext::new(sema, config, hir::FileRange { file_id, range: range.range }); let mut acc = Assists::new(&ctx, resolve); - // the handlers may invoke trait solving related things which accesses salsa structs outside queries - salsa::attach(db, || { - handlers::all().iter().for_each(|handler| { - handler(&mut acc, &ctx); - }); + handlers::all().iter().for_each(|handler| { + handler(&mut acc, &ctx); }); acc.finish() } diff --git a/crates/ide-assists/src/tests.rs b/crates/ide-assists/src/tests.rs index f4daabfe91..c7c322a15e 100644 --- a/crates/ide-assists/src/tests.rs +++ b/crates/ide-assists/src/tests.rs @@ -1,7 +1,7 @@ mod generated; use expect_test::expect; -use hir::{Semantics, setup_tracing}; +use hir::{Semantics, db::HirDatabase, setup_tracing}; use ide_db::{ EditionedFileId, FileRange, RootDatabase, SnippetCap, assists::ExprFillDefaultMode, @@ -16,7 +16,7 @@ use test_utils::{assert_eq_text, extract_offset}; use crate::{ Assist, AssistConfig, AssistContext, AssistKind, AssistResolveStrategy, Assists, SingleResolve, - assists, handlers::Handler, + handlers::Handler, }; pub(crate) const TEST_CONFIG: AssistConfig = AssistConfig { @@ -103,6 +103,18 @@ pub(crate) const TEST_CONFIG_IMPORT_ONE: AssistConfig = AssistConfig { prefer_self_ty: false, }; +fn assists( + db: &RootDatabase, + config: &AssistConfig, + resolve: AssistResolveStrategy, + range: ide_db::FileRange, +) -> Vec { + salsa::attach(db, || { + HirDatabase::zalsa_register_downcaster(db); + crate::assists(db, config, resolve, range) + }) +} + pub(crate) fn with_single_file(text: &str) -> (RootDatabase, EditionedFileId) { RootDatabase::with_single_file(text) } @@ -320,6 +332,7 @@ fn check_with_config( }; let mut acc = Assists::new(&ctx, resolve); salsa::attach(&db, || { + HirDatabase::zalsa_register_downcaster(&db); handler(&mut acc, &ctx); }); let mut res = acc.finish(); diff --git a/crates/ide-completion/src/lib.rs b/crates/ide-completion/src/lib.rs index 1a4c97e70b..a70a1138d2 100644 --- a/crates/ide-completion/src/lib.rs +++ b/crates/ide-completion/src/lib.rs @@ -10,7 +10,6 @@ mod snippet; #[cfg(test)] mod tests; -use base_db::salsa; use ide_db::{ FilePosition, FxHashSet, RootDatabase, imports::insert_use::{self, ImportScope}, @@ -229,7 +228,7 @@ pub fn completions( { let acc = &mut completions; - salsa::attach(db, || match analysis { + match analysis { CompletionAnalysis::Name(name_ctx) => completions::complete_name(acc, ctx, name_ctx), CompletionAnalysis::NameRef(name_ref_ctx) => { completions::complete_name_ref(acc, ctx, name_ref_ctx) @@ -257,7 +256,7 @@ pub fn completions( ); } CompletionAnalysis::UnexpandedAttrTT { .. } | CompletionAnalysis::String { .. } => (), - }) + } } Some(completions.into()) diff --git a/crates/ide-completion/src/tests.rs b/crates/ide-completion/src/tests.rs index 4b3b271ca2..809a26bf5d 100644 --- a/crates/ide-completion/src/tests.rs +++ b/crates/ide-completion/src/tests.rs @@ -24,7 +24,7 @@ mod type_pos; mod use_tree; mod visibility; -use base_db::SourceDatabase; +use base_db::{SourceDatabase, salsa}; use expect_test::Expect; use hir::{PrefixKind, setup_tracing}; use ide_db::{ @@ -243,7 +243,7 @@ pub(crate) fn check_edit_with_config( let ra_fixture_after = trim_indent(ra_fixture_after); let (db, position) = position(ra_fixture_before); let completions: Vec = - crate::completions(&db, &config, position, None).unwrap(); + salsa::attach(&db, || crate::completions(&db, &config, position, None).unwrap()); let (completion,) = completions .iter() .filter(|it| it.lookup() == what) @@ -306,7 +306,7 @@ pub(crate) fn get_all_items( trigger_character: Option, ) -> Vec { let (db, position) = position(code); - let res = crate::completions(&db, &config, position, trigger_character) + let res = salsa::attach(&db, || crate::completions(&db, &config, position, trigger_character)) .map_or_else(Vec::default, Into::into); // validate res.iter().for_each(|it| { diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs index a4eb3d47d7..a1db92641f 100644 --- a/crates/ide-diagnostics/src/lib.rs +++ b/crates/ide-diagnostics/src/lib.rs @@ -92,7 +92,7 @@ use hir::{ use ide_db::{ EditionedFileId, FileId, FileRange, FxHashMap, FxHashSet, RootDatabase, Severity, SnippetCap, assists::{Assist, AssistId, AssistResolveStrategy, ExprFillDefaultMode}, - base_db::{ReleaseChannel, RootQueryDb as _, salsa}, + base_db::{ReleaseChannel, RootQueryDb as _}, generated::lints::{CLIPPY_LINT_GROUPS, DEFAULT_LINT_GROUPS, DEFAULT_LINTS, Lint, LintGroup}, imports::insert_use::InsertUseConfig, label::Label, @@ -537,12 +537,10 @@ pub fn full_diagnostics( resolve: &AssistResolveStrategy, file_id: FileId, ) -> Vec { - salsa::attach(db, || { - let mut res = syntax_diagnostics(db, config, file_id); - let sema = semantic_diagnostics(db, config, resolve, file_id); - res.extend(sema); - res - }) + let mut res = syntax_diagnostics(db, config, file_id); + let sema = semantic_diagnostics(db, config, resolve, file_id); + res.extend(sema); + res } /// Returns whether to keep this diagnostic (or remove it). diff --git a/crates/ide-diagnostics/src/tests.rs b/crates/ide-diagnostics/src/tests.rs index c3cc5a08b5..1839ab1c58 100644 --- a/crates/ide-diagnostics/src/tests.rs +++ b/crates/ide-diagnostics/src/tests.rs @@ -6,7 +6,7 @@ use hir::setup_tracing; use ide_db::{ LineIndexDatabase, RootDatabase, assists::{AssistResolveStrategy, ExprFillDefaultMode}, - base_db::SourceDatabase, + base_db::{SourceDatabase, salsa}, }; use itertools::Itertools; use stdx::trim_indent; @@ -74,14 +74,16 @@ fn check_nth_fix_with_config( let after = trim_indent(ra_fixture_after); let (db, file_position) = RootDatabase::with_position(ra_fixture_before); - let diagnostic = super::full_diagnostics( - &db, - &config, - &AssistResolveStrategy::All, - file_position.file_id.file_id(&db), - ) - .pop() - .expect("no diagnostics"); + let diagnostic = salsa::attach(&db, || { + super::full_diagnostics( + &db, + &config, + &AssistResolveStrategy::All, + file_position.file_id.file_id(&db), + ) + .pop() + .expect("no diagnostics") + }); let fix = &diagnostic .fixes .unwrap_or_else(|| panic!("{:?} diagnostic misses fixes", diagnostic.code))[nth]; @@ -127,12 +129,14 @@ pub(crate) fn check_has_fix( let (db, file_position) = RootDatabase::with_position(ra_fixture_before); let mut conf = DiagnosticsConfig::test_sample(); conf.expr_fill_default = ExprFillDefaultMode::Default; - let fix = super::full_diagnostics( - &db, - &conf, - &AssistResolveStrategy::All, - file_position.file_id.file_id(&db), - ) + let fix = salsa::attach(&db, || { + super::full_diagnostics( + &db, + &conf, + &AssistResolveStrategy::All, + file_position.file_id.file_id(&db), + ) + }) .into_iter() .find(|d| { d.fixes @@ -166,12 +170,14 @@ pub(crate) fn check_has_fix( /// Checks that there's a diagnostic *without* fix at `$0`. pub(crate) fn check_no_fix(#[rust_analyzer::rust_fixture] ra_fixture: &str) { let (db, file_position) = RootDatabase::with_position(ra_fixture); - let diagnostic = super::full_diagnostics( - &db, - &DiagnosticsConfig::test_sample(), - &AssistResolveStrategy::All, - file_position.file_id.file_id(&db), - ) + let diagnostic = salsa::attach(&db, || { + super::full_diagnostics( + &db, + &DiagnosticsConfig::test_sample(), + &AssistResolveStrategy::All, + file_position.file_id.file_id(&db), + ) + }) .pop() .unwrap(); assert!(diagnostic.fixes.is_none(), "got a fix when none was expected: {diagnostic:?}"); @@ -206,7 +212,13 @@ pub(crate) fn check_diagnostics_with_config( .iter() .copied() .flat_map(|file_id| { - super::full_diagnostics(&db, &config, &AssistResolveStrategy::All, file_id.file_id(&db)) + salsa::attach(&db, || { + super::full_diagnostics( + &db, + &config, + &AssistResolveStrategy::All, + file_id.file_id(&db), + ) .into_iter() .map(|d| { let mut annotation = String::new(); @@ -224,6 +236,7 @@ pub(crate) fn check_diagnostics_with_config( annotation.push_str(&d.message); (d.range, annotation) }) + }) }) .map(|(diagnostic, annotation)| (diagnostic.file_id, (diagnostic.range, annotation))) .into_group_map(); @@ -275,15 +288,19 @@ fn test_disabled_diagnostics() { let (db, file_id) = RootDatabase::with_single_file(r#"mod foo;"#); let file_id = file_id.file_id(&db); - let diagnostics = super::full_diagnostics(&db, &config, &AssistResolveStrategy::All, file_id); + let diagnostics = salsa::attach(&db, || { + super::full_diagnostics(&db, &config, &AssistResolveStrategy::All, file_id) + }); assert!(diagnostics.is_empty()); - let diagnostics = super::full_diagnostics( - &db, - &DiagnosticsConfig::test_sample(), - &AssistResolveStrategy::All, - file_id, - ); + let diagnostics = salsa::attach(&db, || { + super::full_diagnostics( + &db, + &DiagnosticsConfig::test_sample(), + &AssistResolveStrategy::All, + file_id, + ) + }); assert!(!diagnostics.is_empty()); } diff --git a/crates/ide/src/goto_definition.rs b/crates/ide/src/goto_definition.rs index 633feec622..f768d4b68f 100644 --- a/crates/ide/src/goto_definition.rs +++ b/crates/ide/src/goto_definition.rs @@ -10,7 +10,7 @@ use hir::{ }; use ide_db::{ RootDatabase, SymbolKind, - base_db::{AnchoredPath, SourceDatabase, salsa}, + base_db::{AnchoredPath, SourceDatabase}, defs::{Definition, IdentClass}, famous_defs::FamousDefs, helpers::pick_best_token, @@ -108,7 +108,7 @@ pub(crate) fn goto_definition( } Some( - salsa::attach(sema.db, || IdentClass::classify_node(sema, &parent))? + IdentClass::classify_node(sema, &parent)? .definitions() .into_iter() .flat_map(|(def, _)| { diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index b0ef83e050..44c98a43f6 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -12,7 +12,6 @@ use hir::{ }; use ide_db::{ FileRange, FxIndexSet, Ranker, RootDatabase, - base_db::salsa, defs::{Definition, IdentClass, NameRefClass, OperatorClass}, famous_defs::FamousDefs, helpers::pick_best_token, @@ -137,20 +136,18 @@ pub(crate) fn hover( let edition = sema.attach_first_edition(file_id).map(|it| it.edition(db)).unwrap_or(Edition::CURRENT); let display_target = sema.first_crate(file_id)?.to_display_target(db); - let mut res = salsa::attach(sema.db, || { - if range.is_empty() { - hover_offset( - sema, - FilePosition { file_id, offset: range.start() }, - file, - config, - edition, - display_target, - ) - } else { - hover_ranged(sema, frange, file, config, edition, display_target) - } - })?; + let mut res = if range.is_empty() { + hover_offset( + sema, + FilePosition { file_id, offset: range.start() }, + file, + config, + edition, + display_target, + ) + } else { + hover_ranged(sema, frange, file, config, edition, display_target) + }?; if let HoverDocFormat::PlainText = config.format { res.info.markup = remove_markdown(res.info.markup.as_str()).into(); @@ -293,7 +290,7 @@ fn hover_offset( .into_iter() .unique_by(|&((def, _), _, _, _)| def) .map(|((def, subst), macro_arm, hovered_definition, node)| { - salsa::attach(sema.db, || hover_for_definition( + hover_for_definition( sema, file_id, def, @@ -304,7 +301,7 @@ fn hover_offset( config, edition, display_target, - )) + ) }) .collect::>(), ) @@ -583,13 +580,11 @@ fn goto_type_action_for_def( }); } - salsa::attach(db, || { - if let Ok(generic_def) = GenericDef::try_from(def) { - generic_def.type_or_const_params(db).into_iter().for_each(|it| { - walk_and_push_ty(db, &it.ty(db), &mut push_new_def); - }); - } - }); + if let Ok(generic_def) = GenericDef::try_from(def) { + generic_def.type_or_const_params(db).into_iter().for_each(|it| { + walk_and_push_ty(db, &it.ty(db), &mut push_new_def); + }); + } let ty = match def { Definition::Local(it) => Some(it.ty(db)), diff --git a/crates/ide/src/hover/render.rs b/crates/ide/src/hover/render.rs index 1f9d10c92b..290ee80984 100644 --- a/crates/ide/src/hover/render.rs +++ b/crates/ide/src/hover/render.rs @@ -10,7 +10,6 @@ use hir::{ }; use ide_db::{ RootDatabase, - base_db::salsa, defs::Definition, documentation::{DocsRangeMap, HasDocs}, famous_defs::FamousDefs, @@ -45,7 +44,7 @@ pub(super) fn type_info_of( Either::Left(expr) => sema.type_of_expr(expr)?, Either::Right(pat) => sema.type_of_pat(pat)?, }; - salsa::attach(sema.db, || type_info(sema, _config, ty_info, edition, display_target)) + type_info(sema, _config, ty_info, edition, display_target) } pub(super) fn closure_expr( @@ -912,7 +911,7 @@ pub(super) fn literal( }; let ty = ty.display(sema.db, display_target); - let mut s = salsa::attach(sema.db, || format!("```rust\n{ty}\n```\n___\n\n")); + let mut s = format!("```rust\n{ty}\n```\n___\n\n"); match value { Ok(value) => { let backtick_len = value.chars().filter(|c| *c == '`').count(); diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index e491c9214b..874e04702e 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -62,7 +62,7 @@ use std::panic::{AssertUnwindSafe, UnwindSafe}; use cfg::CfgOptions; use fetch_crates::CrateInfo; -use hir::{ChangeWithProcMacros, EditionedFileId, crate_def_map, sym}; +use hir::{ChangeWithProcMacros, EditionedFileId, crate_def_map, db::HirDatabase, sym}; use ide_db::{ FxHashMap, FxIndexSet, LineIndexDatabase, base_db::{ @@ -478,10 +478,12 @@ impl Analysis { /// Fuzzy searches for a symbol. pub fn symbol_search(&self, query: Query, limit: usize) -> Cancellable> { - self.with_db(|db| { - symbol_index::world_symbols(db, query) - .into_iter() // xx: should we make this a par iter? - .filter_map(|s| s.try_to_nav(db)) + // `world_symbols` currently clones the database to run stuff in parallel, which will make any query panic + // if we were to attach it here. + Cancelled::catch(|| { + symbol_index::world_symbols(&self.db, query) + .into_iter() + .filter_map(|s| s.try_to_nav(&self.db)) .take(limit) .map(UpmappingResult::call_site) .collect::>() @@ -660,15 +662,6 @@ impl Analysis { }) } - /// Computes syntax highlighting for the given file - pub fn highlight( - &self, - highlight_config: HighlightConfig, - file_id: FileId, - ) -> Cancellable> { - self.with_db(|db| syntax_highlighting::highlight(db, highlight_config, file_id, None)) - } - /// Computes all ranges to highlight for a given item in a file. pub fn highlight_related( &self, @@ -682,20 +675,42 @@ impl Analysis { }) } + /// Computes syntax highlighting for the given file + pub fn highlight( + &self, + highlight_config: HighlightConfig, + file_id: FileId, + ) -> Cancellable> { + // highlighting may construct a new database for "speculative" execution, so we can't currently attach the database + // highlighting instead sets up the attach hook where neceesary for the trait solver + Cancelled::catch(|| { + syntax_highlighting::highlight(&self.db, highlight_config, file_id, None) + }) + } + /// Computes syntax highlighting for the given file range. pub fn highlight_range( &self, highlight_config: HighlightConfig, frange: FileRange, ) -> Cancellable> { - self.with_db(|db| { - syntax_highlighting::highlight(db, highlight_config, frange.file_id, Some(frange.range)) + // highlighting may construct a new database for "speculative" execution, so we can't currently attach the database + // highlighting instead sets up the attach hook where neceesary for the trait solver + Cancelled::catch(|| { + syntax_highlighting::highlight( + &self.db, + highlight_config, + frange.file_id, + Some(frange.range), + ) }) } /// Computes syntax highlighting for the given file. pub fn highlight_as_html(&self, file_id: FileId, rainbow: bool) -> Cancellable { - self.with_db(|db| syntax_highlighting::highlight_as_html(db, file_id, rainbow)) + // highlighting may construct a new database for "speculative" execution, so we can't currently attach the database + // highlighting instead sets up the attach hook where neceesary for the trait solver + Cancelled::catch(|| syntax_highlighting::highlight_as_html(&self.db, file_id, rainbow)) } /// Computes completions at the given position. @@ -873,8 +888,12 @@ impl Analysis { where F: FnOnce(&RootDatabase) -> T + std::panic::UnwindSafe, { - let snap = self.db.clone(); - Cancelled::catch(|| f(&snap)) + salsa::attach(&self.db, || { + // the trait solver code may invoke `as_view` outside of queries, + // so technically we might run into a panic in salsa if the downcaster has not yet been registered. + HirDatabase::zalsa_register_downcaster(&self.db); + Cancelled::catch(|| f(&self.db)) + }) } } diff --git a/crates/parser/src/lexed_str.rs b/crates/parser/src/lexed_str.rs index dcf397142c..edc3f406a6 100644 --- a/crates/parser/src/lexed_str.rs +++ b/crates/parser/src/lexed_str.rs @@ -289,8 +289,7 @@ impl<'a> Converter<'a> { let error_msg = if has_unterminated { format!( - "unknown literal prefix `{}` (note: check for unterminated string literal)", - token_text + "unknown literal prefix `{token_text}` (note: check for unterminated string literal)" ) } else { "unknown literal prefix".to_owned()