diff options
author | Aleksey Kladov <[email protected]> | 2019-11-20 13:03:59 +0000 |
---|---|---|
committer | Aleksey Kladov <[email protected]> | 2019-11-20 13:22:58 +0000 |
commit | cebeedc66fc40097eae20bf1767a285d00269966 (patch) | |
tree | e54c306ac0512e3610430574dc8bea39e4b50218 /crates/ra_hir | |
parent | 06fa3d8389c833b01f482bf35b0f850e627612b9 (diff) |
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
Diffstat (limited to 'crates/ra_hir')
-rw-r--r-- | crates/ra_hir/src/code_model.rs | 25 | ||||
-rw-r--r-- | crates/ra_hir/src/code_model/src.rs | 3 | ||||
-rw-r--r-- | crates/ra_hir/src/from_source.rs | 65 | ||||
-rw-r--r-- | crates/ra_hir/src/source_binder.rs | 2 |
4 files changed, 76 insertions, 19 deletions
diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index f436d5d5e..c49190a0f 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs | |||
@@ -12,7 +12,8 @@ use hir_def::{ | |||
12 | builtin_type::BuiltinType, | 12 | builtin_type::BuiltinType, |
13 | traits::TraitData, | 13 | traits::TraitData, |
14 | type_ref::{Mutability, TypeRef}, | 14 | type_ref::{Mutability, TypeRef}, |
15 | AssocItemId, CrateModuleId, ImplId, LocalEnumVariantId, LocalStructFieldId, ModuleId, UnionId, | 15 | AssocItemId, CrateModuleId, FunctionContainerId, HasModule, ImplId, LocalEnumVariantId, |
16 | LocalStructFieldId, Lookup, ModuleId, UnionId, | ||
16 | }; | 17 | }; |
17 | use hir_expand::{ | 18 | use hir_expand::{ |
18 | diagnostics::DiagnosticSink, | 19 | diagnostics::DiagnosticSink, |
@@ -647,7 +648,7 @@ impl FnData { | |||
647 | 648 | ||
648 | impl Function { | 649 | impl Function { |
649 | pub fn module(self, db: &impl DefDatabase) -> Module { | 650 | pub fn module(self, db: &impl DefDatabase) -> Module { |
650 | Module { id: self.id.module(db) } | 651 | self.id.lookup(db).module(db).into() |
651 | } | 652 | } |
652 | 653 | ||
653 | pub fn krate(self, db: &impl DefDatabase) -> Option<Crate> { | 654 | pub fn krate(self, db: &impl DefDatabase) -> Option<Crate> { |
@@ -680,21 +681,25 @@ impl Function { | |||
680 | 681 | ||
681 | /// The containing impl block, if this is a method. | 682 | /// The containing impl block, if this is a method. |
682 | pub fn impl_block(self, db: &impl DefDatabase) -> Option<ImplBlock> { | 683 | pub fn impl_block(self, db: &impl DefDatabase) -> Option<ImplBlock> { |
683 | ImplBlock::containing(db, self.into()) | 684 | match self.container(db) { |
685 | Some(Container::ImplBlock(it)) => Some(it), | ||
686 | _ => None, | ||
687 | } | ||
684 | } | 688 | } |
685 | 689 | ||
686 | /// The containing trait, if this is a trait method definition. | 690 | /// The containing trait, if this is a trait method definition. |
687 | pub fn parent_trait(self, db: &impl DefDatabase) -> Option<Trait> { | 691 | pub fn parent_trait(self, db: &impl DefDatabase) -> Option<Trait> { |
688 | db.trait_items_index(self.module(db).id).get_parent_trait(self.id.into()).map(Trait::from) | 692 | match self.container(db) { |
693 | Some(Container::Trait(it)) => Some(it), | ||
694 | _ => None, | ||
695 | } | ||
689 | } | 696 | } |
690 | 697 | ||
691 | pub fn container(self, db: &impl DefDatabase) -> Option<Container> { | 698 | pub fn container(self, db: &impl DefDatabase) -> Option<Container> { |
692 | if let Some(impl_block) = self.impl_block(db) { | 699 | match self.id.lookup(db).container { |
693 | Some(impl_block.into()) | 700 | FunctionContainerId::TraitId(it) => Some(Container::Trait(it.into())), |
694 | } else if let Some(trait_) = self.parent_trait(db) { | 701 | FunctionContainerId::ImplId(it) => Some(Container::ImplBlock(it.into())), |
695 | Some(trait_.into()) | 702 | FunctionContainerId::ModuleId(_) => None, |
696 | } else { | ||
697 | None | ||
698 | } | 703 | } |
699 | } | 704 | } |
700 | 705 | ||
diff --git a/crates/ra_hir/src/code_model/src.rs b/crates/ra_hir/src/code_model/src.rs index 556417b0f..91cab7414 100644 --- a/crates/ra_hir/src/code_model/src.rs +++ b/crates/ra_hir/src/code_model/src.rs | |||
@@ -1,5 +1,6 @@ | |||
1 | //! FIXME: write short doc here | 1 | //! FIXME: write short doc here |
2 | 2 | ||
3 | use hir_def::{HasSource as _, Lookup}; | ||
3 | use ra_syntax::ast::{self, AstNode}; | 4 | use ra_syntax::ast::{self, AstNode}; |
4 | 5 | ||
5 | use crate::{ | 6 | use crate::{ |
@@ -113,7 +114,7 @@ impl HasSource for EnumVariant { | |||
113 | impl HasSource for Function { | 114 | impl HasSource for Function { |
114 | type Ast = ast::FnDef; | 115 | type Ast = ast::FnDef; |
115 | fn source(self, db: &(impl DefDatabase + AstDatabase)) -> Source<ast::FnDef> { | 116 | fn source(self, db: &(impl DefDatabase + AstDatabase)) -> Source<ast::FnDef> { |
116 | self.id.source(db) | 117 | self.id.lookup(db).source(db) |
117 | } | 118 | } |
118 | } | 119 | } |
119 | impl HasSource for Const { | 120 | impl HasSource for Const { |
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}; | |||
4 | use hir_expand::{name::AsName, AstId, MacroDefId, MacroDefKind}; | 4 | use hir_expand::{name::AsName, AstId, MacroDefId, MacroDefKind}; |
5 | use ra_syntax::{ | 5 | use ra_syntax::{ |
6 | ast::{self, AstNode, NameOwner}, | 6 | ast::{self, AstNode, NameOwner}, |
7 | match_ast, | 7 | match_ast, AstPtr, |
8 | }; | 8 | }; |
9 | 9 | ||
10 | use crate::{ | 10 | use crate::{ |
11 | db::{AstDatabase, DefDatabase, HirDatabase}, | 11 | db::{AstDatabase, DefDatabase, HirDatabase}, |
12 | ids::{AstItemDef, LocationCtx}, | 12 | ids::{AstItemDef, LocationCtx}, |
13 | Const, DefWithBody, Enum, EnumVariant, FieldSource, Function, HasBody, HasSource, ImplBlock, | 13 | AssocItem, Const, DefWithBody, Enum, EnumVariant, FieldSource, Function, HasBody, HasSource, |
14 | Local, MacroDef, Module, ModuleSource, Source, Static, Struct, StructField, Trait, TypeAlias, | 14 | ImplBlock, Local, MacroDef, Module, ModuleDef, ModuleSource, Source, Static, Struct, |
15 | Union, VariantDef, | 15 | StructField, Trait, TypeAlias, Union, VariantDef, |
16 | }; | 16 | }; |
17 | 17 | ||
18 | pub trait FromSource: Sized { | 18 | pub trait FromSource: Sized { |
@@ -52,10 +52,51 @@ impl FromSource for Trait { | |||
52 | impl FromSource for Function { | 52 | impl FromSource for Function { |
53 | type Ast = ast::FnDef; | 53 | type Ast = ast::FnDef; |
54 | fn from_source(db: &(impl DefDatabase + AstDatabase), src: Source<Self::Ast>) -> Option<Self> { | 54 | fn from_source(db: &(impl DefDatabase + AstDatabase), src: Source<Self::Ast>) -> Option<Self> { |
55 | let id = from_source(db, src)?; | 55 | // FIXME: this doesn't try to handle nested declarations |
56 | Some(Function { id }) | 56 | for container in src.value.syntax().ancestors() { |
57 | let res = match_ast! { | ||
58 | match container { | ||
59 | ast::TraitDef(it) => { | ||
60 | let c = Trait::from_source(db, src.with_value(it))?; | ||
61 | c.items(db) | ||
62 | .into_iter() | ||
63 | .filter_map(|it| match it { | ||
64 | AssocItem::Function(it) => Some(it), | ||
65 | _ => None | ||
66 | }) | ||
67 | .find(|it| same_source(&it.source(db), &src))? | ||
68 | }, | ||
69 | ast::ImplBlock(it) => { | ||
70 | let c = ImplBlock::from_source(db, src.with_value(it))?; | ||
71 | c.items(db) | ||
72 | .into_iter() | ||
73 | .filter_map(|it| match it { | ||
74 | AssocItem::Function(it) => Some(it), | ||
75 | _ => None | ||
76 | }) | ||
77 | .find(|it| same_source(&it.source(db), &src))? | ||
78 | |||
79 | }, | ||
80 | _ => { continue }, | ||
81 | } | ||
82 | }; | ||
83 | return Some(res); | ||
84 | } | ||
85 | |||
86 | let module_source = ModuleSource::from_child_node(db, src.as_ref().map(|it| it.syntax())); | ||
87 | let c = Module::from_definition(db, src.with_value(module_source))?; | ||
88 | let res = c | ||
89 | .declarations(db) | ||
90 | .into_iter() | ||
91 | .filter_map(|it| match it { | ||
92 | ModuleDef::Function(it) => Some(it), | ||
93 | _ => None, | ||
94 | }) | ||
95 | .find(|it| same_source(&it.source(db), &src)); | ||
96 | res | ||
57 | } | 97 | } |
58 | } | 98 | } |
99 | |||
59 | impl FromSource for Const { | 100 | impl FromSource for Const { |
60 | type Ast = ast::ConstDef; | 101 | type Ast = ast::ConstDef; |
61 | fn from_source(db: &(impl DefDatabase + AstDatabase), src: Source<Self::Ast>) -> Option<Self> { | 102 | fn from_source(db: &(impl DefDatabase + AstDatabase), src: Source<Self::Ast>) -> Option<Self> { |
@@ -108,7 +149,7 @@ impl FromSource for EnumVariant { | |||
108 | let parent_enum = src.value.parent_enum(); | 149 | let parent_enum = src.value.parent_enum(); |
109 | let src_enum = Source { file_id: src.file_id, value: parent_enum }; | 150 | let src_enum = Source { file_id: src.file_id, value: parent_enum }; |
110 | let variants = Enum::from_source(db, src_enum)?.variants(db); | 151 | let variants = Enum::from_source(db, src_enum)?.variants(db); |
111 | variants.into_iter().find(|v| v.source(db) == src) | 152 | variants.into_iter().find(|v| same_source(&v.source(db), &src)) |
112 | } | 153 | } |
113 | } | 154 | } |
114 | 155 | ||
@@ -216,3 +257,13 @@ where | |||
216 | let ctx = LocationCtx::new(db, module.id, src.file_id); | 257 | let ctx = LocationCtx::new(db, module.id, src.file_id); |
217 | Some(DEF::from_ast(ctx, &src.value)) | 258 | Some(DEF::from_ast(ctx, &src.value)) |
218 | } | 259 | } |
260 | |||
261 | /// XXX: AST Nodes and SyntaxNodes have identity equality semantics: nodes are | ||
262 | /// equal if they point to exactly the same object. | ||
263 | /// | ||
264 | /// In general, we do not guarantee that we have exactly one instance of a | ||
265 | /// syntax tree for each file. We probably should add such guanratree, but, for | ||
266 | /// the time being, we will use identity-less AstPtr comparison. | ||
267 | fn same_source<N: AstNode>(s1: &Source<N>, s2: &Source<N>) -> bool { | ||
268 | s1.as_ref().map(AstPtr::new) == s2.as_ref().map(AstPtr::new) | ||
269 | } | ||
diff --git a/crates/ra_hir/src/source_binder.rs b/crates/ra_hir/src/source_binder.rs index a1c1daacd..5d9e22ee2 100644 --- a/crates/ra_hir/src/source_binder.rs +++ b/crates/ra_hir/src/source_binder.rs | |||
@@ -70,7 +70,7 @@ fn def_with_body_from_child_node( | |||
70 | child.value.ancestors().find_map(|node| { | 70 | child.value.ancestors().find_map(|node| { |
71 | match_ast! { | 71 | match_ast! { |
72 | match node { | 72 | match node { |
73 | ast::FnDef(def) => { Some(Function {id: ctx.to_def(&def) }.into()) }, | 73 | ast::FnDef(def) => { return Function::from_source(db, child.with_value(def)).map(DefWithBody::from); }, |
74 | ast::ConstDef(def) => { Some(Const { id: ctx.to_def(&def) }.into()) }, | 74 | ast::ConstDef(def) => { Some(Const { id: ctx.to_def(&def) }.into()) }, |
75 | ast::StaticDef(def) => { Some(Static { id: ctx.to_def(&def) }.into()) }, | 75 | ast::StaticDef(def) => { Some(Static { id: ctx.to_def(&def) }.into()) }, |
76 | _ => { None }, | 76 | _ => { None }, |