Merge #8026

8026: Simplify source maps for fields r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>

5 files changed, 33 insertions, 45 deletions

diff --git a/crates/hir/src/source_analyzer.rs b/crates/hir/src/source_analyzer.rs
index 055a3e5d0..4d59293e9 100644
--- a/crates/hir/src/source_analyzer.rs
+++ b/crates/hir/src/source_analyzer.rs
@@ -24,7 +24,7 @@ use hir_ty::{
 };
 use syntax::{
     ast::{self, AstNode},
-    AstPtr, SyntaxNode, TextRange, TextSize,
+    SyntaxNode, TextRange, TextSize,
 };
 
 use crate::{
@@ -161,26 +161,8 @@ impl SourceAnalyzer {
         db: &dyn HirDatabase,
         field: &ast::RecordExprField,
     ) -> Option<(Field, Option<Local>)> {
-        let expr_id = {
-            let record_lit = field.parent_record_lit();
-            let record_lit_expr = self.expr_id(db, &ast::Expr::from(record_lit))?;
-            let body = self.body.as_ref()?;
-            let body_source_map = self.body_source_map.as_ref()?;
-            match &body[record_lit_expr] {
-                hir_def::expr::Expr::RecordLit { fields, .. } => {
-                    let field_ptr = InFile::new(self.file_id, AstPtr::new(field));
-                    fields.iter().enumerate().find_map(|(i, f)| {
-                        let ptr = body_source_map.field_syntax(record_lit_expr, i);
-                        if ptr == field_ptr {
-                            Some(f.expr)
-                        } else {
-                            None
-                        }
-                    })?
-                }
-                _ => return None,
-            }
-        };
+        let expr_id =
+            self.body_source_map.as_ref()?.node_field(InFile::new(self.file_id, field))?;
 
         let local = if field.name_ref().is_some() {
             None
diff --git a/crates/hir_def/src/body.rs b/crates/hir_def/src/body.rs
index 19c4eb521..8bcc350ce 100644
--- a/crates/hir_def/src/body.rs
+++ b/crates/hir_def/src/body.rs
@@ -253,11 +253,18 @@ pub type LabelSource = InFile<LabelPtr>;
 pub struct BodySourceMap {
     expr_map: FxHashMap<ExprSource, ExprId>,
     expr_map_back: ArenaMap<ExprId, Result<ExprSource, SyntheticSyntax>>,
+
     pat_map: FxHashMap<PatSource, PatId>,
     pat_map_back: ArenaMap<PatId, Result<PatSource, SyntheticSyntax>>,
+
     label_map: FxHashMap<LabelSource, LabelId>,
     label_map_back: ArenaMap<LabelId, LabelSource>,
-    field_map: FxHashMap<(ExprId, usize), InFile<AstPtr<ast::RecordExprField>>>,
+
+    /// We don't create explicit nodes for record fields (`S { record_field: 92 }`).
+    /// Instead, we use id of expression (`92`) to identify the field.
+    field_map: FxHashMap<InFile<AstPtr<ast::RecordExprField>>, ExprId>,
+    field_map_back: FxHashMap<ExprId, InFile<AstPtr<ast::RecordExprField>>>,
+
     expansions: FxHashMap<InFile<AstPtr<ast::MacroCall>>, HirFileId>,
 
     /// Diagnostics accumulated during body lowering. These contain `AstPtr`s and so are stored in
@@ -337,6 +344,8 @@ impl Index<LabelId> for Body {
     }
 }
 
+// FIXME: Change `node_` prefix to something more reasonable.
+// Perhaps `expr_syntax` and `expr_id`?
 impl BodySourceMap {
     pub fn expr_syntax(&self, expr: ExprId) -> Result<ExprSource, SyntheticSyntax> {
         self.expr_map_back[expr].clone()
@@ -375,8 +384,12 @@ impl BodySourceMap {
         self.label_map.get(&src).cloned()
     }
 
-    pub fn field_syntax(&self, expr: ExprId, field: usize) -> InFile<AstPtr<ast::RecordExprField>> {
-        self.field_map[&(expr, field)].clone()
+    pub fn field_syntax(&self, expr: ExprId) -> InFile<AstPtr<ast::RecordExprField>> {
+        self.field_map_back[&expr].clone()
+    }
+    pub fn node_field(&self, node: InFile<&ast::RecordExprField>) -> Option<ExprId> {
+        let src = node.map(|it| AstPtr::new(it));
+        self.field_map.get(&src).cloned()
     }
 
     pub(crate) fn add_diagnostics(&self, _db: &dyn DefDatabase, sink: &mut DiagnosticSink<'_>) {
diff --git a/crates/hir_def/src/body/lower.rs b/crates/hir_def/src/body/lower.rs
index 8c8eb8007..8934ae6c9 100644
--- a/crates/hir_def/src/body/lower.rs
+++ b/crates/hir_def/src/body/lower.rs
@@ -379,23 +379,22 @@ impl ExprCollector<'_> {
             }
             ast::Expr::RecordExpr(e) => {
                 let path = e.path().and_then(|path| self.expander.parse_path(path));
-                let mut field_ptrs = Vec::new();
                 let record_lit = if let Some(nfl) = e.record_expr_field_list() {
                     let fields = nfl
                         .fields()
-                        .inspect(|field| field_ptrs.push(AstPtr::new(field)))
                         .filter_map(|field| {
                             self.check_cfg(&field)?;
 
                             let name = field.field_name()?.as_name();
 
-                            Some(RecordLitField {
-                                name,
-                                expr: match field.expr() {
-                                    Some(e) => self.collect_expr(e),
-                                    None => self.missing_expr(),
-                                },
-                            })
+                            let expr = match field.expr() {
+                                Some(e) => self.collect_expr(e),
+                                None => self.missing_expr(),
+                            };
+                            let src = self.expander.to_source(AstPtr::new(&field));
+                            self.source_map.field_map.insert(src.clone(), expr);
+                            self.source_map.field_map_back.insert(expr, src);
+                            Some(RecordLitField { name, expr })
                         })
                         .collect();
                     let spread = nfl.spread().map(|s| self.collect_expr(s));
@@ -404,12 +403,7 @@ impl ExprCollector<'_> {
                     Expr::RecordLit { path, fields: Vec::new(), spread: None }
                 };
 
-                let res = self.alloc_expr(record_lit, syntax_ptr);
-                for (i, ptr) in field_ptrs.into_iter().enumerate() {
-                    let src = self.expander.to_source(ptr);
-                    self.source_map.field_map.insert((res, i), src);
-                }
-                res
+                self.alloc_expr(record_lit, syntax_ptr)
             }
             ast::Expr::FieldExpr(e) => {
                 let expr = self.collect_expr_opt(e.expr());
diff --git a/crates/hir_ty/src/infer.rs b/crates/hir_ty/src/infer.rs
index fbfedb4e6..9f29098ee 100644
--- a/crates/hir_ty/src/infer.rs
+++ b/crates/hir_ty/src/infer.rs
@@ -784,7 +784,7 @@ mod diagnostics {
 
     #[derive(Debug, PartialEq, Eq, Clone)]
     pub(super) enum InferenceDiagnostic {
-        NoSuchField { expr: ExprId, field: usize },
+        NoSuchField { expr: ExprId },
         BreakOutsideOfLoop { expr: ExprId },
     }
 
@@ -796,9 +796,9 @@ mod diagnostics {
             sink: &mut DiagnosticSink,
         ) {
             match self {
-                InferenceDiagnostic::NoSuchField { expr, field } => {
+                InferenceDiagnostic::NoSuchField { expr } => {
                     let (_, source_map) = db.body_with_source_map(owner);
-                    let field = source_map.field_syntax(*expr, *field);
+                    let field = source_map.field_syntax(*expr);
                     sink.push(NoSuchField { file: field.file_id, field: field.value })
                 }
                 InferenceDiagnostic::BreakOutsideOfLoop { expr } => {
diff --git a/crates/hir_ty/src/infer/expr.rs b/crates/hir_ty/src/infer/expr.rs
index 55163c963..1a11b146a 100644
--- a/crates/hir_ty/src/infer/expr.rs
+++ b/crates/hir_ty/src/infer/expr.rs
@@ -405,14 +405,13 @@ impl<'a> InferenceContext<'a> {
                 let substs = ty.substs().cloned().unwrap_or_else(Substs::empty);
                 let field_types = def_id.map(|it| self.db.field_types(it)).unwrap_or_default();
                 let variant_data = def_id.map(|it| variant_data(self.db.upcast(), it));
-                for (field_idx, field) in fields.iter().enumerate() {
+                for field in fields.iter() {
                     let field_def =
                         variant_data.as_ref().and_then(|it| match it.field(&field.name) {
                             Some(local_id) => Some(FieldId { parent: def_id.unwrap(), local_id }),
                             None => {
                                 self.push_diagnostic(InferenceDiagnostic::NoSuchField {
-                                    expr: tgt_expr,
-                                    field: field_idx,
+                                    expr: field.expr,
                                 });
                                 None
                             }