From 8886d707b84e3623211b82841f7539b570479d4f Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 4 Jul 2023 09:16:15 +0200 Subject: [PATCH] Stronger typing for AstId and AstIdMap --- crates/hir-def/src/data.rs | 2 +- crates/hir-def/src/data/adt.rs | 10 +-- crates/hir-def/src/item_tree.rs | 4 +- crates/hir-def/src/lib.rs | 6 +- crates/hir-def/src/lower.rs | 8 ++- crates/hir-def/src/nameres/collector.rs | 2 +- crates/hir-def/src/nameres/diagnostics.rs | 11 ++-- crates/hir-expand/src/ast_id_map.rs | 79 +++++++++++++++++------ crates/hir-expand/src/lib.rs | 28 +++++--- crates/hir/src/lib.rs | 2 +- 10 files changed, 102 insertions(+), 50 deletions(-) diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs index 4a9f08dca6d..54fe9a2e844 100644 --- a/crates/hir-def/src/data.rs +++ b/crates/hir-def/src/data.rs @@ -599,7 +599,7 @@ fn collect(&mut self, item_tree: &ItemTree, tree_id: TreeId, assoc_items: &[Asso if !attrs.is_cfg_enabled(self.expander.cfg_options()) { self.diagnostics.push(DefDiagnostic::unconfigured_code( self.module_id.local_id, - InFile::new(self.expander.current_file_id(), item.ast_id(item_tree).upcast()), + InFile::new(self.expander.current_file_id(), item.ast_id(item_tree).erase()), attrs.cfg().unwrap(), self.expander.cfg_options().clone(), )); diff --git a/crates/hir-def/src/data/adt.rs b/crates/hir-def/src/data/adt.rs index 612a5ca1a28..595b9b770b7 100644 --- a/crates/hir-def/src/data/adt.rs +++ b/crates/hir-def/src/data/adt.rs @@ -330,7 +330,7 @@ pub(crate) fn enum_data_with_diagnostics_query( } else { diagnostics.push(DefDiagnostic::unconfigured_code( loc.container.local_id, - InFile::new(loc.id.file_id(), var.ast_id.upcast()), + InFile::new(loc.id.file_id(), var.ast_id.erase()), attrs.cfg().unwrap(), cfg_options.clone(), )) @@ -540,8 +540,8 @@ fn lower_fields( InFile::new( current_file_id, match field.ast_id { - FieldAstId::Record(it) => it.upcast(), - FieldAstId::Tuple(it) => it.upcast(), + FieldAstId::Record(it) => it.erase(), + FieldAstId::Tuple(it) => it.erase(), }, ), attrs.cfg().unwrap(), @@ -564,8 +564,8 @@ fn lower_fields( InFile::new( current_file_id, match field.ast_id { - FieldAstId::Record(it) => it.upcast(), - FieldAstId::Tuple(it) => it.upcast(), + FieldAstId::Record(it) => it.erase(), + FieldAstId::Tuple(it) => it.erase(), }, ), attrs.cfg().unwrap(), diff --git a/crates/hir-def/src/item_tree.rs b/crates/hir-def/src/item_tree.rs index e74b71888c2..6f80bb6e07c 100644 --- a/crates/hir-def/src/item_tree.rs +++ b/crates/hir-def/src/item_tree.rs @@ -46,7 +46,7 @@ use base_db::CrateId; use either::Either; use hir_expand::{ - ast_id_map::FileAstId, + ast_id_map::{AstIdNode, FileAstId}, attrs::RawAttrs, hygiene::Hygiene, name::{name, AsName, Name}, @@ -314,7 +314,7 @@ fn from(t: $t) -> AttrOwner { /// Trait implemented by all item nodes in the item tree. pub trait ItemTreeNode: Clone { - type Source: AstNode + Into; + type Source: AstIdNode + Into; fn ast_id(&self) -> FileAstId; diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs index e5149ab577c..c0701e2576a 100644 --- a/crates/hir-def/src/lib.rs +++ b/crates/hir-def/src/lib.rs @@ -64,7 +64,7 @@ macro_rules! eprintln { use base_db::{impl_intern_key, salsa, CrateId, ProcMacroKind}; use hir_expand::{ - ast_id_map::FileAstId, + ast_id_map::{AstIdNode, FileAstId}, attrs::{Attr, AttrId, AttrInput}, builtin_attr_macro::BuiltinAttrExpander, builtin_derive_macro::BuiltinDeriveExpander, @@ -1124,12 +1124,12 @@ fn as_call_id_with_errors( /// Helper wrapper for `AstId` with `ModPath` #[derive(Clone, Debug, Eq, PartialEq)] -struct AstIdWithPath { +struct AstIdWithPath { ast_id: AstId, path: path::ModPath, } -impl AstIdWithPath { +impl AstIdWithPath { fn new(file_id: HirFileId, ast_id: FileAstId, path: path::ModPath) -> AstIdWithPath { AstIdWithPath { ast_id: AstId::new(file_id, ast_id), path } } diff --git a/crates/hir-def/src/lower.rs b/crates/hir-def/src/lower.rs index af623fd0e5d..e523c229179 100644 --- a/crates/hir-def/src/lower.rs +++ b/crates/hir-def/src/lower.rs @@ -1,5 +1,9 @@ //! Context for lowering paths. -use hir_expand::{ast_id_map::AstIdMap, hygiene::Hygiene, AstId, HirFileId, InFile}; +use hir_expand::{ + ast_id_map::{AstIdMap, AstIdNode}, + hygiene::Hygiene, + AstId, HirFileId, InFile, +}; use once_cell::unsync::OnceCell; use syntax::ast; use triomphe::Arc; @@ -37,7 +41,7 @@ pub(crate) fn lower_path(&self, ast: ast::Path) -> Option { Path::from_src(ast, self) } - pub(crate) fn ast_id(&self, item: &N) -> Option> { + pub(crate) fn ast_id(&self, item: &N) -> Option> { let &(file_id, ref ast_id_map) = self.ast_id_map.as_ref()?; let ast_id_map = ast_id_map.get_or_init(|| self.db.ast_id_map(file_id)); Some(InFile::new(file_id, ast_id_map.ast_id(item))) diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index a5cdee9628d..c048716d740 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -2280,7 +2280,7 @@ fn is_cfg_enabled(&self, cfg: &CfgExpr) -> bool { fn emit_unconfigured_diagnostic(&mut self, item: ModItem, cfg: &CfgExpr) { let ast_id = item.ast_id(self.item_tree); - let ast_id = InFile::new(self.file_id(), ast_id.upcast()); + let ast_id = InFile::new(self.file_id(), ast_id.erase()); self.def_collector.def_map.diagnostics.push(DefDiagnostic::unconfigured_code( self.module_id, ast_id, diff --git a/crates/hir-def/src/nameres/diagnostics.rs b/crates/hir-def/src/nameres/diagnostics.rs index 18b424255cd..e82e97b628e 100644 --- a/crates/hir-def/src/nameres/diagnostics.rs +++ b/crates/hir-def/src/nameres/diagnostics.rs @@ -2,12 +2,9 @@ use base_db::CrateId; use cfg::{CfgExpr, CfgOptions}; -use hir_expand::{attrs::AttrId, MacroCallKind}; +use hir_expand::{attrs::AttrId, ErasedAstId, MacroCallKind}; use la_arena::Idx; -use syntax::{ - ast::{self, AnyHasAttrs}, - SyntaxError, -}; +use syntax::{ast, SyntaxError}; use crate::{ item_tree::{self, ItemTreeId}, @@ -24,7 +21,7 @@ pub enum DefDiagnosticKind { UnresolvedImport { id: ItemTreeId, index: Idx }, - UnconfiguredCode { ast: AstId, cfg: CfgExpr, opts: CfgOptions }, + UnconfiguredCode { ast: ErasedAstId, cfg: CfgExpr, opts: CfgOptions }, UnresolvedProcMacro { ast: MacroCallKind, krate: CrateId }, @@ -81,7 +78,7 @@ pub(super) fn unresolved_import( pub fn unconfigured_code( container: LocalModuleId, - ast: AstId, + ast: ErasedAstId, cfg: CfgExpr, opts: CfgOptions, ) -> Self { diff --git a/crates/hir-expand/src/ast_id_map.rs b/crates/hir-expand/src/ast_id_map.rs index c2b0d5985e3..1906ed15bae 100644 --- a/crates/hir-expand/src/ast_id_map.rs +++ b/crates/hir-expand/src/ast_id_map.rs @@ -18,47 +18,89 @@ use syntax::{ast, AstNode, AstPtr, SyntaxNode, SyntaxNodePtr}; /// `AstId` points to an AST node in a specific file. -pub struct FileAstId { +pub struct FileAstId { raw: ErasedFileAstId, covariant: PhantomData N>, } -impl Clone for FileAstId { +impl Clone for FileAstId { fn clone(&self) -> FileAstId { *self } } -impl Copy for FileAstId {} +impl Copy for FileAstId {} -impl PartialEq for FileAstId { +impl PartialEq for FileAstId { fn eq(&self, other: &Self) -> bool { self.raw == other.raw } } -impl Eq for FileAstId {} -impl Hash for FileAstId { +impl Eq for FileAstId {} +impl Hash for FileAstId { fn hash(&self, hasher: &mut H) { self.raw.hash(hasher); } } -impl fmt::Debug for FileAstId { +impl fmt::Debug for FileAstId { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "FileAstId::<{}>({})", type_name::(), self.raw.into_raw()) } } -impl FileAstId { +impl FileAstId { // Can't make this a From implementation because of coherence - pub fn upcast(self) -> FileAstId + pub fn upcast(self) -> FileAstId where N: Into, { FileAstId { raw: self.raw, covariant: PhantomData } } + + pub fn erase(self) -> ErasedFileAstId { + self.raw + } } -type ErasedFileAstId = Idx; +pub type ErasedFileAstId = Idx; + +pub trait AstIdNode: AstNode {} +macro_rules! register_ast_id_node { + (impl AstIdNode for $($ident:ident),+ ) => { + $( + impl AstIdNode for ast::$ident {} + )+ + fn should_alloc_id(kind: syntax::SyntaxKind) -> bool { + $( + ast::$ident::can_cast(kind) + )||+ + } + }; +} +register_ast_id_node! { + impl AstIdNode for + Item, + Adt, + Enum, + Struct, + Union, + Const, + ExternBlock, + ExternCrate, + Fn, + Impl, + Macro, + MacroDef, + MacroRules, + MacroCall, + Module, + Static, + Trait, + TraitAlias, + TypeAlias, + Use, + AssocItem, BlockExpr, Variant, RecordField, TupleField, ConstArg +} /// Maps items' `SyntaxNode`s to `ErasedFileAstId`s and back. #[derive(Default)] @@ -92,14 +134,7 @@ pub(crate) fn from_source(node: &SyntaxNode) -> AstIdMap { // change parent's id. This means that, say, adding a new function to a // trait does not change ids of top-level items, which helps caching. bdfs(node, |it| { - let kind = it.kind(); - if ast::Item::can_cast(kind) - || ast::BlockExpr::can_cast(kind) - || ast::Variant::can_cast(kind) - || ast::RecordField::can_cast(kind) - || ast::TupleField::can_cast(kind) - || ast::ConstArg::can_cast(kind) - { + if should_alloc_id(it.kind()) { res.alloc(&it); true } else { @@ -120,15 +155,19 @@ pub(crate) fn from_source(node: &SyntaxNode) -> AstIdMap { res } - pub fn ast_id(&self, item: &N) -> FileAstId { + pub fn ast_id(&self, item: &N) -> FileAstId { let raw = self.erased_ast_id(item.syntax()); FileAstId { raw, covariant: PhantomData } } - pub fn get(&self, id: FileAstId) -> AstPtr { + pub fn get(&self, id: FileAstId) -> AstPtr { AstPtr::try_from_raw(self.arena[id.raw].clone()).unwrap() } + pub(crate) fn get_raw(&self, id: ErasedFileAstId) -> SyntaxNodePtr { + self.arena[id].clone() + } + fn erased_ast_id(&self, item: &SyntaxNode) -> ErasedFileAstId { let ptr = SyntaxNodePtr::new(item); let hash = hash_ptr(&ptr); diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs index b0dc8e1b5c9..3a534f56e4f 100644 --- a/crates/hir-expand/src/lib.rs +++ b/crates/hir-expand/src/lib.rs @@ -37,11 +37,11 @@ use syntax::{ algo::{self, skip_trivia_token}, ast::{self, AstNode, HasDocComments}, - Direction, SyntaxNode, SyntaxToken, + AstPtr, Direction, SyntaxNode, SyntaxNodePtr, SyntaxToken, }; use crate::{ - ast_id_map::FileAstId, + ast_id_map::{AstIdNode, ErasedFileAstId, FileAstId}, attrs::AttrId, builtin_attr_macro::BuiltinAttrExpander, builtin_derive_macro::BuiltinDeriveExpander, @@ -551,9 +551,9 @@ pub fn original_call_range_with_body(self, db: &dyn db::ExpandDatabase) -> FileR }; let range = match kind { - MacroCallKind::FnLike { ast_id, .. } => ast_id.to_node(db).syntax().text_range(), - MacroCallKind::Derive { ast_id, .. } => ast_id.to_node(db).syntax().text_range(), - MacroCallKind::Attr { ast_id, .. } => ast_id.to_node(db).syntax().text_range(), + MacroCallKind::FnLike { ast_id, .. } => ast_id.to_ptr(db).text_range(), + MacroCallKind::Derive { ast_id, .. } => ast_id.to_ptr(db).text_range(), + MacroCallKind::Attr { ast_id, .. } => ast_id.to_ptr(db).text_range(), }; FileRange { range, file_id } @@ -801,13 +801,25 @@ pub fn map_token_up( /// It is stable across reparses, and can be used as salsa key/value. pub type AstId = InFile>; -impl AstId { +impl AstId { pub fn to_node(&self, db: &dyn db::ExpandDatabase) -> N { - let root = db.parse_or_expand(self.file_id); - db.ast_id_map(self.file_id).get(self.value).to_node(&root) + self.to_ptr(db).to_node(&db.parse_or_expand(self.file_id)) + } + pub fn to_ptr(&self, db: &dyn db::ExpandDatabase) -> AstPtr { + db.ast_id_map(self.file_id).get(self.value) } } +pub type ErasedAstId = InFile; + +impl ErasedAstId { + pub fn to_node(&self, db: &dyn db::ExpandDatabase) -> SyntaxNode { + self.to_ptr(db).to_node(&db.parse_or_expand(self.file_id)) + } + pub fn to_ptr(&self, db: &dyn db::ExpandDatabase) -> SyntaxNodePtr { + db.ast_id_map(self.file_id).get_raw(self.value) + } +} /// `InFile` stores a value of `T` inside a particular file/syntax tree. /// /// Typical usages are: diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 645ea42f538..3688fd0e837 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -754,7 +754,7 @@ fn emit_def_diagnostic_( let item = ast.to_node(db.upcast()); acc.push( InactiveCode { - node: ast.with_value(AstPtr::new(&item).into()), + node: ast.with_value(SyntaxNodePtr::new(&item).into()), cfg: cfg.clone(), opts: opts.clone(), }