Handle macro-generated expressions slightly less wrong

This commit is contained in:
Aleksey Kladov 2019-11-14 10:30:30 +03:00
parent a73b7bb3f6
commit da2ca01eba
6 changed files with 62 additions and 48 deletions

View file

@ -166,6 +166,7 @@ fn compute_expr_scopes(expr: ExprId, body: &Body, scopes: &mut ExprScopes, scope
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use hir_expand::Source;
use ra_db::{fixture::WithFixture, SourceDatabase}; use ra_db::{fixture::WithFixture, SourceDatabase};
use ra_syntax::{algo::find_node_at_offset, ast, AstNode}; use ra_syntax::{algo::find_node_at_offset, ast, AstNode};
use test_utils::{assert_eq_text, extract_offset}; 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 analyzer = SourceAnalyzer::new(&db, file_id, marker.syntax(), None);
let scopes = analyzer.scopes(); 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 scope = scopes.scope_for(expr_id);
let actual = scopes let actual = scopes

View file

@ -145,7 +145,8 @@ impl Local {
Some(res) Some(res)
})?; })?;
let (_body, source_map) = db.body_with_source_map(parent); 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 }) Some(Local { parent, pat_id })
} }
} }

View file

@ -7,8 +7,11 @@
//! purely for "IDE needs". //! purely for "IDE needs".
use std::sync::Arc; use std::sync::Arc;
use hir_def::path::known; use hir_def::{
use hir_expand::name::AsName; expr::{ExprId, PatId},
path::known,
};
use hir_expand::{name::AsName, Source};
use ra_db::FileId; use ra_db::FileId;
use ra_syntax::{ use ra_syntax::{
ast::{self, AstNode}, 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. /// original source files. It should not be used inside the HIR itself.
#[derive(Debug)] #[derive(Debug)]
pub struct SourceAnalyzer { pub struct SourceAnalyzer {
// FIXME: this doesn't handle macros at all
file_id: FileId,
resolver: Resolver, resolver: Resolver,
body_owner: Option<DefWithBody>, body_owner: Option<DefWithBody>,
body_source_map: Option<Arc<BodySourceMap>>, body_source_map: Option<Arc<BodySourceMap>>,
@ -147,7 +152,7 @@ impl SourceAnalyzer {
let source_map = def.body_source_map(db); let source_map = def.body_source_map(db);
let scopes = db.expr_scopes(def); let scopes = db.expr_scopes(def);
let scope = match offset { 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), Some(offset) => scope_for_offset(&scopes, &source_map, file_id.into(), offset),
}; };
let resolver = expr::resolver_for_scope(db, def, scope); let resolver = expr::resolver_for_scope(db, def, scope);
@ -157,6 +162,7 @@ impl SourceAnalyzer {
body_source_map: Some(source_map), body_source_map: Some(source_map),
infer: Some(def.infer(db)), infer: Some(def.infer(db)),
scopes: Some(scopes), scopes: Some(scopes),
file_id,
} }
} else { } else {
SourceAnalyzer { SourceAnalyzer {
@ -168,17 +174,28 @@ impl SourceAnalyzer {
body_source_map: None, body_source_map: None,
infer: None, infer: None,
scopes: None, scopes: None,
file_id,
} }
} }
} }
fn expr_id(&self, expr: &ast::Expr) -> Option<ExprId> {
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<PatId> {
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<crate::Ty> { pub fn type_of(&self, _db: &impl HirDatabase, expr: &ast::Expr) -> Option<crate::Ty> {
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()) Some(self.infer.as_ref()?[expr_id].clone())
} }
pub fn type_of_pat(&self, _db: &impl HirDatabase, pat: &ast::Pat) -> Option<crate::Ty> { pub fn type_of_pat(&self, _db: &impl HirDatabase, pat: &ast::Pat) -> Option<crate::Ty> {
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()) Some(self.infer.as_ref()?[pat_id].clone())
} }
@ -191,22 +208,22 @@ impl SourceAnalyzer {
} }
pub fn resolve_method_call(&self, call: &ast::MethodCallExpr) -> Option<Function> { pub fn resolve_method_call(&self, call: &ast::MethodCallExpr) -> Option<Function> {
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) self.infer.as_ref()?.method_resolution(expr_id)
} }
pub fn resolve_field(&self, field: &ast::FieldExpr) -> Option<crate::StructField> { pub fn resolve_field(&self, field: &ast::FieldExpr) -> Option<crate::StructField> {
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) self.infer.as_ref()?.field_resolution(expr_id)
} }
pub fn resolve_record_literal(&self, record_lit: &ast::RecordLit) -> Option<crate::VariantDef> { pub fn resolve_record_literal(&self, record_lit: &ast::RecordLit) -> Option<crate::VariantDef> {
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) self.infer.as_ref()?.variant_resolution_for_expr(expr_id)
} }
pub fn resolve_record_pattern(&self, record_pat: &ast::RecordPat) -> Option<crate::VariantDef> { pub fn resolve_record_pattern(&self, record_pat: &ast::RecordPat) -> Option<crate::VariantDef> {
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) 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<PathResolution> { pub fn resolve_path(&self, db: &impl HirDatabase, path: &ast::Path) -> Option<PathResolution> {
if let Some(path_expr) = path.syntax().parent().and_then(ast::PathExpr::cast) { 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) { if let Some(assoc) = self.infer.as_ref()?.assoc_resolutions_for_expr(expr_id) {
return Some(PathResolution::AssocItem(assoc)); return Some(PathResolution::AssocItem(assoc));
} }
} }
if let Some(path_pat) = path.syntax().parent().and_then(ast::PathPat::cast) { 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) { if let Some(assoc) = self.infer.as_ref()?.assoc_resolutions_for_pat(pat_id) {
return Some(PathResolution::AssocItem(assoc)); return Some(PathResolution::AssocItem(assoc));
} }
@ -285,7 +302,7 @@ impl SourceAnalyzer {
let name = name_ref.as_name(); let name = name_ref.as_name();
let source_map = self.body_source_map.as_ref()?; let source_map = self.body_source_map.as_ref()?;
let scopes = self.scopes.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 let ret = scopes
.scope_chain(scope) .scope_chain(scope)
.flat_map(|scope| scopes.entries(scope).iter()) .flat_map(|scope| scopes.entries(scope).iter())
@ -418,11 +435,12 @@ impl SourceAnalyzer {
fn scope_for( fn scope_for(
scopes: &ExprScopes, scopes: &ExprScopes,
source_map: &BodySourceMap, source_map: &BodySourceMap,
file_id: HirFileId,
node: &SyntaxNode, node: &SyntaxNode,
) -> Option<ScopeId> { ) -> Option<ScopeId> {
node.ancestors() node.ancestors()
.filter_map(ast::Expr::cast) .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)) .find_map(|it| scopes.scope_for(it))
} }

View file

@ -21,7 +21,6 @@ use crate::{
pub struct Expander { pub struct Expander {
crate_def_map: Arc<CrateDefMap>, crate_def_map: Arc<CrateDefMap>,
original_file_id: HirFileId,
current_file_id: HirFileId, current_file_id: HirFileId,
hygiene: Hygiene, hygiene: Hygiene,
module: ModuleId, module: ModuleId,
@ -31,13 +30,7 @@ impl Expander {
pub fn new(db: &impl DefDatabase2, current_file_id: HirFileId, module: ModuleId) -> 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 crate_def_map = db.crate_def_map(module.krate);
let hygiene = Hygiene::new(db, current_file_id); let hygiene = Hygiene::new(db, current_file_id);
Expander { Expander { crate_def_map, current_file_id, hygiene, module }
crate_def_map,
original_file_id: current_file_id,
current_file_id,
hygiene,
module,
}
} }
fn expand( fn expand(
@ -82,11 +75,6 @@ impl Expander {
std::mem::forget(mark); std::mem::forget(mark);
} }
// FIXME: remove this.
fn is_in_expansion(&self) -> bool {
self.original_file_id != self.current_file_id
}
fn to_source<T>(&self, ast: T) -> Source<T> { fn to_source<T>(&self, ast: T) -> Source<T> {
Source { file_id: self.current_file_id, ast } Source { file_id: self.current_file_id, ast }
} }
@ -147,9 +135,9 @@ pub type PatSource = Source<PatPtr>;
/// this properly for macros. /// this properly for macros.
#[derive(Default, Debug, Eq, PartialEq)] #[derive(Default, Debug, Eq, PartialEq)]
pub struct BodySourceMap { pub struct BodySourceMap {
expr_map: FxHashMap<ExprPtr, ExprId>, expr_map: FxHashMap<ExprSource, ExprId>,
expr_map_back: ArenaMap<ExprId, ExprSource>, expr_map_back: ArenaMap<ExprId, ExprSource>,
pat_map: FxHashMap<PatPtr, PatId>, pat_map: FxHashMap<PatSource, PatId>,
pat_map_back: ArenaMap<PatId, PatSource>, pat_map_back: ArenaMap<PatId, PatSource>,
field_map: FxHashMap<(ExprId, usize), AstPtr<ast::RecordField>>, field_map: FxHashMap<(ExprId, usize), AstPtr<ast::RecordField>>,
} }
@ -202,16 +190,18 @@ impl BodySourceMap {
self.expr_map_back.get(expr).copied() self.expr_map_back.get(expr).copied()
} }
pub fn node_expr(&self, node: &ast::Expr) -> Option<ExprId> { pub fn node_expr(&self, node: Source<&ast::Expr>) -> Option<ExprId> {
self.expr_map.get(&Either::A(AstPtr::new(node))).cloned() let src = node.map(|it| Either::A(AstPtr::new(it)));
self.expr_map.get(&src).cloned()
} }
pub fn pat_syntax(&self, pat: PatId) -> Option<PatSource> { pub fn pat_syntax(&self, pat: PatId) -> Option<PatSource> {
self.pat_map_back.get(pat).copied() self.pat_map_back.get(pat).copied()
} }
pub fn node_pat(&self, node: &ast::Pat) -> Option<PatId> { pub fn node_pat(&self, node: Source<&ast::Pat>) -> Option<PatId> {
self.pat_map.get(&Either::A(AstPtr::new(node))).cloned() 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<ast::RecordField> { pub fn field_syntax(&self, expr: ExprId, field: usize) -> AstPtr<ast::RecordField> {

View file

@ -94,10 +94,9 @@ where
fn alloc_expr(&mut self, expr: Expr, ptr: AstPtr<ast::Expr>) -> ExprId { fn alloc_expr(&mut self, expr: Expr, ptr: AstPtr<ast::Expr>) -> ExprId {
let ptr = Either::A(ptr); let ptr = Either::A(ptr);
let id = self.body.exprs.alloc(expr); let id = self.body.exprs.alloc(expr);
if !self.expander.is_in_expansion() { let src = self.expander.to_source(ptr);
self.source_map.expr_map.insert(ptr, id); self.source_map.expr_map.insert(src, id);
} self.source_map.expr_map_back.insert(id, src);
self.source_map.expr_map_back.insert(id, self.expander.to_source(ptr));
id id
} }
// desugared exprs don't have ptr, that's wrong and should be fixed // 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<ast::RecordField>) -> ExprId { fn alloc_expr_field_shorthand(&mut self, expr: Expr, ptr: AstPtr<ast::RecordField>) -> ExprId {
let ptr = Either::B(ptr); let ptr = Either::B(ptr);
let id = self.body.exprs.alloc(expr); let id = self.body.exprs.alloc(expr);
if !self.expander.is_in_expansion() { let src = self.expander.to_source(ptr);
self.source_map.expr_map.insert(ptr, id); self.source_map.expr_map.insert(src, id);
} self.source_map.expr_map_back.insert(id, src);
self.source_map.expr_map_back.insert(id, self.expander.to_source(ptr));
id id
} }
fn alloc_pat(&mut self, pat: Pat, ptr: PatPtr) -> PatId { fn alloc_pat(&mut self, pat: Pat, ptr: PatPtr) -> PatId {
let id = self.body.pats.alloc(pat); let id = self.body.pats.alloc(pat);
if !self.expander.is_in_expansion() { let src = self.expander.to_source(ptr);
self.source_map.pat_map.insert(ptr, id); self.source_map.pat_map.insert(src, id);
} self.source_map.pat_map_back.insert(id, src);
self.source_map.pat_map_back.insert(id, self.expander.to_source(ptr));
id id
} }
@ -277,7 +274,8 @@ where
ast::Expr::ParenExpr(e) => { ast::Expr::ParenExpr(e) => {
let inner = self.collect_expr_opt(e.expr()); let inner = self.collect_expr_opt(e.expr());
// make the paren expr point to the inner expression as well // 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 inner
} }
ast::Expr::ReturnExpr(e) => { ast::Expr::ReturnExpr(e) => {

View file

@ -223,7 +223,7 @@ impl<N: AstNode> AstId<N> {
} }
} }
#[derive(Debug, PartialEq, Eq, Clone, Copy)] #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)]
pub struct Source<T> { pub struct Source<T> {
pub file_id: HirFileId, pub file_id: HirFileId,
pub ast: T, pub ast: T,
@ -233,6 +233,9 @@ impl<T> Source<T> {
pub fn map<F: FnOnce(T) -> U, U>(self, f: F) -> Source<U> { pub fn map<F: FnOnce(T) -> U, U>(self, f: F) -> Source<U> {
Source { file_id: self.file_id, ast: f(self.ast) } 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 { pub fn file_syntax(&self, db: &impl db::AstDatabase) -> SyntaxNode {
db.parse_or_expand(self.file_id).expect("source created from invalid file") db.parse_or_expand(self.file_id).expect("source created from invalid file")
} }