From 36e3fc9d5413f7e6e17e82867aae1318645880a3 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 20 Nov 2019 09:40:36 +0300 Subject: Rename Source::ast -> Source::value --- crates/ra_hir/src/from_source.rs | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) (limited to 'crates/ra_hir/src/from_source.rs') diff --git a/crates/ra_hir/src/from_source.rs b/crates/ra_hir/src/from_source.rs index 1c26756c9..f4dca25cb 100644 --- a/crates/ra_hir/src/from_source.rs +++ b/crates/ra_hir/src/from_source.rs @@ -87,7 +87,7 @@ impl FromSource for MacroDef { let module = Module::from_definition(db, Source::new(src.file_id, module_src))?; let krate = module.krate().crate_id(); - let ast_id = AstId::new(src.file_id, db.ast_id_map(src.file_id).ast_id(&src.ast)); + let ast_id = AstId::new(src.file_id, db.ast_id_map(src.file_id).ast_id(&src.value)); let id: MacroDefId = MacroDefId { krate, ast_id, kind }; Some(MacroDef { id }) @@ -105,8 +105,8 @@ impl FromSource for ImplBlock { impl FromSource for EnumVariant { type Ast = ast::EnumVariant; fn from_source(db: &(impl DefDatabase + AstDatabase), src: Source) -> Option { - let parent_enum = src.ast.parent_enum(); - let src_enum = Source { file_id: src.file_id, ast: parent_enum }; + let parent_enum = src.value.parent_enum(); + let src_enum = Source { file_id: src.file_id, value: parent_enum }; let variants = Enum::from_source(db, src_enum)?.variants(db); variants.into_iter().find(|v| v.source(db) == src) } @@ -115,16 +115,16 @@ impl FromSource for EnumVariant { impl FromSource for StructField { type Ast = FieldSource; fn from_source(db: &(impl DefDatabase + AstDatabase), src: Source) -> Option { - let variant_def: VariantDef = match src.ast { + let variant_def: VariantDef = match src.value { FieldSource::Named(ref field) => { - let ast = field.syntax().ancestors().find_map(ast::StructDef::cast)?; - let src = Source { file_id: src.file_id, ast }; + let value = field.syntax().ancestors().find_map(ast::StructDef::cast)?; + let src = Source { file_id: src.file_id, value }; let def = Struct::from_source(db, src)?; VariantDef::from(def) } FieldSource::Pos(ref field) => { - let ast = field.syntax().ancestors().find_map(ast::EnumVariant::cast)?; - let src = Source { file_id: src.file_id, ast }; + let value = field.syntax().ancestors().find_map(ast::EnumVariant::cast)?; + let src = Source { file_id: src.file_id, value }; let def = EnumVariant::from_source(db, src)?; VariantDef::from(def) } @@ -142,12 +142,12 @@ impl FromSource for StructField { impl Local { pub fn from_source(db: &impl HirDatabase, src: Source) -> Option { let file_id = src.file_id; - let parent: DefWithBody = src.ast.syntax().ancestors().find_map(|it| { + let parent: DefWithBody = src.value.syntax().ancestors().find_map(|it| { let res = match_ast! { match it { - ast::ConstDef(ast) => { Const::from_source(db, Source { ast, file_id})?.into() }, - ast::StaticDef(ast) => { Static::from_source(db, Source { ast, file_id})?.into() }, - ast::FnDef(ast) => { Function::from_source(db, Source { ast, file_id})?.into() }, + ast::ConstDef(value) => { Const::from_source(db, Source { value, file_id})?.into() }, + ast::StaticDef(value) => { Static::from_source(db, Source { value, file_id})?.into() }, + ast::FnDef(value) => { Function::from_source(db, Source { value, file_id})?.into() }, _ => return None, } }; @@ -162,33 +162,33 @@ impl Local { impl Module { pub fn from_declaration(db: &impl DefDatabase, src: Source) -> Option { - let parent_declaration = src.ast.syntax().ancestors().skip(1).find_map(ast::Module::cast); + let parent_declaration = src.value.syntax().ancestors().skip(1).find_map(ast::Module::cast); let parent_module = match parent_declaration { Some(parent_declaration) => { - let src_parent = Source { file_id: src.file_id, ast: parent_declaration }; + let src_parent = Source { file_id: src.file_id, value: parent_declaration }; Module::from_declaration(db, src_parent) } _ => { let src_parent = Source { file_id: src.file_id, - ast: ModuleSource::new(db, Some(src.file_id.original_file(db)), None), + value: ModuleSource::new(db, Some(src.file_id.original_file(db)), None), }; Module::from_definition(db, src_parent) } }?; - let child_name = src.ast.name()?; + let child_name = src.value.name()?; parent_module.child(db, &child_name.as_name()) } pub fn from_definition(db: &impl DefDatabase, src: Source) -> Option { - match src.ast { + match src.value { ModuleSource::Module(ref module) => { assert!(!module.has_semi()); return Module::from_declaration( db, - Source { file_id: src.file_id, ast: module.clone() }, + Source { file_id: src.file_id, value: module.clone() }, ); } ModuleSource::SourceFile(_) => (), @@ -214,5 +214,5 @@ where let module_src = ModuleSource::from_child_node(db, src.as_ref().map(|it| it.syntax())); let module = Module::from_definition(db, Source::new(src.file_id, module_src))?; let ctx = LocationCtx::new(db, module.id, src.file_id); - Some(DEF::from_ast(ctx, &src.ast)) + Some(DEF::from_ast(ctx, &src.value)) } -- cgit v1.2.3 From cebeedc66fc40097eae20bf1767a285d00269966 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 20 Nov 2019 16:03:59 +0300 Subject: Next gen IDs for functions The current system with AstIds has two primaraly drawbacks: * It is possible to manufacture IDs out of thin air. For example, it's possible to create IDs for items which are not considered in CrateDefMap due to cfg. Or it is possible to mixup structs and unions, because they share ID space. * Getting the ID of a parent requires a secondary index. Instead, the plan is to pursue the more traditional approach, where each items stores the id of the parent declaration. This makes `FromSource` more awkward, but also more correct: now, to get from an AST to HIR, we first do this recursively for the parent item, and the just search the children of the parent for the matching def --- crates/ra_hir/src/from_source.rs | 65 +++++++++++++++++++++++++++++++++++----- 1 file changed, 58 insertions(+), 7 deletions(-) (limited to 'crates/ra_hir/src/from_source.rs') diff --git a/crates/ra_hir/src/from_source.rs b/crates/ra_hir/src/from_source.rs index f4dca25cb..303d5f138 100644 --- a/crates/ra_hir/src/from_source.rs +++ b/crates/ra_hir/src/from_source.rs @@ -4,15 +4,15 @@ use hir_def::{ModuleId, StructId, StructOrUnionId, UnionId}; use hir_expand::{name::AsName, AstId, MacroDefId, MacroDefKind}; use ra_syntax::{ ast::{self, AstNode, NameOwner}, - match_ast, + match_ast, AstPtr, }; use crate::{ db::{AstDatabase, DefDatabase, HirDatabase}, ids::{AstItemDef, LocationCtx}, - Const, DefWithBody, Enum, EnumVariant, FieldSource, Function, HasBody, HasSource, ImplBlock, - Local, MacroDef, Module, ModuleSource, Source, Static, Struct, StructField, Trait, TypeAlias, - Union, VariantDef, + AssocItem, Const, DefWithBody, Enum, EnumVariant, FieldSource, Function, HasBody, HasSource, + ImplBlock, Local, MacroDef, Module, ModuleDef, ModuleSource, Source, Static, Struct, + StructField, Trait, TypeAlias, Union, VariantDef, }; pub trait FromSource: Sized { @@ -52,10 +52,51 @@ impl FromSource for Trait { impl FromSource for Function { type Ast = ast::FnDef; fn from_source(db: &(impl DefDatabase + AstDatabase), src: Source) -> Option { - let id = from_source(db, src)?; - Some(Function { id }) + // FIXME: this doesn't try to handle nested declarations + for container in src.value.syntax().ancestors() { + let res = match_ast! { + match container { + ast::TraitDef(it) => { + let c = Trait::from_source(db, src.with_value(it))?; + c.items(db) + .into_iter() + .filter_map(|it| match it { + AssocItem::Function(it) => Some(it), + _ => None + }) + .find(|it| same_source(&it.source(db), &src))? + }, + ast::ImplBlock(it) => { + let c = ImplBlock::from_source(db, src.with_value(it))?; + c.items(db) + .into_iter() + .filter_map(|it| match it { + AssocItem::Function(it) => Some(it), + _ => None + }) + .find(|it| same_source(&it.source(db), &src))? + + }, + _ => { continue }, + } + }; + return Some(res); + } + + let module_source = ModuleSource::from_child_node(db, src.as_ref().map(|it| it.syntax())); + let c = Module::from_definition(db, src.with_value(module_source))?; + let res = c + .declarations(db) + .into_iter() + .filter_map(|it| match it { + ModuleDef::Function(it) => Some(it), + _ => None, + }) + .find(|it| same_source(&it.source(db), &src)); + res } } + impl FromSource for Const { type Ast = ast::ConstDef; fn from_source(db: &(impl DefDatabase + AstDatabase), src: Source) -> Option { @@ -108,7 +149,7 @@ impl FromSource for EnumVariant { let parent_enum = src.value.parent_enum(); let src_enum = Source { file_id: src.file_id, value: parent_enum }; let variants = Enum::from_source(db, src_enum)?.variants(db); - variants.into_iter().find(|v| v.source(db) == src) + variants.into_iter().find(|v| same_source(&v.source(db), &src)) } } @@ -216,3 +257,13 @@ where let ctx = LocationCtx::new(db, module.id, src.file_id); Some(DEF::from_ast(ctx, &src.value)) } + +/// XXX: AST Nodes and SyntaxNodes have identity equality semantics: nodes are +/// equal if they point to exactly the same object. +/// +/// In general, we do not guarantee that we have exactly one instance of a +/// syntax tree for each file. We probably should add such guanratree, but, for +/// the time being, we will use identity-less AstPtr comparison. +fn same_source(s1: &Source, s2: &Source) -> bool { + s1.as_ref().map(AstPtr::new) == s2.as_ref().map(AstPtr::new) +} -- cgit v1.2.3 From 64c21ed19594b323e72605ba8c5dd4c6eee433f6 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 20 Nov 2019 17:39:58 +0300 Subject: Switch type aliases to new sources --- crates/ra_hir/src/from_source.rs | 112 +++++++++++++++++++++++++-------------- 1 file changed, 71 insertions(+), 41 deletions(-) (limited to 'crates/ra_hir/src/from_source.rs') diff --git a/crates/ra_hir/src/from_source.rs b/crates/ra_hir/src/from_source.rs index 303d5f138..f5fdaafa3 100644 --- a/crates/ra_hir/src/from_source.rs +++ b/crates/ra_hir/src/from_source.rs @@ -4,7 +4,7 @@ use hir_def::{ModuleId, StructId, StructOrUnionId, UnionId}; use hir_expand::{name::AsName, AstId, MacroDefId, MacroDefKind}; use ra_syntax::{ ast::{self, AstNode, NameOwner}, - match_ast, AstPtr, + match_ast, AstPtr, SyntaxNode, }; use crate::{ @@ -52,48 +52,27 @@ impl FromSource for Trait { impl FromSource for Function { type Ast = ast::FnDef; fn from_source(db: &(impl DefDatabase + AstDatabase), src: Source) -> Option { - // FIXME: this doesn't try to handle nested declarations - for container in src.value.syntax().ancestors() { - let res = match_ast! { - match container { - ast::TraitDef(it) => { - let c = Trait::from_source(db, src.with_value(it))?; - c.items(db) - .into_iter() - .filter_map(|it| match it { - AssocItem::Function(it) => Some(it), - _ => None - }) - .find(|it| same_source(&it.source(db), &src))? - }, - ast::ImplBlock(it) => { - let c = ImplBlock::from_source(db, src.with_value(it))?; - c.items(db) - .into_iter() - .filter_map(|it| match it { - AssocItem::Function(it) => Some(it), - _ => None - }) - .find(|it| same_source(&it.source(db), &src))? - - }, - _ => { continue }, - } - }; - return Some(res); - } - - let module_source = ModuleSource::from_child_node(db, src.as_ref().map(|it| it.syntax())); - let c = Module::from_definition(db, src.with_value(module_source))?; - let res = c - .declarations(db) + let items = match Container::find(db, src.as_ref().map(|it| it.syntax()))? { + Container::Trait(it) => it.items(db), + Container::ImplBlock(it) => it.items(db), + Container::Module(m) => { + return m + .declarations(db) + .into_iter() + .filter_map(|it| match it { + ModuleDef::Function(it) => Some(it), + _ => None, + }) + .find(|it| same_source(&it.source(db), &src)) + } + }; + items .into_iter() .filter_map(|it| match it { - ModuleDef::Function(it) => Some(it), + AssocItem::Function(it) => Some(it), _ => None, }) - .find(|it| same_source(&it.source(db), &src)); - res + .find(|it| same_source(&it.source(db), &src)) } } @@ -114,8 +93,27 @@ impl FromSource for Static { impl FromSource for TypeAlias { type Ast = ast::TypeAliasDef; fn from_source(db: &(impl DefDatabase + AstDatabase), src: Source) -> Option { - let id = from_source(db, src)?; - Some(TypeAlias { id }) + let items = match Container::find(db, src.as_ref().map(|it| it.syntax()))? { + Container::Trait(it) => it.items(db), + Container::ImplBlock(it) => it.items(db), + Container::Module(m) => { + return m + .declarations(db) + .into_iter() + .filter_map(|it| match it { + ModuleDef::TypeAlias(it) => Some(it), + _ => None, + }) + .find(|it| same_source(&it.source(db), &src)) + } + }; + items + .into_iter() + .filter_map(|it| match it { + AssocItem::TypeAlias(it) => Some(it), + _ => None, + }) + .find(|it| same_source(&it.source(db), &src)) } } @@ -258,6 +256,38 @@ where Some(DEF::from_ast(ctx, &src.value)) } +enum Container { + Trait(Trait), + ImplBlock(ImplBlock), + Module(Module), +} + +impl Container { + fn find(db: &impl DefDatabase, src: Source<&SyntaxNode>) -> Option { + // FIXME: this doesn't try to handle nested declarations + for container in src.value.ancestors() { + let res = match_ast! { + match container { + ast::TraitDef(it) => { + let c = Trait::from_source(db, src.with_value(it))?; + Container::Trait(c) + }, + ast::ImplBlock(it) => { + let c = ImplBlock::from_source(db, src.with_value(it))?; + Container::ImplBlock(c) + }, + _ => { continue }, + } + }; + return Some(res); + } + + let module_source = ModuleSource::from_child_node(db, src); + let c = Module::from_definition(db, src.with_value(module_source))?; + Some(Container::Module(c)) + } +} + /// XXX: AST Nodes and SyntaxNodes have identity equality semantics: nodes are /// equal if they point to exactly the same object. /// -- cgit v1.2.3 From 111891dc2dc1d2c7ea87144e8e3ddefe23fc7b6d Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 20 Nov 2019 18:00:01 +0300 Subject: Move constants to new ID This allows us to get rid of trait item index --- crates/ra_hir/src/from_source.rs | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) (limited to 'crates/ra_hir/src/from_source.rs') diff --git a/crates/ra_hir/src/from_source.rs b/crates/ra_hir/src/from_source.rs index f5fdaafa3..b86307c58 100644 --- a/crates/ra_hir/src/from_source.rs +++ b/crates/ra_hir/src/from_source.rs @@ -79,8 +79,27 @@ impl FromSource for Function { impl FromSource for Const { type Ast = ast::ConstDef; fn from_source(db: &(impl DefDatabase + AstDatabase), src: Source) -> Option { - let id = from_source(db, src)?; - Some(Const { id }) + let items = match Container::find(db, src.as_ref().map(|it| it.syntax()))? { + Container::Trait(it) => it.items(db), + Container::ImplBlock(it) => it.items(db), + Container::Module(m) => { + return m + .declarations(db) + .into_iter() + .filter_map(|it| match it { + ModuleDef::Const(it) => Some(it), + _ => None, + }) + .find(|it| same_source(&it.source(db), &src)) + } + }; + items + .into_iter() + .filter_map(|it| match it { + AssocItem::Const(it) => Some(it), + _ => None, + }) + .find(|it| same_source(&it.source(db), &src)) } } impl FromSource for Static { @@ -292,7 +311,7 @@ impl Container { /// equal if they point to exactly the same object. /// /// In general, we do not guarantee that we have exactly one instance of a -/// syntax tree for each file. We probably should add such guanratree, but, for +/// syntax tree for each file. We probably should add such guarantee, but, for /// the time being, we will use identity-less AstPtr comparison. fn same_source(s1: &Source, s2: &Source) -> bool { s1.as_ref().map(AstPtr::new) == s2.as_ref().map(AstPtr::new) -- cgit v1.2.3