Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Correctly set and mark the proc-macro spans #16175

Merged
merged 4 commits into from
Dec 21, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions crates/hir-def/src/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -637,9 +637,12 @@ impl<'attr> AttrQuery<'attr> {
}
}

fn any_has_attrs(
db: &dyn DefDatabase,
id: impl Lookup<Data = impl HasSource<Value = impl ast::HasAttrs>>,
fn any_has_attrs<'db>(
db: &(dyn DefDatabase + 'db),
id: impl Lookup<
Database<'db> = dyn DefDatabase + 'db,
Data = impl HasSource<Value = impl ast::HasAttrs>,
>,
) -> InFile<ast::AnyHasAttrs> {
id.lookup(db).source(db).map(ast::AnyHasAttrs::new)
}
Expand All @@ -650,17 +653,17 @@ fn attrs_from_item_tree<N: ItemTreeNode>(db: &dyn DefDatabase, id: ItemTreeId<N>
tree.raw_attrs(mod_item.into()).clone()
}

fn attrs_from_item_tree_loc<N: ItemTreeNode>(
db: &dyn DefDatabase,
lookup: impl Lookup<Data = ItemLoc<N>>,
fn attrs_from_item_tree_loc<'db, N: ItemTreeNode>(
db: &(dyn DefDatabase + 'db),
lookup: impl Lookup<Database<'db> = dyn DefDatabase + 'db, Data = ItemLoc<N>>,
) -> RawAttrs {
let id = lookup.lookup(db).id;
attrs_from_item_tree(db, id)
}

fn attrs_from_item_tree_assoc<N: ItemTreeNode>(
db: &dyn DefDatabase,
lookup: impl Lookup<Data = AssocItemLoc<N>>,
fn attrs_from_item_tree_assoc<'db, N: ItemTreeNode>(
db: &(dyn DefDatabase + 'db),
lookup: impl Lookup<Database<'db> = dyn DefDatabase + 'db, Data = AssocItemLoc<N>>,
) -> RawAttrs {
let id = lookup.lookup(db).id;
attrs_from_item_tree(db, id)
Expand Down
36 changes: 22 additions & 14 deletions crates/hir-def/src/body/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -959,20 +959,29 @@ impl ExprCollector<'_> {
// File containing the macro call. Expansion errors will be attached here.
let outer_file = self.expander.current_file_id;

let macro_call_ptr = self.expander.to_source(AstPtr::new(&mcall));
let macro_call_ptr = self.expander.to_source(syntax_ptr);
let module = self.expander.module.local_id;
let res = self.expander.enter_expand(self.db, mcall, |path| {
self.def_map
.resolve_path(
self.db,
module,
&path,
crate::item_scope::BuiltinShadowMode::Other,
Some(MacroSubNs::Bang),
)
.0
.take_macros()
});

let res = match self.def_map.modules[module]
.scope
.macro_invocations
.get(&InFile::new(outer_file, self.ast_id_map.ast_id_for_ptr(syntax_ptr)))
{
// fast path, macro call is in a block module
Some(&call) => Ok(self.expander.enter_expand_id(self.db, call)),
None => self.expander.enter_expand(self.db, mcall, |path| {
self.def_map
.resolve_path(
self.db,
module,
&path,
crate::item_scope::BuiltinShadowMode::Other,
Some(MacroSubNs::Bang),
)
.0
.take_macros()
}),
};

let res = match res {
Ok(res) => res,
Expand All @@ -986,7 +995,6 @@ impl ExprCollector<'_> {
return collector(self, None);
}
};

if record_diagnostics {
match &res.err {
Some(ExpandError::UnresolvedProcMacro(krate)) => {
Expand Down
4 changes: 2 additions & 2 deletions crates/hir-def/src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::{
db::DefDatabase,
expander::{Expander, Mark},
item_tree::{self, AssocItem, FnFlags, ItemTree, ItemTreeId, MacroCall, ModItem, TreeId},
macro_call_as_call_id, macro_id_to_def_id,
macro_call_as_call_id,
nameres::{
attr_resolution::ResolvedAttr,
diagnostics::DefDiagnostic,
Expand Down Expand Up @@ -720,7 +720,7 @@ impl<'a> AssocItemCollector<'a> {
)
.0
.take_macros()
.map(|it| macro_id_to_def_id(self.db, it))
.map(|it| self.db.macro_def(it))
};
match macro_call_as_call_id(
self.db.upcast(),
Expand Down
79 changes: 75 additions & 4 deletions crates/hir-def/src/db.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Defines database & queries for name resolution.
use base_db::{salsa, CrateId, SourceDatabase, Upcast};
use either::Either;
use hir_expand::{db::ExpandDatabase, HirFileId};
use hir_expand::{db::ExpandDatabase, HirFileId, MacroDefId};
use intern::Interned;
use la_arena::ArenaMap;
use syntax::{ast, AstPtr};
Expand All @@ -24,9 +24,9 @@ use crate::{
AttrDefId, BlockId, BlockLoc, ConstBlockId, ConstBlockLoc, ConstId, ConstLoc, DefWithBodyId,
EnumId, EnumLoc, ExternBlockId, ExternBlockLoc, ExternCrateId, ExternCrateLoc, FunctionId,
FunctionLoc, GenericDefId, ImplId, ImplLoc, InTypeConstId, InTypeConstLoc, LocalEnumVariantId,
LocalFieldId, Macro2Id, Macro2Loc, MacroRulesId, MacroRulesLoc, ProcMacroId, ProcMacroLoc,
StaticId, StaticLoc, StructId, StructLoc, TraitAliasId, TraitAliasLoc, TraitId, TraitLoc,
TypeAliasId, TypeAliasLoc, UnionId, UnionLoc, UseId, UseLoc, VariantId,
LocalFieldId, Macro2Id, Macro2Loc, MacroId, MacroRulesId, MacroRulesLoc, ProcMacroId,
ProcMacroLoc, StaticId, StaticLoc, StructId, StructLoc, TraitAliasId, TraitAliasLoc, TraitId,
TraitLoc, TypeAliasId, TypeAliasLoc, UnionId, UnionLoc, UseId, UseLoc, VariantId,
};

#[salsa::query_group(InternDatabaseStorage)]
Expand Down Expand Up @@ -110,6 +110,8 @@ pub trait DefDatabase: InternDatabase + ExpandDatabase + Upcast<dyn ExpandDataba
#[salsa::invoke(DefMap::block_def_map_query)]
fn block_def_map(&self, block: BlockId) -> Arc<DefMap>;

fn macro_def(&self, m: MacroId) -> MacroDefId;

// region:data

#[salsa::invoke(StructData::struct_data_query)]
Expand Down Expand Up @@ -305,3 +307,72 @@ fn crate_supports_no_std(db: &dyn DefDatabase, crate_id: CrateId) -> bool {

false
}

fn macro_def(db: &dyn DefDatabase, id: MacroId) -> MacroDefId {
use hir_expand::InFile;

use crate::{Lookup, MacroDefKind, MacroExpander};

let kind = |expander, file_id, m| {
let in_file = InFile::new(file_id, m);
match expander {
MacroExpander::Declarative => MacroDefKind::Declarative(in_file),
MacroExpander::BuiltIn(it) => MacroDefKind::BuiltIn(it, in_file),
MacroExpander::BuiltInAttr(it) => MacroDefKind::BuiltInAttr(it, in_file),
MacroExpander::BuiltInDerive(it) => MacroDefKind::BuiltInDerive(it, in_file),
MacroExpander::BuiltInEager(it) => MacroDefKind::BuiltInEager(it, in_file),
}
};

match id {
MacroId::Macro2Id(it) => {
let loc: Macro2Loc = it.lookup(db);

let item_tree = loc.id.item_tree(db);
let makro = &item_tree[loc.id.value];
MacroDefId {
krate: loc.container.krate,
kind: kind(loc.expander, loc.id.file_id(), makro.ast_id.upcast()),
local_inner: false,
allow_internal_unsafe: loc.allow_internal_unsafe,
def_site: db
.span_map(loc.id.file_id())
.span_for_range(db.ast_id_map(loc.id.file_id()).get(makro.ast_id).text_range()),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slow down comes from this, which I assume misses the cache for the span maps in some occurrences unfortunately

}
}
MacroId::MacroRulesId(it) => {
let loc: MacroRulesLoc = it.lookup(db);

let item_tree = loc.id.item_tree(db);
let makro = &item_tree[loc.id.value];
MacroDefId {
krate: loc.container.krate,
kind: kind(loc.expander, loc.id.file_id(), makro.ast_id.upcast()),
local_inner: loc.local_inner,
allow_internal_unsafe: loc.allow_internal_unsafe,
def_site: db
.span_map(loc.id.file_id())
.span_for_range(db.ast_id_map(loc.id.file_id()).get(makro.ast_id).text_range()),
}
}
MacroId::ProcMacroId(it) => {
let loc = it.lookup(db);

let item_tree = loc.id.item_tree(db);
let makro = &item_tree[loc.id.value];
MacroDefId {
krate: loc.container.krate,
kind: MacroDefKind::ProcMacro(
loc.expander,
loc.kind,
InFile::new(loc.id.file_id(), makro.ast_id),
),
local_inner: false,
allow_internal_unsafe: false,
def_site: db
.span_map(loc.id.file_id())
.span_for_range(db.ast_id_map(loc.id.file_id()).get(makro.ast_id).text_range()),
}
}
}
}
6 changes: 3 additions & 3 deletions crates/hir-def/src/expander.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use limit::Limit;
use syntax::{ast, Parse, SyntaxNode};

use crate::{
attr::Attrs, db::DefDatabase, lower::LowerCtx, macro_id_to_def_id, path::Path, AsMacroCall,
MacroId, ModuleId, UnresolvedMacro,
attr::Attrs, db::DefDatabase, lower::LowerCtx, path::Path, AsMacroCall, MacroId, ModuleId,
UnresolvedMacro,
};

#[derive(Debug)]
Expand Down Expand Up @@ -58,7 +58,7 @@ impl Expander {
let result = self.within_limit(db, |this| {
let macro_call = InFile::new(this.current_file_id, &macro_call);
match macro_call.as_call_id_with_errors(db.upcast(), this.module.krate(), |path| {
resolver(path).map(|it| macro_id_to_def_id(db, it))
resolver(path).map(|it| db.macro_def(it))
}) {
Ok(call_id) => call_id,
Err(resolve_err) => {
Expand Down
10 changes: 9 additions & 1 deletion crates/hir-def/src/item_scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,10 @@ pub struct ItemScope {
// FIXME: Macro shadowing in one module is not properly handled. Non-item place macros will
// be all resolved to the last one defined if shadowing happens.
legacy_macros: FxHashMap<Name, SmallVec<[MacroId; 1]>>,
/// The derive macro invocations in this scope.
/// The attribute macro invocations in this scope.
attr_macros: FxHashMap<AstId<ast::Item>, MacroCallId>,
/// The macro invocations in this scope.
pub macro_invocations: FxHashMap<AstId<ast::MacroCall>, MacroCallId>,
/// The derive macro invocations in this scope, keyed by the owner item over the actual derive attributes
/// paired with the derive macro invocations for the specific attribute.
derive_macros: FxHashMap<AstId<ast::Adt>, SmallVec<[DeriveMacroInvocation; 1]>>,
Expand Down Expand Up @@ -345,6 +347,10 @@ impl ItemScope {
self.attr_macros.insert(item, call);
}

pub(crate) fn add_macro_invoc(&mut self, call: AstId<ast::MacroCall>, call_id: MacroCallId) {
self.macro_invocations.insert(call, call_id);
}

pub(crate) fn attr_macro_invocs(
&self,
) -> impl Iterator<Item = (AstId<ast::Item>, MacroCallId)> + '_ {
Expand Down Expand Up @@ -692,6 +698,7 @@ impl ItemScope {
use_imports_values,
use_imports_types,
use_imports_macros,
macro_invocations,
} = self;
types.shrink_to_fit();
values.shrink_to_fit();
Expand All @@ -709,6 +716,7 @@ impl ItemScope {
derive_macros.shrink_to_fit();
extern_crate_decls.shrink_to_fit();
use_decls.shrink_to_fit();
macro_invocations.shrink_to_fit();
}
}

Expand Down
14 changes: 9 additions & 5 deletions crates/hir-def/src/item_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
//!
//! In general, any item in the `ItemTree` stores its `AstId`, which allows mapping it back to its
//! surface syntax.
//!
//! Note that we cannot store [`span::Span`]s inside of this, as typing in an item invalidates its
//! encompassing span!

mod lower;
mod pretty;
Expand Down Expand Up @@ -281,7 +284,7 @@ struct ItemTreeData {
mods: Arena<Mod>,
macro_calls: Arena<MacroCall>,
macro_rules: Arena<MacroRules>,
macro_defs: Arena<MacroDef>,
macro_defs: Arena<Macro2>,

vis: ItemVisibilities,
}
Expand Down Expand Up @@ -514,7 +517,7 @@ mod_items! {
Mod in mods -> ast::Module,
MacroCall in macro_calls -> ast::MacroCall,
MacroRules in macro_rules -> ast::MacroRules,
MacroDef in macro_defs -> ast::MacroDef,
Macro2 in macro_defs -> ast::MacroDef,
}

macro_rules! impl_index {
Expand Down Expand Up @@ -747,6 +750,7 @@ pub struct MacroCall {
pub path: Interned<ModPath>,
pub ast_id: FileAstId<ast::MacroCall>,
pub expand_to: ExpandTo,
// FIXME: We need to move this out. It invalidates the item tree when typing inside the macro call.
pub call_site: Span,
}

Expand All @@ -759,7 +763,7 @@ pub struct MacroRules {

/// "Macros 2.0" macro definition.
#[derive(Debug, Clone, Eq, PartialEq)]
pub struct MacroDef {
pub struct Macro2 {
pub name: Name,
pub visibility: RawVisibilityId,
pub ast_id: FileAstId<ast::MacroDef>,
Expand Down Expand Up @@ -918,7 +922,7 @@ impl ModItem {
| ModItem::Impl(_)
| ModItem::Mod(_)
| ModItem::MacroRules(_)
| ModItem::MacroDef(_) => None,
| ModItem::Macro2(_) => None,
ModItem::MacroCall(call) => Some(AssocItem::MacroCall(*call)),
ModItem::Const(konst) => Some(AssocItem::Const(*konst)),
ModItem::TypeAlias(alias) => Some(AssocItem::TypeAlias(*alias)),
Expand All @@ -944,7 +948,7 @@ impl ModItem {
ModItem::Mod(it) => tree[it.index()].ast_id().upcast(),
ModItem::MacroCall(it) => tree[it.index()].ast_id().upcast(),
ModItem::MacroRules(it) => tree[it.index()].ast_id().upcast(),
ModItem::MacroDef(it) => tree[it.index()].ast_id().upcast(),
ModItem::Macro2(it) => tree[it.index()].ast_id().upcast(),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/hir-def/src/item_tree/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,13 +562,13 @@ impl<'a> Ctx<'a> {
Some(id(self.data().macro_rules.alloc(res)))
}

fn lower_macro_def(&mut self, m: &ast::MacroDef) -> Option<FileItemTreeId<MacroDef>> {
fn lower_macro_def(&mut self, m: &ast::MacroDef) -> Option<FileItemTreeId<Macro2>> {
let name = m.name().map(|it| it.as_name())?;

let ast_id = self.source_ast_id_map.ast_id(m);
let visibility = self.lower_visibility(m);

let res = MacroDef { name, ast_id, visibility };
let res = Macro2 { name, ast_id, visibility };
Some(id(self.data().macro_defs.alloc(res)))
}

Expand Down
4 changes: 2 additions & 2 deletions crates/hir-def/src/item_tree/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -464,8 +464,8 @@ impl Printer<'_> {
let MacroRules { name, ast_id: _ } = &self.tree[it];
wln!(self, "macro_rules! {} {{ ... }}", name.display(self.db.upcast()));
}
ModItem::MacroDef(it) => {
let MacroDef { name, visibility, ast_id: _ } = &self.tree[it];
ModItem::Macro2(it) => {
let Macro2 { name, visibility, ast_id: _ } = &self.tree[it];
self.print_visibility(*visibility);
wln!(self, "macro {} {{ ... }}", name.display(self.db.upcast()));
}
Expand Down
Loading