Auto merge of #17380 - Veykril:sema-source, r=Veykril

fix: Remove extra parse cache from Semantics again

Should fix https://github.com/rust-lang/rust-analyzer/issues/17376, specifically 30c04d5aa9

The recent changes here were heavily triggering what I realized in https://github.com/rust-lang/rust-analyzer/issues/17367#issuecomment-2156374726

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,
+    db::HirDatabase, Adt, Callee, Const, Enum, ExternCrateDecl, Field, FieldSource, Function, Impl,
-    LifetimeParam, LocalSource, Macro, Module, Static, Struct, Trait, TraitAlias, TypeAlias,
+    Label, LifetimeParam, LocalSource, Macro, Module, Param, SelfParam, Static, Struct, Trait,
-    TypeOrConstParam, Union, Variant,
+    TraitAlias, TypeAlias, TypeOrConstParam, Union, Variant,
 };
 
 pub trait HasSource {
@@ -222,6 +223,68 @@ impl HasSource for LocalSource {
     }
 }
 
+impl HasSource for Param {
+    type Ast = Either<ast::SelfParam, ast::Param>;
+
+    fn source(self, db: &dyn HirDatabase) -> Option<InFile<Self::Ast>> {
+        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<InFile<Self::Ast>> {
+        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<InFile<Self::Ast>> {
+        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;
 

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<Self>
+    where
+        Self: Sized,
+    {
+        if ast::RecordField::can_cast(syntax.kind()) {
+            <ast::RecordField as AstNode>::cast(syntax).map(FieldSource::Named)
+        } else if ast::TupleField::can_cast(syntax.kind()) {
+            <ast::TupleField as AstNode>::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<ast::Pat> {
+    pub fn pattern_source(self, db: &dyn HirDatabase) -> Option<ast::Pat> {
         self.source(db).and_then(|p| p.value.right()?.pat())
     }
-
-    pub fn source(
-        &self,
-        db: &dyn HirDatabase,
-    ) -> Option<InFile<Either<ast::SelfParam, ast::Param>>> {
-        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<InFile<ast::SelfParam>> {
-        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<ast::Label> {
-        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)]

crates/hir/src/semantics.rs

@@ -132,9 +132,6 @@ pub struct SemanticsImpl<'db> {
     s2d_cache: RefCell<SourceToDefCache>,
     /// Rootnode to HirFileId cache
     root_to_file_cache: RefCell<FxHashMap<SyntaxNode, HirFileId>>,
-    /// HirFileId to Rootnode cache (this adds a layer over the database LRU cache to prevent
-    /// possibly frequent invalidation)
-    parse_cache: RefCell<FxHashMap<HirFileId, SyntaxNode>>,
     /// MacroCall to its expansion's MacroFileId cache
     macro_call_cache: RefCell<FxHashMap<InFile<ast::MacroCall>, 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) {

crates/ide-assists/src/utils/suggest_name.rs

@@ -254,7 +254,7 @@ fn from_param(expr: &ast::Expr, sema: &Semantics<'_, RootDatabase>) -> Option<St
 
     let (idx, _) = arg_list.args().find_position(|it| it == expr).unwrap();
     let param = func.params().into_iter().nth(idx)?;
-    let pat = param.source(sema.db)?.value.right()?.pat()?;
+    let pat = sema.source(param)?.value.right()?.pat()?;
     let name = var_name_from_pat(&pat)?;
     normalize(&name.to_string())
 }

crates/ide-db/src/active_parameter.rs

@@ -28,7 +28,7 @@ impl ActiveParameter {
             return None;
         }
         let param = params.swap_remove(idx);
-        Some(ActiveParameter { ty: param.ty().clone(), src: param.source(sema.db) })
+        Some(ActiveParameter { ty: param.ty().clone(), src: sema.source(param) })
     }
 
     pub fn ident(&self) -> Option<ast::Name> {

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)
         }

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);
                     }

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, _)| {

crates/ide/src/navigation_target.rs

@@ -220,7 +220,7 @@ impl TryToNav for Definition {
     fn try_to_nav(&self, db: &RootDatabase) -> Option<UpmappingResult<NavigationTarget>> {
         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 {
+impl TryToNav for hir::Label {
-    fn to_nav(&self, db: &RootDatabase) -> UpmappingResult<NavigationTarget> {
+    fn try_to_nav(&self, db: &RootDatabase) -> Option<UpmappingResult<NavigationTarget>> {
-        let InFile { file_id, value } = self.source(db);
+        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,
             },
-        )
+        ))
     }
 }
 

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 } =
+    let InFile { file_id, value: param_source } = sema
-        first_param.source(sema.db).ok_or_else(|| format_err!("No source for parameter found"))?;
+        .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();

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),