From da2ca01ebaaaaa47aa09c5848c53b145a68af8fa Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 14 Nov 2019 10:30:30 +0300 Subject: Handle macro-generated expressions slightly less wrong --- crates/ra_hir/src/expr/scope.rs | 6 ++++- crates/ra_hir/src/from_source.rs | 3 ++- crates/ra_hir/src/source_binder.rs | 44 ++++++++++++++++++++++++++----------- crates/ra_hir_def/src/body.rs | 28 ++++++++--------------- crates/ra_hir_def/src/body/lower.rs | 24 ++++++++++---------- crates/ra_hir_expand/src/lib.rs | 5 ++++- 6 files changed, 62 insertions(+), 48 deletions(-) diff --git a/crates/ra_hir/src/expr/scope.rs b/crates/ra_hir/src/expr/scope.rs index fe5e836f2..afba66069 100644 --- a/crates/ra_hir/src/expr/scope.rs +++ b/crates/ra_hir/src/expr/scope.rs @@ -166,6 +166,7 @@ fn compute_expr_scopes(expr: ExprId, body: &Body, scopes: &mut ExprScopes, scope #[cfg(test)] mod tests { + use hir_expand::Source; use ra_db::{fixture::WithFixture, SourceDatabase}; use ra_syntax::{algo::find_node_at_offset, ast, AstNode}; use test_utils::{assert_eq_text, extract_offset}; @@ -189,7 +190,10 @@ mod tests { let analyzer = SourceAnalyzer::new(&db, file_id, marker.syntax(), None); let scopes = analyzer.scopes(); - let expr_id = analyzer.body_source_map().node_expr(&marker.into()).unwrap(); + let expr_id = analyzer + .body_source_map() + .node_expr(Source { file_id: file_id.into(), ast: &marker.into() }) + .unwrap(); let scope = scopes.scope_for(expr_id); let actual = scopes diff --git a/crates/ra_hir/src/from_source.rs b/crates/ra_hir/src/from_source.rs index 2c441b0f4..4b561c63d 100644 --- a/crates/ra_hir/src/from_source.rs +++ b/crates/ra_hir/src/from_source.rs @@ -145,7 +145,8 @@ impl Local { Some(res) })?; let (_body, source_map) = db.body_with_source_map(parent); - let pat_id = source_map.node_pat(&src.ast.into())?; + let src = src.map(ast::Pat::from); + let pat_id = source_map.node_pat(src.as_ref())?; Some(Local { parent, pat_id }) } } diff --git a/crates/ra_hir/src/source_binder.rs b/crates/ra_hir/src/source_binder.rs index f28e9c931..88eed1137 100644 --- a/crates/ra_hir/src/source_binder.rs +++ b/crates/ra_hir/src/source_binder.rs @@ -7,8 +7,11 @@ //! purely for "IDE needs". use std::sync::Arc; -use hir_def::path::known; -use hir_expand::name::AsName; +use hir_def::{ + expr::{ExprId, PatId}, + path::known, +}; +use hir_expand::{name::AsName, Source}; use ra_db::FileId; use ra_syntax::{ ast::{self, AstNode}, @@ -93,6 +96,8 @@ fn def_with_body_from_child_node( /// original source files. It should not be used inside the HIR itself. #[derive(Debug)] pub struct SourceAnalyzer { + // FIXME: this doesn't handle macros at all + file_id: FileId, resolver: Resolver, body_owner: Option, body_source_map: Option>, @@ -147,7 +152,7 @@ impl SourceAnalyzer { let source_map = def.body_source_map(db); let scopes = db.expr_scopes(def); let scope = match offset { - None => scope_for(&scopes, &source_map, &node), + None => scope_for(&scopes, &source_map, file_id.into(), &node), Some(offset) => scope_for_offset(&scopes, &source_map, file_id.into(), offset), }; let resolver = expr::resolver_for_scope(db, def, scope); @@ -157,6 +162,7 @@ impl SourceAnalyzer { body_source_map: Some(source_map), infer: Some(def.infer(db)), scopes: Some(scopes), + file_id, } } else { SourceAnalyzer { @@ -168,17 +174,28 @@ impl SourceAnalyzer { body_source_map: None, infer: None, scopes: None, + file_id, } } } + fn expr_id(&self, expr: &ast::Expr) -> Option { + let src = Source { file_id: self.file_id.into(), ast: expr }; + self.body_source_map.as_ref()?.node_expr(src) + } + + fn pat_id(&self, pat: &ast::Pat) -> Option { + let src = Source { file_id: self.file_id.into(), ast: pat }; + self.body_source_map.as_ref()?.node_pat(src) + } + pub fn type_of(&self, _db: &impl HirDatabase, expr: &ast::Expr) -> Option { - let expr_id = self.body_source_map.as_ref()?.node_expr(expr)?; + let expr_id = self.expr_id(expr)?; Some(self.infer.as_ref()?[expr_id].clone()) } pub fn type_of_pat(&self, _db: &impl HirDatabase, pat: &ast::Pat) -> Option { - let pat_id = self.body_source_map.as_ref()?.node_pat(pat)?; + let pat_id = self.pat_id(pat)?; Some(self.infer.as_ref()?[pat_id].clone()) } @@ -191,22 +208,22 @@ impl SourceAnalyzer { } pub fn resolve_method_call(&self, call: &ast::MethodCallExpr) -> Option { - let expr_id = self.body_source_map.as_ref()?.node_expr(&call.clone().into())?; + let expr_id = self.expr_id(&call.clone().into())?; self.infer.as_ref()?.method_resolution(expr_id) } pub fn resolve_field(&self, field: &ast::FieldExpr) -> Option { - let expr_id = self.body_source_map.as_ref()?.node_expr(&field.clone().into())?; + let expr_id = self.expr_id(&field.clone().into())?; self.infer.as_ref()?.field_resolution(expr_id) } pub fn resolve_record_literal(&self, record_lit: &ast::RecordLit) -> Option { - let expr_id = self.body_source_map.as_ref()?.node_expr(&record_lit.clone().into())?; + let expr_id = self.expr_id(&record_lit.clone().into())?; self.infer.as_ref()?.variant_resolution_for_expr(expr_id) } pub fn resolve_record_pattern(&self, record_pat: &ast::RecordPat) -> Option { - let pat_id = self.body_source_map.as_ref()?.node_pat(&record_pat.clone().into())?; + let pat_id = self.pat_id(&record_pat.clone().into())?; self.infer.as_ref()?.variant_resolution_for_pat(pat_id) } @@ -264,13 +281,13 @@ impl SourceAnalyzer { pub fn resolve_path(&self, db: &impl HirDatabase, path: &ast::Path) -> Option { if let Some(path_expr) = path.syntax().parent().and_then(ast::PathExpr::cast) { - let expr_id = self.body_source_map.as_ref()?.node_expr(&path_expr.into())?; + let expr_id = self.expr_id(&path_expr.into())?; if let Some(assoc) = self.infer.as_ref()?.assoc_resolutions_for_expr(expr_id) { return Some(PathResolution::AssocItem(assoc)); } } if let Some(path_pat) = path.syntax().parent().and_then(ast::PathPat::cast) { - let pat_id = self.body_source_map.as_ref()?.node_pat(&path_pat.into())?; + let pat_id = self.pat_id(&path_pat.into())?; if let Some(assoc) = self.infer.as_ref()?.assoc_resolutions_for_pat(pat_id) { return Some(PathResolution::AssocItem(assoc)); } @@ -285,7 +302,7 @@ impl SourceAnalyzer { let name = name_ref.as_name(); let source_map = self.body_source_map.as_ref()?; let scopes = self.scopes.as_ref()?; - let scope = scope_for(scopes, source_map, name_ref.syntax()); + let scope = scope_for(scopes, source_map, self.file_id.into(), name_ref.syntax()); let ret = scopes .scope_chain(scope) .flat_map(|scope| scopes.entries(scope).iter()) @@ -418,11 +435,12 @@ impl SourceAnalyzer { fn scope_for( scopes: &ExprScopes, source_map: &BodySourceMap, + file_id: HirFileId, node: &SyntaxNode, ) -> Option { node.ancestors() .filter_map(ast::Expr::cast) - .filter_map(|it| source_map.node_expr(&it)) + .filter_map(|it| source_map.node_expr(Source { file_id, ast: &it })) .find_map(|it| scopes.scope_for(it)) } diff --git a/crates/ra_hir_def/src/body.rs b/crates/ra_hir_def/src/body.rs index 65fefd912..75bba31c2 100644 --- a/crates/ra_hir_def/src/body.rs +++ b/crates/ra_hir_def/src/body.rs @@ -21,7 +21,6 @@ use crate::{ pub struct Expander { crate_def_map: Arc, - original_file_id: HirFileId, current_file_id: HirFileId, hygiene: Hygiene, module: ModuleId, @@ -31,13 +30,7 @@ impl Expander { pub fn new(db: &impl DefDatabase2, current_file_id: HirFileId, module: ModuleId) -> Expander { let crate_def_map = db.crate_def_map(module.krate); let hygiene = Hygiene::new(db, current_file_id); - Expander { - crate_def_map, - original_file_id: current_file_id, - current_file_id, - hygiene, - module, - } + Expander { crate_def_map, current_file_id, hygiene, module } } fn expand( @@ -82,11 +75,6 @@ impl Expander { std::mem::forget(mark); } - // FIXME: remove this. - fn is_in_expansion(&self) -> bool { - self.original_file_id != self.current_file_id - } - fn to_source(&self, ast: T) -> Source { Source { file_id: self.current_file_id, ast } } @@ -147,9 +135,9 @@ pub type PatSource = Source; /// this properly for macros. #[derive(Default, Debug, Eq, PartialEq)] pub struct BodySourceMap { - expr_map: FxHashMap, + expr_map: FxHashMap, expr_map_back: ArenaMap, - pat_map: FxHashMap, + pat_map: FxHashMap, pat_map_back: ArenaMap, field_map: FxHashMap<(ExprId, usize), AstPtr>, } @@ -202,16 +190,18 @@ impl BodySourceMap { self.expr_map_back.get(expr).copied() } - pub fn node_expr(&self, node: &ast::Expr) -> Option { - self.expr_map.get(&Either::A(AstPtr::new(node))).cloned() + pub fn node_expr(&self, node: Source<&ast::Expr>) -> Option { + let src = node.map(|it| Either::A(AstPtr::new(it))); + self.expr_map.get(&src).cloned() } pub fn pat_syntax(&self, pat: PatId) -> Option { self.pat_map_back.get(pat).copied() } - pub fn node_pat(&self, node: &ast::Pat) -> Option { - self.pat_map.get(&Either::A(AstPtr::new(node))).cloned() + pub fn node_pat(&self, node: Source<&ast::Pat>) -> Option { + let src = node.map(|it| Either::A(AstPtr::new(it))); + self.pat_map.get(&src).cloned() } pub fn field_syntax(&self, expr: ExprId, field: usize) -> AstPtr { diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index c45500195..f6d79ddf0 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -94,10 +94,9 @@ where fn alloc_expr(&mut self, expr: Expr, ptr: AstPtr) -> ExprId { let ptr = Either::A(ptr); let id = self.body.exprs.alloc(expr); - if !self.expander.is_in_expansion() { - self.source_map.expr_map.insert(ptr, id); - } - self.source_map.expr_map_back.insert(id, self.expander.to_source(ptr)); + let src = self.expander.to_source(ptr); + self.source_map.expr_map.insert(src, id); + self.source_map.expr_map_back.insert(id, src); id } // desugared exprs don't have ptr, that's wrong and should be fixed @@ -108,18 +107,16 @@ where fn alloc_expr_field_shorthand(&mut self, expr: Expr, ptr: AstPtr) -> ExprId { let ptr = Either::B(ptr); let id = self.body.exprs.alloc(expr); - if !self.expander.is_in_expansion() { - self.source_map.expr_map.insert(ptr, id); - } - self.source_map.expr_map_back.insert(id, self.expander.to_source(ptr)); + let src = self.expander.to_source(ptr); + self.source_map.expr_map.insert(src, id); + self.source_map.expr_map_back.insert(id, src); id } fn alloc_pat(&mut self, pat: Pat, ptr: PatPtr) -> PatId { let id = self.body.pats.alloc(pat); - if !self.expander.is_in_expansion() { - self.source_map.pat_map.insert(ptr, id); - } - self.source_map.pat_map_back.insert(id, self.expander.to_source(ptr)); + let src = self.expander.to_source(ptr); + self.source_map.pat_map.insert(src, id); + self.source_map.pat_map_back.insert(id, src); id } @@ -277,7 +274,8 @@ where ast::Expr::ParenExpr(e) => { let inner = self.collect_expr_opt(e.expr()); // make the paren expr point to the inner expression as well - self.source_map.expr_map.insert(Either::A(syntax_ptr), inner); + let src = self.expander.to_source(Either::A(syntax_ptr)); + self.source_map.expr_map.insert(src, inner); inner } ast::Expr::ReturnExpr(e) => { diff --git a/crates/ra_hir_expand/src/lib.rs b/crates/ra_hir_expand/src/lib.rs index c6ffa2c6f..930789b0f 100644 --- a/crates/ra_hir_expand/src/lib.rs +++ b/crates/ra_hir_expand/src/lib.rs @@ -223,7 +223,7 @@ impl AstId { } } -#[derive(Debug, PartialEq, Eq, Clone, Copy)] +#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] pub struct Source { pub file_id: HirFileId, pub ast: T, @@ -233,6 +233,9 @@ impl Source { pub fn map U, U>(self, f: F) -> Source { Source { file_id: self.file_id, ast: f(self.ast) } } + pub fn as_ref(&self) -> Source<&T> { + Source { file_id: self.file_id, ast: &self.ast } + } pub fn file_syntax(&self, db: &impl db::AstDatabase) -> SyntaxNode { db.parse_or_expand(self.file_id).expect("source created from invalid file") } -- cgit v1.2.3