From 8520a0c585d4f6772b40de7c4511e2ae891bab08 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 10 Jun 2024 10:33:07 +0200 Subject: [PATCH 1/2] Thread more HasSource::source calls through Semantics for caching --- crates/hir/src/has_source.rs | 69 +++++++++++++++- crates/hir/src/lib.rs | 85 +++++++------------- crates/ide-assists/src/utils/suggest_name.rs | 2 +- crates/ide-db/src/active_parameter.rs | 2 +- crates/ide-db/src/rename.rs | 20 ++--- crates/ide-ssr/src/matching.rs | 2 +- crates/ide/src/inlay_hints/param_name.rs | 4 +- crates/ide/src/navigation_target.rs | 12 +-- crates/ide/src/rename.rs | 7 +- crates/ide/src/signature_help.rs | 2 +- 10 files changed, 121 insertions(+), 84 deletions(-) diff --git a/crates/hir/src/has_source.rs b/crates/hir/src/has_source.rs index 308e8a31ca..4b3b7ff4c4 100644 --- a/crates/hir/src/has_source.rs +++ b/crates/hir/src/has_source.rs @@ -8,13 +8,14 @@ use hir_def::{ Lookup, MacroId, VariantId, }; use hir_expand::{HirFileId, InFile}; +use hir_ty::{db::InternedClosure, CallableDefId}; use syntax::ast; use tt::TextRange; use crate::{ - db::HirDatabase, Adt, Const, Enum, ExternCrateDecl, Field, FieldSource, Function, Impl, - LifetimeParam, LocalSource, Macro, Module, Static, Struct, Trait, TraitAlias, TypeAlias, - TypeOrConstParam, Union, Variant, + db::HirDatabase, Adt, Callee, Const, Enum, ExternCrateDecl, Field, FieldSource, Function, Impl, + Label, LifetimeParam, LocalSource, Macro, Module, Param, SelfParam, Static, Struct, Trait, + TraitAlias, TypeAlias, TypeOrConstParam, Union, Variant, }; pub trait HasSource { @@ -222,6 +223,68 @@ impl HasSource for LocalSource { } } +impl HasSource for Param { + type Ast = Either; + + fn source(self, db: &dyn HirDatabase) -> Option> { + match self.func { + Callee::Def(CallableDefId::FunctionId(func)) => { + let InFile { file_id, value } = Function { id: func }.source(db)?; + let params = value.param_list()?; + if let Some(self_param) = params.self_param() { + if let Some(idx) = self.idx.checked_sub(1) { + params.params().nth(idx).map(Either::Right) + } else { + Some(Either::Left(self_param)) + } + } else { + params.params().nth(self.idx).map(Either::Right) + } + .map(|value| InFile { file_id, value }) + } + Callee::Closure(closure, _) => { + let InternedClosure(owner, expr_id) = db.lookup_intern_closure(closure.into()); + let (_, source_map) = db.body_with_source_map(owner); + let ast @ InFile { file_id, value } = source_map.expr_syntax(expr_id).ok()?; + let root = db.parse_or_expand(file_id); + match value.to_node(&root) { + ast::Expr::ClosureExpr(it) => it + .param_list()? + .params() + .nth(self.idx) + .map(Either::Right) + .map(|value| InFile { file_id: ast.file_id, value }), + _ => None, + } + } + _ => None, + } + } +} + +impl HasSource for SelfParam { + type Ast = ast::SelfParam; + + fn source(self, db: &dyn HirDatabase) -> Option> { + let InFile { file_id, value } = Function::from(self.func).source(db)?; + value + .param_list() + .and_then(|params| params.self_param()) + .map(|value| InFile { file_id, value }) + } +} + +impl HasSource for Label { + type Ast = ast::Label; + + fn source(self, db: &dyn HirDatabase) -> Option> { + let (_body, source_map) = db.body_with_source_map(self.parent); + let src = source_map.label_syntax(self.label_id); + let root = src.file_syntax(db.upcast()); + Some(src.map(|ast| ast.to_node(&root))) + } +} + impl HasSource for ExternCrateDecl { type Ast = ast::ExternCrate; diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index c69336407e..05fa7c4964 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -64,7 +64,6 @@ use hir_expand::{ use hir_ty::{ all_super_traits, autoderef, check_orphan_rules, consteval::{try_const_usize, unknown_const_as_generic, ConstExt}, - db::InternedClosure, diagnostics::BodyValidationDiagnostic, error_lifetime, known_const_to_ast, layout::{Layout as TyLayout, RustcEnumVariantIdx, RustcFieldIdx, TagEncoding}, @@ -1099,6 +1098,35 @@ pub enum FieldSource { Pos(ast::TupleField), } +impl AstNode for FieldSource { + fn can_cast(kind: syntax::SyntaxKind) -> bool + where + Self: Sized, + { + ast::RecordField::can_cast(kind) || ast::TupleField::can_cast(kind) + } + + fn cast(syntax: SyntaxNode) -> Option + where + Self: Sized, + { + if ast::RecordField::can_cast(syntax.kind()) { + ::cast(syntax).map(FieldSource::Named) + } else if ast::TupleField::can_cast(syntax.kind()) { + ::cast(syntax).map(FieldSource::Pos) + } else { + None + } + } + + fn syntax(&self) -> &SyntaxNode { + match self { + FieldSource::Named(it) => it.syntax(), + FieldSource::Pos(it) => it.syntax(), + } + } +} + impl Field { pub fn name(&self, db: &dyn HirDatabase) -> Name { self.parent.variant_data(db).fields()[self.id].name.clone() @@ -2216,47 +2244,9 @@ impl Param { } } - pub fn pattern_source(&self, db: &dyn HirDatabase) -> Option { + pub fn pattern_source(self, db: &dyn HirDatabase) -> Option { self.source(db).and_then(|p| p.value.right()?.pat()) } - - pub fn source( - &self, - db: &dyn HirDatabase, - ) -> Option>> { - match self.func { - Callee::Def(CallableDefId::FunctionId(func)) => { - let InFile { file_id, value } = Function { id: func }.source(db)?; - let params = value.param_list()?; - if let Some(self_param) = params.self_param() { - if let Some(idx) = self.idx.checked_sub(1) { - params.params().nth(idx).map(Either::Right) - } else { - Some(Either::Left(self_param)) - } - } else { - params.params().nth(self.idx).map(Either::Right) - } - .map(|value| InFile { file_id, value }) - } - Callee::Closure(closure, _) => { - let InternedClosure(owner, expr_id) = db.lookup_intern_closure(closure.into()); - let (_, source_map) = db.body_with_source_map(owner); - let ast @ InFile { file_id, value } = source_map.expr_syntax(expr_id).ok()?; - let root = db.parse_or_expand(file_id); - match value.to_node(&root) { - ast::Expr::ClosureExpr(it) => it - .param_list()? - .params() - .nth(self.idx) - .map(Either::Right) - .map(|value| InFile { file_id: ast.file_id, value }), - _ => None, - } - } - _ => None, - } - } } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -2280,14 +2270,6 @@ impl SelfParam { .unwrap_or(Access::Owned) } - pub fn source(&self, db: &dyn HirDatabase) -> Option> { - let InFile { file_id, value } = Function::from(self.func).source(db)?; - value - .param_list() - .and_then(|params| params.self_param()) - .map(|value| InFile { file_id, value }) - } - pub fn parent_fn(&self) -> Function { Function::from(self.func) } @@ -3458,13 +3440,6 @@ impl Label { let body = db.body(self.parent); body[self.label_id].name.clone() } - - pub fn source(self, db: &dyn HirDatabase) -> InFile { - let (_body, source_map) = db.body_with_source_map(self.parent); - let src = source_map.label_syntax(self.label_id); - let root = src.file_syntax(db.upcast()); - src.map(|ast| ast.to_node(&root)) - } } #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] diff --git a/crates/ide-assists/src/utils/suggest_name.rs b/crates/ide-assists/src/utils/suggest_name.rs index 23a06404f3..f2a097afc8 100644 --- a/crates/ide-assists/src/utils/suggest_name.rs +++ b/crates/ide-assists/src/utils/suggest_name.rs @@ -254,7 +254,7 @@ fn from_param(expr: &ast::Expr, sema: &Semantics<'_, RootDatabase>) -> Option Option { diff --git a/crates/ide-db/src/rename.rs b/crates/ide-db/src/rename.rs index 288d56b534..484c65c2b0 100644 --- a/crates/ide-db/src/rename.rs +++ b/crates/ide-db/src/rename.rs @@ -24,7 +24,7 @@ use std::fmt; use base_db::{AnchoredPathBuf, FileId, FileRange}; use either::Either; -use hir::{FieldSource, HasSource, HirFileIdExt, InFile, ModuleSource, Semantics}; +use hir::{FieldSource, HirFileIdExt, InFile, ModuleSource, Semantics}; use span::SyntaxContextId; use stdx::{never, TupleExt}; use syntax::{ @@ -109,7 +109,7 @@ impl Definition { let syn_ctx_is_root = |(range, ctx): (_, SyntaxContextId)| ctx.is_root().then_some(range); let res = match self { Definition::Macro(mac) => { - let src = mac.source(sema.db)?; + let src = sema.source(mac)?; let name = match &src.value { Either::Left(it) => it.name()?, Either::Right(it) => it.name()?, @@ -119,7 +119,7 @@ impl Definition { .and_then(syn_ctx_is_root) } Definition::Field(field) => { - let src = field.source(sema.db)?; + let src = sema.source(field)?; match &src.value { FieldSource::Named(record_field) => { let name = record_field.name()?; @@ -154,18 +154,18 @@ impl Definition { } Definition::GenericParam(generic_param) => match generic_param { hir::GenericParam::LifetimeParam(lifetime_param) => { - let src = lifetime_param.source(sema.db)?; + let src = sema.source(lifetime_param)?; src.with_value(src.value.lifetime()?.syntax()) .original_file_range_opt(sema.db) .and_then(syn_ctx_is_root) } _ => { - let x = match generic_param { + let param = match generic_param { hir::GenericParam::TypeParam(it) => it.merge(), hir::GenericParam::ConstParam(it) => it.merge(), hir::GenericParam::LifetimeParam(_) => return None, }; - let src = x.source(sema.db)?; + let src = sema.source(param)?; let name = match &src.value { Either::Left(x) => x.name()?, Either::Right(_) => return None, @@ -176,14 +176,14 @@ impl Definition { } }, Definition::Label(label) => { - let src = label.source(sema.db); + let src = sema.source(label)?; let lifetime = src.value.lifetime()?; src.with_value(lifetime.syntax()) .original_file_range_opt(sema.db) .and_then(syn_ctx_is_root) } Definition::ExternCrateDecl(it) => { - let src = it.source(sema.db)?; + let src = sema.source(it)?; if let Some(rename) = src.value.rename() { let name = rename.name()?; src.with_value(name.syntax()) @@ -212,10 +212,10 @@ impl Definition { sema: &Semantics<'_, RootDatabase>, ) -> Option<(FileRange, SyntaxContextId)> where - D: HasSource, + D: hir::HasSource, D::Ast: ast::HasName, { - let src = def.source(sema.db)?; + let src = sema.source(def)?; let name = src.value.name()?; src.with_value(name.syntax()).original_file_range_opt(sema.db) } diff --git a/crates/ide-ssr/src/matching.rs b/crates/ide-ssr/src/matching.rs index cf7e7e08bc..b29053c0c2 100644 --- a/crates/ide-ssr/src/matching.rs +++ b/crates/ide-ssr/src/matching.rs @@ -575,7 +575,7 @@ impl<'db, 'sema> Matcher<'db, 'sema> { .resolve_method_call_as_callable(code) .and_then(|callable| { let (self_param, _) = callable.receiver_param(self.sema.db)?; - Some(self_param.source(self.sema.db)?.value.kind()) + Some(self.sema.source(self_param)?.value.kind()) }) .unwrap_or(ast::SelfParamKind::Owned); } diff --git a/crates/ide/src/inlay_hints/param_name.rs b/crates/ide/src/inlay_hints/param_name.rs index fb50c49a3a..9819d0e3fb 100644 --- a/crates/ide/src/inlay_hints/param_name.rs +++ b/crates/ide/src/inlay_hints/param_name.rs @@ -30,7 +30,7 @@ pub(super) fn hints( .filter_map(|(p, arg)| { // Only annotate hints for expressions that exist in the original file let range = sema.original_range_opt(arg.syntax())?; - let source = p.source(sema.db)?; + let source = sema.source(p)?; let (param_name, name_syntax) = match source.value.as_ref() { Either::Left(pat) => (pat.name()?, pat.name()), Either::Right(param) => match param.pat()? { @@ -38,8 +38,6 @@ pub(super) fn hints( _ => return None, }, }; - // make sure the file is cached so we can map out of macros - sema.parse_or_expand(source.file_id); Some((name_syntax, param_name, arg, range)) }) .filter(|(_, param_name, arg, _)| { diff --git a/crates/ide/src/navigation_target.rs b/crates/ide/src/navigation_target.rs index a93a8da57e..bfd62e7624 100644 --- a/crates/ide/src/navigation_target.rs +++ b/crates/ide/src/navigation_target.rs @@ -220,7 +220,7 @@ impl TryToNav for Definition { fn try_to_nav(&self, db: &RootDatabase) -> Option> { match self { Definition::Local(it) => Some(it.to_nav(db)), - Definition::Label(it) => Some(it.to_nav(db)), + Definition::Label(it) => it.try_to_nav(db), Definition::Module(it) => Some(it.to_nav(db)), Definition::Macro(it) => it.try_to_nav(db), Definition::Field(it) => it.try_to_nav(db), @@ -562,12 +562,12 @@ impl ToNav for hir::Local { } } -impl ToNav for hir::Label { - fn to_nav(&self, db: &RootDatabase) -> UpmappingResult { - let InFile { file_id, value } = self.source(db); +impl TryToNav for hir::Label { + fn try_to_nav(&self, db: &RootDatabase) -> Option> { + let InFile { file_id, value } = self.source(db)?; let name = self.name(db).to_smol_str(); - orig_range_with_focus(db, file_id, value.syntax(), value.lifetime()).map( + Some(orig_range_with_focus(db, file_id, value.syntax(), value.lifetime()).map( |(FileRange { file_id, range: full_range }, focus_range)| NavigationTarget { file_id, name: name.clone(), @@ -579,7 +579,7 @@ impl ToNav for hir::Label { description: None, docs: None, }, - ) + )) } } diff --git a/crates/ide/src/rename.rs b/crates/ide/src/rename.rs index a13758a9f4..3d08e2f371 100644 --- a/crates/ide/src/rename.rs +++ b/crates/ide/src/rename.rs @@ -361,8 +361,9 @@ fn rename_to_self( bail!("Parameter type differs from impl block type"); } - let InFile { file_id, value: param_source } = - first_param.source(sema.db).ok_or_else(|| format_err!("No source for parameter found"))?; + let InFile { file_id, value: param_source } = sema + .source(first_param.clone()) + .ok_or_else(|| format_err!("No source for parameter found"))?; let def = Definition::Local(local); let usages = def.usages(sema).all(); @@ -392,7 +393,7 @@ fn rename_self_to_param( let identifier_kind = IdentifierKind::classify(new_name)?; let InFile { file_id, value: self_param } = - self_param.source(sema.db).ok_or_else(|| format_err!("cannot find function source"))?; + sema.source(self_param).ok_or_else(|| format_err!("cannot find function source"))?; let def = Definition::Local(local); let usages = def.usages(sema).all(); diff --git a/crates/ide/src/signature_help.rs b/crates/ide/src/signature_help.rs index 378a38892c..89c725a6c4 100644 --- a/crates/ide/src/signature_help.rs +++ b/crates/ide/src/signature_help.rs @@ -226,7 +226,7 @@ fn signature_help_for_call( let mut buf = String::new(); for (idx, p) in callable.params().into_iter().enumerate() { buf.clear(); - if let Some(param) = p.source(sema.db) { + if let Some(param) = sema.source(p.clone()) { match param.value { Either::Right(param) => match param.pat() { Some(pat) => format_to!(buf, "{}: ", pat), From 30c04d5aa9523140f0f2daa07bc461534fb09b95 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 10 Jun 2024 12:04:35 +0200 Subject: [PATCH 2/2] Remove extra parse cache from Semantics again --- crates/hir/src/semantics.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index 964401d48d..a38cef2fd5 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -132,9 +132,6 @@ pub struct SemanticsImpl<'db> { s2d_cache: RefCell, /// Rootnode to HirFileId cache root_to_file_cache: RefCell>, - /// HirFileId to Rootnode cache (this adds a layer over the database LRU cache to prevent - /// possibly frequent invalidation) - parse_cache: RefCell>, /// MacroCall to its expansion's MacroFileId cache macro_call_cache: RefCell, MacroFileId>>, } @@ -295,7 +292,6 @@ impl<'db> SemanticsImpl<'db> { db, s2d_cache: Default::default(), root_to_file_cache: Default::default(), - parse_cache: Default::default(), macro_call_cache: Default::default(), } } @@ -307,9 +303,6 @@ impl<'db> SemanticsImpl<'db> { } pub fn parse_or_expand(&self, file_id: HirFileId) -> SyntaxNode { - if let Some(root) = self.parse_cache.borrow().get(&file_id) { - return root.clone(); - } let node = self.db.parse_or_expand(file_id); self.cache(node.clone(), file_id); node @@ -1490,9 +1483,8 @@ impl<'db> SemanticsImpl<'db> { fn cache(&self, root_node: SyntaxNode, file_id: HirFileId) { assert!(root_node.parent().is_none()); let mut cache = self.root_to_file_cache.borrow_mut(); - let prev = cache.insert(root_node.clone(), file_id); + let prev = cache.insert(root_node, file_id); assert!(prev.is_none() || prev == Some(file_id)); - self.parse_cache.borrow_mut().insert(file_id, root_node); } pub fn assert_contains_node(&self, node: &SyntaxNode) {