From 15d183be79f5d31a46f40b54a8308f13cf2879cd Mon Sep 17 00:00:00 2001 From: Wyatt Herkamp Date: Thu, 14 Mar 2024 08:52:13 -0400 Subject: [PATCH 1/8] Initial Attempt limiting number of token tree in macro expansion. --- crates/hir-def/src/nameres/attr_resolution.rs | 2 + crates/hir-def/src/nameres/collector.rs | 122 ++++++++++++------ crates/hir-expand/src/db.rs | 31 ++++- crates/hir-expand/src/lib.rs | 3 + crates/hir/src/lib.rs | 6 +- 5 files changed, 119 insertions(+), 45 deletions(-) diff --git a/crates/hir-def/src/nameres/attr_resolution.rs b/crates/hir-def/src/nameres/attr_resolution.rs index 662c80edf324..eb7f4c05ae21 100644 --- a/crates/hir-def/src/nameres/attr_resolution.rs +++ b/crates/hir-def/src/nameres/attr_resolution.rs @@ -136,6 +136,7 @@ pub(super) fn derive_macro_as_call_id( call_site: SyntaxContextId, krate: CrateId, resolver: impl Fn(path::ModPath) -> Option<(MacroId, MacroDefId)>, + derive_macro_id: MacroCallId, ) -> Result<(MacroId, MacroDefId, MacroCallId), UnresolvedMacro> { let (macro_id, def_id) = resolver(item_attr.path.clone()) .filter(|(_, def_id)| def_id.is_derive()) @@ -147,6 +148,7 @@ pub(super) fn derive_macro_as_call_id( ast_id: item_attr.ast_id, derive_index: derive_pos, derive_attr_index, + derive_macro_id, }, call_site, ); diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 312938e2c17f..acdf5022c532 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -3,58 +3,86 @@ //! `DefCollector::collect` contains the fixed-point iteration loop which //! resolves imports and expands macros. -use std::{cmp::Ordering, iter, mem, ops::Not}; +use std::{iter, mem}; +use crate::attr::Attrs; +use crate::item_tree::Fields; +use crate::item_tree::FileItemTreeId; +use crate::item_tree::MacroCall; +use crate::item_tree::MacroRules; +use crate::item_tree::Mod; +use crate::item_tree::ModKind; +use crate::macro_call_as_call_id_with_eager; + +use crate::nameres::mod_resolution::ModDir; + +use crate::item_tree::ItemTree; use base_db::{CrateId, Dependency, FileId}; use cfg::{CfgExpr, CfgOptions}; + +use crate::item_tree::TreeId; + +use crate::LocalModuleId; +use crate::{ + item_tree::{ExternCrate, ItemTreeId, Macro2, ModItem}, + nameres::{ + diagnostics::DefDiagnostic, proc_macro::parse_macro_name_and_helper_attrs, + sub_namespace_match, BuiltinShadowMode, DefMap, MacroSubNs, ModuleData, ModuleOrigin, + ResolveMode, + }, + path::ModPath, + per_ns::PerNs, + tt, + visibility::Visibility, + AstId, AstIdWithPath, ConstLoc, EnumLoc, EnumVariantLoc, ExternBlockLoc, ExternCrateId, + ExternCrateLoc, FunctionLoc, ImplLoc, Intern, ItemContainerId, Macro2Loc, MacroExpander, + MacroRulesLoc, MacroRulesLocFlags, ModuleDefId, ModuleId, StaticLoc, StructLoc, TraitAliasLoc, + TraitLoc, TypeAliasLoc, UnionLoc, UseLoc, +}; + +use std::{cmp::Ordering, ops::Not}; + use either::Either; use hir_expand::{ - attrs::{Attr, AttrId}, - builtin_attr_macro::{find_builtin_attr, BuiltinAttrExpander}, + builtin_attr_macro::find_builtin_attr, builtin_derive_macro::find_builtin_derive, builtin_fn_macro::find_builtin_macro, - name::{name, AsName, Name}, + name::{AsName, Name}, + HirFileId, InFile, +}; + +use itertools::Itertools; +use span::{ErasedFileAstId, FileAstId, Span, SyntaxContextId}; + +use hir_expand::{ + attrs::{Attr, AttrId}, + builtin_attr_macro::BuiltinAttrExpander, + name::name, proc_macro::CustomProcMacroExpander, - ExpandResult, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroCallLoc, - MacroDefId, MacroDefKind, + ExpandResult, ExpandTo, MacroCallId, MacroCallKind, MacroCallLoc, MacroDefId, MacroDefKind, }; -use itertools::{izip, Itertools}; +use itertools::izip; use la_arena::Idx; use limit::Limit; use rustc_hash::{FxHashMap, FxHashSet}; -use span::{Edition, ErasedFileAstId, FileAstId, Span, SyntaxContextId}; use stdx::always; use syntax::{ast, SmolStr}; use triomphe::Arc; use crate::{ - attr::Attrs, db::DefDatabase, item_scope::{ImportId, ImportOrExternCrate, ImportType, PerNsGlobImports}, - item_tree::{ - self, ExternCrate, Fields, FileItemTreeId, ImportKind, ItemTree, ItemTreeId, ItemTreeNode, - Macro2, MacroCall, MacroRules, Mod, ModItem, ModKind, TreeId, - }, - macro_call_as_call_id, macro_call_as_call_id_with_eager, + item_tree::{self, ImportKind, ItemTreeNode}, + macro_call_as_call_id, nameres::{ attr_resolution::{attr_macro_as_call_id, derive_macro_as_call_id}, - diagnostics::DefDiagnostic, - mod_resolution::ModDir, path_resolution::ReachedFixedPoint, - proc_macro::{parse_macro_name_and_helper_attrs, ProcMacroDef, ProcMacroKind}, - sub_namespace_match, BuiltinShadowMode, DefMap, MacroSubNs, ModuleData, ModuleOrigin, - ResolveMode, + proc_macro::{ProcMacroDef, ProcMacroKind}, }, - path::{ImportAlias, ModPath, PathKind}, - per_ns::PerNs, - tt, - visibility::{RawVisibility, Visibility}, - AdtId, AstId, AstIdWithPath, ConstLoc, CrateRootModuleId, EnumLoc, EnumVariantLoc, - ExternBlockLoc, ExternCrateId, ExternCrateLoc, FunctionId, FunctionLoc, ImplLoc, Intern, - ItemContainerId, LocalModuleId, Lookup, Macro2Id, Macro2Loc, MacroExpander, MacroId, - MacroRulesId, MacroRulesLoc, MacroRulesLocFlags, ModuleDefId, ModuleId, ProcMacroId, - ProcMacroLoc, StaticLoc, StructLoc, TraitAliasLoc, TraitLoc, TypeAliasLoc, UnionLoc, - UnresolvedMacro, UseId, UseLoc, + path::{ImportAlias, PathKind}, + visibility::RawVisibility, + AdtId, CrateRootModuleId, FunctionId, Lookup, Macro2Id, MacroId, MacroRulesId, ProcMacroId, + ProcMacroLoc, UnresolvedMacro, UseId, }; static GLOB_RECURSION_LIMIT: Limit = Limit::new(100); @@ -237,6 +265,8 @@ enum MacroDirectiveKind { derive_attr: AttrId, derive_pos: usize, ctxt: SyntaxContextId, + /// The "parent" macro it is resolved to. + derive_macro_id: MacroCallId, }, Attr { ast_id: AstIdWithPath, @@ -1146,7 +1176,13 @@ impl DefCollector<'_> { return Resolved::Yes; } } - MacroDirectiveKind::Derive { ast_id, derive_attr, derive_pos, ctxt: call_site } => { + MacroDirectiveKind::Derive { + ast_id, + derive_attr, + derive_pos, + ctxt: call_site, + derive_macro_id, + } => { let id = derive_macro_as_call_id( self.db, ast_id, @@ -1155,6 +1191,7 @@ impl DefCollector<'_> { *call_site, self.def_map.krate, resolver, + *derive_macro_id, ); if let Ok((macro_id, def_id, call_id)) = id { @@ -1223,7 +1260,9 @@ impl DefCollector<'_> { Some(def) if def.is_attribute() => def, _ => return Resolved::No, }; - + // We will treat derive macros as an attribute as a reference for the input to derives + let call_id = + attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def); if let MacroDefId { kind: MacroDefKind::BuiltInAttr( @@ -1267,6 +1306,7 @@ impl DefCollector<'_> { derive_attr: attr.id, derive_pos: idx, ctxt: call_site.ctx, + derive_macro_id: call_id, }, container: directive.container, }); @@ -1301,10 +1341,6 @@ impl DefCollector<'_> { return recollect_without(self); } - // Not resolved to a derive helper or the derive attribute, so try to treat as a normal attribute. - let call_id = - attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def); - // Skip #[test]/#[bench] expansion, which would merely result in more memory usage // due to duplicating functions into macro expansions if matches!( @@ -1460,13 +1496,20 @@ impl DefCollector<'_> { )); } } - MacroDirectiveKind::Derive { ast_id, derive_attr, derive_pos, ctxt: _ } => { + MacroDirectiveKind::Derive { + ast_id, + derive_attr, + derive_pos, + derive_macro_id, + .. + } => { self.def_map.diagnostics.push(DefDiagnostic::unresolved_macro_call( directive.module_id, MacroCallKind::Derive { ast_id: ast_id.ast_id, derive_attr_index: *derive_attr, derive_index: *derive_pos as u32, + derive_macro_id: *derive_macro_id, }, ast_id.path.clone(), )); @@ -2289,7 +2332,7 @@ impl ModCollector<'_, '_> { fn collect_macro_call( &mut self, - &MacroCall { ref path, ast_id, expand_to, ctxt }: &MacroCall, + &MacroCall { ref path, ast_id, expand_to, ctxt, .. }: &MacroCall, container: ItemContainerId, ) { let ast_id = AstIdWithPath::new(self.file_id(), ast_id, ModPath::clone(path)); @@ -2428,7 +2471,10 @@ mod tests { use base_db::SourceDatabase; use test_fixture::WithFixture; - use crate::{nameres::DefMapCrateData, test_db::TestDB}; + use crate::{ + nameres::{DefMapCrateData, ModuleData, ModuleOrigin}, + test_db::TestDB, + }; use super::*; diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs index ec68f2f96e5e..d7036788c30d 100644 --- a/crates/hir-expand/src/db.rs +++ b/crates/hir-expand/src/db.rs @@ -338,13 +338,33 @@ pub(crate) fn parse_with_map( } } } +/// This is just to ensure the types of smart_macro_arg and macro_arg are the same +type MacroArgResult = (Arc, SyntaxFixupUndoInfo, Span) ; +/// Imagine the word smart in quotes. +/// +/// This resolves the [MacroCallId] to check if it is a derive macro if so get the [macro_arg] for the derive. +/// Other wise return the [macro_arg] for the macro_call_id. +/// +/// This is not connected to the database so it does not cached the result. However, the inner [macro_arg] query is +/// +/// FIXME: Pick a better name +fn smart_macro_arg(db: &dyn ExpandDatabase, id: MacroCallId) -> MacroArgResult { + let loc = db.lookup_intern_macro_call(id); + // FIXME: We called lookup_intern_macro_call twice. + match loc.kind { + // Get the macro arg for the derive macro + MacroCallKind::Derive { derive_macro_id, .. } => db.macro_arg(derive_macro_id), + // Normal macro arg + _ => db.macro_arg(id), + } +} -// FIXME: for derive attributes, this will return separate copies of the same structures! Though -// they may differ in spans due to differing call sites... fn macro_arg( db: &dyn ExpandDatabase, id: MacroCallId, -) -> (Arc, SyntaxFixupUndoInfo, Span) { + // FIXME: consider the following by putting fixup info into eager call info args + // ) -> ValueResult, Arc>> { +) -> MacroArgResult { let loc = db.lookup_intern_macro_call(id); if let MacroCallLoc { @@ -526,7 +546,8 @@ fn macro_expand( let (ExpandResult { value: tt, err }, span) = match loc.def.kind { MacroDefKind::ProcMacro(..) => return db.expand_proc_macro(macro_call_id).map(CowArc::Arc), _ => { - let (macro_arg, undo_info, span) = db.macro_arg(macro_call_id); + let (macro_arg, undo_info, span) = + smart_macro_arg(db, macro_call_id); let arg = &*macro_arg; let res = @@ -603,7 +624,7 @@ fn proc_macro_span(db: &dyn ExpandDatabase, ast: AstId) -> Span { fn expand_proc_macro(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandResult> { let loc = db.lookup_intern_macro_call(id); - let (macro_arg, undo_info, span) = db.macro_arg(id); + let (macro_arg, undo_info, span) = smart_macro_arg(db, id); let (expander, ast) = match loc.def.kind { MacroDefKind::ProcMacro(expander, _, ast) => (expander, ast), diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs index 2233ae51fa84..e7a313c68cdd 100644 --- a/crates/hir-expand/src/lib.rs +++ b/crates/hir-expand/src/lib.rs @@ -224,6 +224,9 @@ pub enum MacroCallKind { derive_attr_index: AttrId, /// Index of the derive macro in the derive attribute derive_index: u32, + /// The "parent" macro call. + /// We will resolve the same token tree for all derive macros in the same derive attribute. + derive_macro_id: MacroCallId, }, Attr { ast_id: AstId, diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 15a0967b3d7f..2585d8e6f4e0 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -972,7 +972,8 @@ fn precise_macro_call_location( MacroKind::ProcMacro, ) } - MacroCallKind::Derive { ast_id, derive_attr_index, derive_index } => { + // TODO: derive_macro_id + MacroCallKind::Derive { ast_id, derive_attr_index, derive_index, derive_macro_id } => { let node = ast_id.to_node(db.upcast()); // Compute the precise location of the macro name's token in the derive // list. @@ -3709,7 +3710,8 @@ impl Impl { let macro_file = src.file_id.macro_file()?; let loc = macro_file.macro_call_id.lookup(db.upcast()); let (derive_attr, derive_index) = match loc.kind { - MacroCallKind::Derive { ast_id, derive_attr_index, derive_index } => { + // TODO: derive_macro_id + MacroCallKind::Derive { ast_id, derive_attr_index, derive_index, derive_macro_id } => { let module_id = self.id.lookup(db.upcast()).container; ( db.crate_def_map(module_id.krate())[module_id.local_id] From f499564d0a39eca95b97c16bde8e34324225b1ab Mon Sep 17 00:00:00 2001 From: Wyatt Herkamp Date: Thu, 14 Mar 2024 09:49:00 -0400 Subject: [PATCH 2/8] censor attribute derive --- crates/hir-expand/src/db.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs index d7036788c30d..163f05bdabbf 100644 --- a/crates/hir-expand/src/db.rs +++ b/crates/hir-expand/src/db.rs @@ -7,6 +7,7 @@ use mbe::syntax_node_to_token_tree; use rustc_hash::FxHashSet; use span::{AstIdMap, Span, SyntaxContextData, SyntaxContextId}; use syntax::{ast, AstNode, Parse, SyntaxElement, SyntaxError, SyntaxNode, SyntaxToken, T}; +use tracing::debug; use triomphe::Arc; use crate::{ @@ -353,7 +354,13 @@ fn smart_macro_arg(db: &dyn ExpandDatabase, id: MacroCallId) -> MacroArgResult { // FIXME: We called lookup_intern_macro_call twice. match loc.kind { // Get the macro arg for the derive macro - MacroCallKind::Derive { derive_macro_id, .. } => db.macro_arg(derive_macro_id), + MacroCallKind::Derive { derive_macro_id, .. } => { + debug!( + ?loc, + "{id:?} is a derive macro. Using the derive macro, id: {derive_macro_id:?}, attribute as the macro arg." + ); + db.macro_arg(derive_macro_id) + } // Normal macro arg _ => db.macro_arg(id), } From 4bd2f948bb9c95fae834111f1e68baa99640e527 Mon Sep 17 00:00:00 2001 From: Wyatt Herkamp Date: Fri, 15 Mar 2024 11:15:02 -0400 Subject: [PATCH 3/8] Formatting --- crates/hir-def/src/nameres/collector.rs | 2 -- crates/hir-expand/src/db.rs | 5 ++--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index acdf5022c532..481e8a4cb434 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -17,8 +17,6 @@ use crate::macro_call_as_call_id_with_eager; use crate::nameres::mod_resolution::ModDir; use crate::item_tree::ItemTree; -use base_db::{CrateId, Dependency, FileId}; -use cfg::{CfgExpr, CfgOptions}; use crate::item_tree::TreeId; diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs index 163f05bdabbf..004c4473011f 100644 --- a/crates/hir-expand/src/db.rs +++ b/crates/hir-expand/src/db.rs @@ -340,7 +340,7 @@ pub(crate) fn parse_with_map( } } /// This is just to ensure the types of smart_macro_arg and macro_arg are the same -type MacroArgResult = (Arc, SyntaxFixupUndoInfo, Span) ; +type MacroArgResult = (Arc, SyntaxFixupUndoInfo, Span); /// Imagine the word smart in quotes. /// /// This resolves the [MacroCallId] to check if it is a derive macro if so get the [macro_arg] for the derive. @@ -553,8 +553,7 @@ fn macro_expand( let (ExpandResult { value: tt, err }, span) = match loc.def.kind { MacroDefKind::ProcMacro(..) => return db.expand_proc_macro(macro_call_id).map(CowArc::Arc), _ => { - let (macro_arg, undo_info, span) = - smart_macro_arg(db, macro_call_id); + let (macro_arg, undo_info, span) = smart_macro_arg(db, macro_call_id); let arg = &*macro_arg; let res = From 2c2bbe07fdc08b52dfb4e9cb47ac66ecd4276027 Mon Sep 17 00:00:00 2001 From: Wyatt Herkamp Date: Mon, 18 Mar 2024 07:10:02 -0400 Subject: [PATCH 4/8] Treat Derive Macro specially. --- crates/hir-def/src/nameres/attr_resolution.rs | 17 +++++++-- crates/hir-def/src/nameres/collector.rs | 18 +++++++--- crates/hir-expand/src/cfg_process.rs | 2 +- crates/hir-expand/src/db.rs | 35 +++++++------------ crates/hir-expand/src/lib.rs | 29 ++++++++++++--- crates/hir/src/lib.rs | 26 +++++++++++--- 6 files changed, 87 insertions(+), 40 deletions(-) diff --git a/crates/hir-def/src/nameres/attr_resolution.rs b/crates/hir-def/src/nameres/attr_resolution.rs index eb7f4c05ae21..e2d83c8e3bfe 100644 --- a/crates/hir-def/src/nameres/attr_resolution.rs +++ b/crates/hir-def/src/nameres/attr_resolution.rs @@ -3,7 +3,7 @@ use base_db::CrateId; use hir_expand::{ attrs::{Attr, AttrId, AttrInput}, - MacroCallId, MacroCallKind, MacroDefId, + AstId, MacroCallId, MacroCallKind, MacroDefId, }; use span::SyntaxContextId; use syntax::{ast, SmolStr}; @@ -98,7 +98,20 @@ impl DefMap { false } } - +pub(super) fn derive_attr_macro_as_call_id( + db: &dyn DefDatabase, + item_attr: &AstId, + macro_attr: &Attr, + krate: CrateId, + def: MacroDefId, +) -> MacroCallId { + def.make_call( + db.upcast(), + krate, + MacroCallKind::DeriveAttr { ast_id: *item_attr, invoc_attr_index: macro_attr.id }, + macro_attr.ctxt, + ) +} pub(super) fn attr_macro_as_call_id( db: &dyn DefDatabase, item_attr: &AstIdWithPath, diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 481e8a4cb434..27ad3ad1f4ca 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -14,6 +14,7 @@ use crate::item_tree::Mod; use crate::item_tree::ModKind; use crate::macro_call_as_call_id_with_eager; +use crate::nameres::attr_resolution::derive_attr_macro_as_call_id; use crate::nameres::mod_resolution::ModDir; use crate::item_tree::ItemTree; @@ -1258,9 +1259,7 @@ impl DefCollector<'_> { Some(def) if def.is_attribute() => def, _ => return Resolved::No, }; - // We will treat derive macros as an attribute as a reference for the input to derives - let call_id = - attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def); + if let MacroDefId { kind: MacroDefKind::BuiltInAttr( @@ -1289,8 +1288,16 @@ impl DefCollector<'_> { return recollect_without(self); } }; - let ast_id = ast_id.with_value(ast_adt_id); + let ast_id = ast_id.with_value(ast_adt_id); + // the call_id for the actual derive macro. This is used so all the derive proc macros can share a token tree + let call_id = derive_attr_macro_as_call_id( + self.db, + &ast_id, + attr, + self.def_map.krate, + def, + ); match attr.parse_path_comma_token_tree(self.db.upcast()) { Some(derive_macros) => { let mut len = 0; @@ -1338,7 +1345,8 @@ impl DefCollector<'_> { return recollect_without(self); } - + let call_id = + attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def); // Skip #[test]/#[bench] expansion, which would merely result in more memory usage // due to duplicating functions into macro expansions if matches!( diff --git a/crates/hir-expand/src/cfg_process.rs b/crates/hir-expand/src/cfg_process.rs index 68de05fafe28..7e9ff5c85b54 100644 --- a/crates/hir-expand/src/cfg_process.rs +++ b/crates/hir-expand/src/cfg_process.rs @@ -180,7 +180,7 @@ pub(crate) fn process_cfg_attrs( db: &dyn ExpandDatabase, ) -> Option> { // FIXME: #[cfg_eval] is not implemented. But it is not stable yet - if !matches!(loc.kind, MacroCallKind::Derive { .. }) { + if !matches!(loc.kind, MacroCallKind::Derive { .. } | MacroCallKind::DeriveAttr { .. }) { return None; } let mut remove = FxHashSet::default(); diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs index 004c4473011f..277c268086bc 100644 --- a/crates/hir-expand/src/db.rs +++ b/crates/hir-expand/src/db.rs @@ -7,7 +7,6 @@ use mbe::syntax_node_to_token_tree; use rustc_hash::FxHashSet; use span::{AstIdMap, Span, SyntaxContextData, SyntaxContextId}; use syntax::{ast, AstNode, Parse, SyntaxElement, SyntaxError, SyntaxNode, SyntaxToken, T}; -use tracing::debug; use triomphe::Arc; use crate::{ @@ -158,7 +157,8 @@ pub fn expand_speculative( SyntaxFixupUndoInfo::NONE, ), MacroCallKind::Derive { derive_attr_index: index, .. } - | MacroCallKind::Attr { invoc_attr_index: index, .. } => { + | MacroCallKind::Attr { invoc_attr_index: index, .. } + | MacroCallKind::DeriveAttr { invoc_attr_index: index, .. } => { let censor = if let MacroCallKind::Derive { .. } = loc.kind { censor_derive_input(index, &ast::Adt::cast(speculative_args.clone())?) } else { @@ -347,31 +347,18 @@ type MacroArgResult = (Arc, SyntaxFixupUndoInfo, Span); /// Other wise return the [macro_arg] for the macro_call_id. /// /// This is not connected to the database so it does not cached the result. However, the inner [macro_arg] query is -/// -/// FIXME: Pick a better name -fn smart_macro_arg(db: &dyn ExpandDatabase, id: MacroCallId) -> MacroArgResult { - let loc = db.lookup_intern_macro_call(id); +fn macro_arg_considering_derives(db: &dyn ExpandDatabase, id: MacroCallId) -> MacroArgResult { + let macro_call_kind = db.lookup_intern_macro_call(id).kind; // FIXME: We called lookup_intern_macro_call twice. - match loc.kind { + match macro_call_kind { // Get the macro arg for the derive macro - MacroCallKind::Derive { derive_macro_id, .. } => { - debug!( - ?loc, - "{id:?} is a derive macro. Using the derive macro, id: {derive_macro_id:?}, attribute as the macro arg." - ); - db.macro_arg(derive_macro_id) - } + MacroCallKind::Derive { derive_macro_id, .. } => db.macro_arg(derive_macro_id), // Normal macro arg _ => db.macro_arg(id), } } -fn macro_arg( - db: &dyn ExpandDatabase, - id: MacroCallId, - // FIXME: consider the following by putting fixup info into eager call info args - // ) -> ValueResult, Arc>> { -) -> MacroArgResult { +fn macro_arg(db: &dyn ExpandDatabase, id: MacroCallId) -> MacroArgResult { let loc = db.lookup_intern_macro_call(id); if let MacroCallLoc { @@ -441,7 +428,9 @@ fn macro_arg( } return (Arc::new(tt), SyntaxFixupUndoInfo::NONE, span); } - MacroCallKind::Derive { ast_id, derive_attr_index, .. } => { + // MacroCallKind::Derive should not be here. As we are getting the argument for the derive macro + MacroCallKind::Derive { ast_id, derive_attr_index, .. } + | MacroCallKind::DeriveAttr { ast_id, invoc_attr_index: derive_attr_index } => { let node = ast_id.to_ptr(db).to_node(&root); let censor_derive_input = censor_derive_input(derive_attr_index, &node); let item_node = node.into(); @@ -553,7 +542,7 @@ fn macro_expand( let (ExpandResult { value: tt, err }, span) = match loc.def.kind { MacroDefKind::ProcMacro(..) => return db.expand_proc_macro(macro_call_id).map(CowArc::Arc), _ => { - let (macro_arg, undo_info, span) = smart_macro_arg(db, macro_call_id); + let (macro_arg, undo_info, span) = macro_arg_considering_derives(db, macro_call_id); let arg = &*macro_arg; let res = @@ -630,7 +619,7 @@ fn proc_macro_span(db: &dyn ExpandDatabase, ast: AstId) -> Span { fn expand_proc_macro(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandResult> { let loc = db.lookup_intern_macro_call(id); - let (macro_arg, undo_info, span) = smart_macro_arg(db, id); + let (macro_arg, undo_info, span) = macro_arg_considering_derives(db, id); let (expander, ast) = match loc.def.kind { MacroDefKind::ProcMacro(expander, _, ast) => (expander, ast), diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs index e7a313c68cdd..870c3dc0aacf 100644 --- a/crates/hir-expand/src/lib.rs +++ b/crates/hir-expand/src/lib.rs @@ -228,6 +228,10 @@ pub enum MacroCallKind { /// We will resolve the same token tree for all derive macros in the same derive attribute. derive_macro_id: MacroCallId, }, + DeriveAttr { + ast_id: AstId, + invoc_attr_index: AttrId, + }, Attr { ast_id: AstId, // FIXME: This shouldn't be here, we can derive this from `invoc_attr_index` @@ -515,7 +519,8 @@ impl MacroCallLoc { MacroCallKind::FnLike { ast_id, .. } => { ast_id.with_value(ast_id.to_node(db).syntax().clone()) } - MacroCallKind::Derive { ast_id, derive_attr_index, .. } => { + MacroCallKind::Derive { ast_id, derive_attr_index, .. } + | MacroCallKind::DeriveAttr { ast_id, invoc_attr_index: derive_attr_index } => { // FIXME: handle `cfg_attr` ast_id.with_value(ast_id.to_node(db)).map(|it| { collect_attrs(&it) @@ -549,7 +554,7 @@ impl MacroCallLoc { fn expand_to(&self) -> ExpandTo { match self.kind { MacroCallKind::FnLike { expand_to, .. } => expand_to, - MacroCallKind::Derive { .. } => ExpandTo::Items, + MacroCallKind::Derive { .. } | MacroCallKind::DeriveAttr { .. } => ExpandTo::Items, MacroCallKind::Attr { .. } if self.def.is_attribute_derive() => ExpandTo::Items, MacroCallKind::Attr { .. } => { // FIXME(stmt_expr_attributes) @@ -583,6 +588,7 @@ impl MacroCallKind { MacroCallKind::FnLike { .. } => "macro call", MacroCallKind::Derive { .. } => "derive macro", MacroCallKind::Attr { .. } => "attribute macro", + MacroCallKind::DeriveAttr { .. } => "derive attribute", } } @@ -591,6 +597,7 @@ impl MacroCallKind { match *self { MacroCallKind::FnLike { ast_id: InFile { file_id, .. }, .. } | MacroCallKind::Derive { ast_id: InFile { file_id, .. }, .. } + | MacroCallKind::DeriveAttr { ast_id: InFile { file_id, .. }, .. } | MacroCallKind::Attr { ast_id: InFile { file_id, .. }, .. } => file_id, } } @@ -598,7 +605,8 @@ impl MacroCallKind { pub fn erased_ast_id(&self) -> ErasedFileAstId { match *self { MacroCallKind::FnLike { ast_id: InFile { value, .. }, .. } => value.erase(), - MacroCallKind::Derive { ast_id: InFile { value, .. }, .. } => value.erase(), + MacroCallKind::Derive { ast_id: InFile { value, .. }, .. } + | MacroCallKind::DeriveAttr { ast_id: InFile { value, .. }, .. } => value.erase(), MacroCallKind::Attr { ast_id: InFile { value, .. }, .. } => value.erase(), } } @@ -619,7 +627,9 @@ impl MacroCallKind { let range = match kind { MacroCallKind::FnLike { ast_id, .. } => ast_id.to_ptr(db).text_range(), - MacroCallKind::Derive { ast_id, .. } => ast_id.to_ptr(db).text_range(), + MacroCallKind::Derive { ast_id, .. } | MacroCallKind::DeriveAttr { ast_id, .. } => { + ast_id.to_ptr(db).text_range() + } MacroCallKind::Attr { ast_id, .. } => ast_id.to_ptr(db).text_range(), }; @@ -665,6 +675,15 @@ impl MacroCallKind { .syntax() .text_range() } + MacroCallKind::DeriveAttr { ast_id, invoc_attr_index } => { + collect_attrs(&ast_id.to_node(db)) + .nth(invoc_attr_index.ast_index()) + .expect("missing attribute") + .1 + .expect_left("attribute macro is a doc comment?") + .syntax() + .text_range() + } }; FileRange { range, file_id } @@ -675,7 +694,7 @@ impl MacroCallKind { MacroCallKind::FnLike { ast_id, .. } => { ast_id.to_in_file_node(db).map(|it| Some(it.token_tree()?.syntax().clone())) } - MacroCallKind::Derive { ast_id, .. } => { + MacroCallKind::Derive { ast_id, .. } | MacroCallKind::DeriveAttr { ast_id, .. } => { ast_id.to_in_file_node(db).syntax().cloned().map(Some) } MacroCallKind::Attr { ast_id, .. } => { diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 2585d8e6f4e0..acd53e19e8b0 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -972,8 +972,7 @@ fn precise_macro_call_location( MacroKind::ProcMacro, ) } - // TODO: derive_macro_id - MacroCallKind::Derive { ast_id, derive_attr_index, derive_index, derive_macro_id } => { + MacroCallKind::Derive { ast_id, derive_attr_index, derive_index, .. } => { let node = ast_id.to_node(db.upcast()); // Compute the precise location of the macro name's token in the derive // list. @@ -1023,6 +1022,26 @@ fn precise_macro_call_location( MacroKind::Attr, ) } + MacroCallKind::DeriveAttr { ast_id, invoc_attr_index } => { + let node = ast_id.to_node(db.upcast()); + let attr = collect_attrs(&node) + .nth(invoc_attr_index.ast_index()) + .and_then(|x| Either::left(x.1)) + .unwrap_or_else(|| { + panic!("cannot find attribute #{}", invoc_attr_index.ast_index()) + }); + + ( + ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&attr))), + Some(attr.syntax().text_range()), + attr.path() + .and_then(|path| path.segment()) + .and_then(|seg| seg.name_ref()) + .as_ref() + .map(ToString::to_string), + MacroKind::Attr, + ) + } } } @@ -3710,8 +3729,7 @@ impl Impl { let macro_file = src.file_id.macro_file()?; let loc = macro_file.macro_call_id.lookup(db.upcast()); let (derive_attr, derive_index) = match loc.kind { - // TODO: derive_macro_id - MacroCallKind::Derive { ast_id, derive_attr_index, derive_index, derive_macro_id } => { + MacroCallKind::Derive { ast_id, derive_attr_index, derive_index, .. } => { let module_id = self.id.lookup(db.upcast()).container; ( db.crate_def_map(module_id.krate())[module_id.local_id] From 70f5344debc2d6ba0a24d832617e5415a9b0514c Mon Sep 17 00:00:00 2001 From: Wyatt Herkamp Date: Mon, 18 Mar 2024 13:20:16 -0400 Subject: [PATCH 5/8] macro_arg_considering_derives is now in ExpandDatabase and now takes in the MacroCallKind --- crates/hir-expand/src/db.rs | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs index 277c268086bc..ae2be4b71453 100644 --- a/crates/hir-expand/src/db.rs +++ b/crates/hir-expand/src/db.rs @@ -24,7 +24,8 @@ use crate::{ HirFileId, HirFileIdRepr, MacroCallId, MacroCallKind, MacroCallLoc, MacroDefId, MacroDefKind, MacroFileId, }; - +/// This is just to ensure the types of smart_macro_arg and macro_arg are the same +type MacroArgResult = (Arc, SyntaxFixupUndoInfo, Span); /// Total limit on the number of tokens produced by any macro invocation. /// /// If an invocation produces more tokens than this limit, it will not be stored in the database and @@ -98,7 +99,13 @@ pub trait ExpandDatabase: SourceDatabase { /// Lowers syntactic macro call to a token tree representation. That's a firewall /// query, only typing in the macro call itself changes the returned /// subtree. - fn macro_arg(&self, id: MacroCallId) -> (Arc, SyntaxFixupUndoInfo, Span); + fn macro_arg(&self, id: MacroCallId) -> MacroArgResult; + #[salsa::transparent] + fn macro_arg_considering_derives( + &self, + id: MacroCallId, + kind: &MacroCallKind, + ) -> MacroArgResult; /// Fetches the expander for this macro. #[salsa::transparent] #[salsa::invoke(TokenExpander::macro_expander)] @@ -339,20 +346,21 @@ pub(crate) fn parse_with_map( } } } -/// This is just to ensure the types of smart_macro_arg and macro_arg are the same -type MacroArgResult = (Arc, SyntaxFixupUndoInfo, Span); + /// Imagine the word smart in quotes. /// /// This resolves the [MacroCallId] to check if it is a derive macro if so get the [macro_arg] for the derive. /// Other wise return the [macro_arg] for the macro_call_id. /// /// This is not connected to the database so it does not cached the result. However, the inner [macro_arg] query is -fn macro_arg_considering_derives(db: &dyn ExpandDatabase, id: MacroCallId) -> MacroArgResult { - let macro_call_kind = db.lookup_intern_macro_call(id).kind; - // FIXME: We called lookup_intern_macro_call twice. - match macro_call_kind { +fn macro_arg_considering_derives( + db: &dyn ExpandDatabase, + id: MacroCallId, + kind: &MacroCallKind, +) -> MacroArgResult { + match kind { // Get the macro arg for the derive macro - MacroCallKind::Derive { derive_macro_id, .. } => db.macro_arg(derive_macro_id), + MacroCallKind::Derive { derive_macro_id, .. } => db.macro_arg(*derive_macro_id), // Normal macro arg _ => db.macro_arg(id), } @@ -542,7 +550,8 @@ fn macro_expand( let (ExpandResult { value: tt, err }, span) = match loc.def.kind { MacroDefKind::ProcMacro(..) => return db.expand_proc_macro(macro_call_id).map(CowArc::Arc), _ => { - let (macro_arg, undo_info, span) = macro_arg_considering_derives(db, macro_call_id); + let (macro_arg, undo_info, span) = + db.macro_arg_considering_derives(macro_call_id, &loc.kind); let arg = &*macro_arg; let res = @@ -619,7 +628,7 @@ fn proc_macro_span(db: &dyn ExpandDatabase, ast: AstId) -> Span { fn expand_proc_macro(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandResult> { let loc = db.lookup_intern_macro_call(id); - let (macro_arg, undo_info, span) = macro_arg_considering_derives(db, id); + let (macro_arg, undo_info, span) = db.macro_arg_considering_derives(id, &loc.kind.clone()); let (expander, ast) = match loc.def.kind { MacroDefKind::ProcMacro(expander, _, ast) => (expander, ast), From c376addfcc46f7eeec02d765027c8febc17a5825 Mon Sep 17 00:00:00 2001 From: Wyatt Herkamp Date: Thu, 21 Mar 2024 08:12:26 -0400 Subject: [PATCH 6/8] Cleanup --- crates/hir-def/src/nameres/collector.rs | 93 +++++++++---------------- 1 file changed, 34 insertions(+), 59 deletions(-) diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 27ad3ad1f4ca..6de967c2c64e 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -3,85 +3,60 @@ //! `DefCollector::collect` contains the fixed-point iteration loop which //! resolves imports and expands macros. -use std::{iter, mem}; - -use crate::attr::Attrs; -use crate::item_tree::Fields; -use crate::item_tree::FileItemTreeId; -use crate::item_tree::MacroCall; -use crate::item_tree::MacroRules; -use crate::item_tree::Mod; -use crate::item_tree::ModKind; -use crate::macro_call_as_call_id_with_eager; - -use crate::nameres::attr_resolution::derive_attr_macro_as_call_id; -use crate::nameres::mod_resolution::ModDir; - -use crate::item_tree::ItemTree; - -use crate::item_tree::TreeId; - -use crate::LocalModuleId; -use crate::{ - item_tree::{ExternCrate, ItemTreeId, Macro2, ModItem}, - nameres::{ - diagnostics::DefDiagnostic, proc_macro::parse_macro_name_and_helper_attrs, - sub_namespace_match, BuiltinShadowMode, DefMap, MacroSubNs, ModuleData, ModuleOrigin, - ResolveMode, - }, - path::ModPath, - per_ns::PerNs, - tt, - visibility::Visibility, - AstId, AstIdWithPath, ConstLoc, EnumLoc, EnumVariantLoc, ExternBlockLoc, ExternCrateId, - ExternCrateLoc, FunctionLoc, ImplLoc, Intern, ItemContainerId, Macro2Loc, MacroExpander, - MacroRulesLoc, MacroRulesLocFlags, ModuleDefId, ModuleId, StaticLoc, StructLoc, TraitAliasLoc, - TraitLoc, TypeAliasLoc, UnionLoc, UseLoc, -}; - -use std::{cmp::Ordering, ops::Not}; +use std::{cmp::Ordering, iter, mem, ops::Not}; +use base_db::{CrateId, Dependency, FileId}; +use cfg::{CfgExpr, CfgOptions}; use either::Either; use hir_expand::{ - builtin_attr_macro::find_builtin_attr, + attrs::{Attr, AttrId}, + builtin_attr_macro::{find_builtin_attr, BuiltinAttrExpander}, builtin_derive_macro::find_builtin_derive, builtin_fn_macro::find_builtin_macro, - name::{AsName, Name}, - HirFileId, InFile, -}; - -use itertools::Itertools; -use span::{ErasedFileAstId, FileAstId, Span, SyntaxContextId}; - -use hir_expand::{ - attrs::{Attr, AttrId}, - builtin_attr_macro::BuiltinAttrExpander, - name::name, + name::{name, AsName, Name}, proc_macro::CustomProcMacroExpander, - ExpandResult, ExpandTo, MacroCallId, MacroCallKind, MacroCallLoc, MacroDefId, MacroDefKind, + ExpandResult, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroCallLoc, + MacroDefId, MacroDefKind, }; -use itertools::izip; +use itertools::{izip, Itertools}; use la_arena::Idx; use limit::Limit; use rustc_hash::{FxHashMap, FxHashSet}; +use span::{Edition, ErasedFileAstId, FileAstId, Span, SyntaxContextId}; use stdx::always; use syntax::{ast, SmolStr}; use triomphe::Arc; use crate::{ + attr::Attrs, db::DefDatabase, item_scope::{ImportId, ImportOrExternCrate, ImportType, PerNsGlobImports}, - item_tree::{self, ImportKind, ItemTreeNode}, - macro_call_as_call_id, + item_tree::{ + self, ExternCrate, Fields, FileItemTreeId, ImportKind, ItemTree, ItemTreeId, ItemTreeNode, + Macro2, MacroCall, MacroRules, Mod, ModItem, ModKind, TreeId, + }, + macro_call_as_call_id, macro_call_as_call_id_with_eager, nameres::{ - attr_resolution::{attr_macro_as_call_id, derive_macro_as_call_id}, + attr_resolution::{ + attr_macro_as_call_id, derive_attr_macro_as_call_id, derive_macro_as_call_id, + }, + diagnostics::DefDiagnostic, + mod_resolution::ModDir, path_resolution::ReachedFixedPoint, - proc_macro::{ProcMacroDef, ProcMacroKind}, + proc_macro::{parse_macro_name_and_helper_attrs, ProcMacroDef, ProcMacroKind}, + sub_namespace_match, BuiltinShadowMode, DefMap, MacroSubNs, ModuleData, ModuleOrigin, + ResolveMode, }, - path::{ImportAlias, PathKind}, - visibility::RawVisibility, - AdtId, CrateRootModuleId, FunctionId, Lookup, Macro2Id, MacroId, MacroRulesId, ProcMacroId, - ProcMacroLoc, UnresolvedMacro, UseId, + path::{ImportAlias, ModPath, PathKind}, + per_ns::PerNs, + tt, + visibility::{RawVisibility, Visibility}, + AdtId, AstId, AstIdWithPath, ConstLoc, CrateRootModuleId, EnumLoc, EnumVariantLoc, + ExternBlockLoc, ExternCrateId, ExternCrateLoc, FunctionId, FunctionLoc, ImplLoc, Intern, + ItemContainerId, LocalModuleId, Lookup, Macro2Id, Macro2Loc, MacroExpander, MacroId, + MacroRulesId, MacroRulesLoc, MacroRulesLocFlags, ModuleDefId, ModuleId, ProcMacroId, + ProcMacroLoc, StaticLoc, StructLoc, TraitAliasLoc, TraitLoc, TypeAliasLoc, UnionLoc, + UnresolvedMacro, UseId, UseLoc, }; static GLOB_RECURSION_LIMIT: Limit = Limit::new(100); From 262e06f1ef5114a636502a93a01aa7836dd3d45f Mon Sep 17 00:00:00 2001 From: Wyatt Herkamp Date: Thu, 21 Mar 2024 12:50:58 -0400 Subject: [PATCH 7/8] Remove MacroCallKind::DeriveAttr --- crates/hir-def/src/nameres/attr_resolution.rs | 17 ++--------- crates/hir-def/src/nameres/collector.rs | 18 ++++-------- crates/hir-expand/src/cfg_process.rs | 10 +++++-- crates/hir-expand/src/db.rs | 19 ++++++++---- crates/hir-expand/src/lib.rs | 29 ++++--------------- crates/hir/src/lib.rs | 20 ------------- 6 files changed, 34 insertions(+), 79 deletions(-) diff --git a/crates/hir-def/src/nameres/attr_resolution.rs b/crates/hir-def/src/nameres/attr_resolution.rs index e2d83c8e3bfe..eb7f4c05ae21 100644 --- a/crates/hir-def/src/nameres/attr_resolution.rs +++ b/crates/hir-def/src/nameres/attr_resolution.rs @@ -3,7 +3,7 @@ use base_db::CrateId; use hir_expand::{ attrs::{Attr, AttrId, AttrInput}, - AstId, MacroCallId, MacroCallKind, MacroDefId, + MacroCallId, MacroCallKind, MacroDefId, }; use span::SyntaxContextId; use syntax::{ast, SmolStr}; @@ -98,20 +98,7 @@ impl DefMap { false } } -pub(super) fn derive_attr_macro_as_call_id( - db: &dyn DefDatabase, - item_attr: &AstId, - macro_attr: &Attr, - krate: CrateId, - def: MacroDefId, -) -> MacroCallId { - def.make_call( - db.upcast(), - krate, - MacroCallKind::DeriveAttr { ast_id: *item_attr, invoc_attr_index: macro_attr.id }, - macro_attr.ctxt, - ) -} + pub(super) fn attr_macro_as_call_id( db: &dyn DefDatabase, item_attr: &AstIdWithPath, diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 6de967c2c64e..2fae253d0a0a 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -37,9 +37,7 @@ use crate::{ }, macro_call_as_call_id, macro_call_as_call_id_with_eager, nameres::{ - attr_resolution::{ - attr_macro_as_call_id, derive_attr_macro_as_call_id, derive_macro_as_call_id, - }, + attr_resolution::{attr_macro_as_call_id, derive_macro_as_call_id}, diagnostics::DefDiagnostic, mod_resolution::ModDir, path_resolution::ReachedFixedPoint, @@ -1234,7 +1232,8 @@ impl DefCollector<'_> { Some(def) if def.is_attribute() => def, _ => return Resolved::No, }; - + let call_id = + attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def); if let MacroDefId { kind: MacroDefKind::BuiltInAttr( @@ -1266,13 +1265,7 @@ impl DefCollector<'_> { let ast_id = ast_id.with_value(ast_adt_id); // the call_id for the actual derive macro. This is used so all the derive proc macros can share a token tree - let call_id = derive_attr_macro_as_call_id( - self.db, - &ast_id, - attr, - self.def_map.krate, - def, - ); + match attr.parse_path_comma_token_tree(self.db.upcast()) { Some(derive_macros) => { let mut len = 0; @@ -1320,8 +1313,7 @@ impl DefCollector<'_> { return recollect_without(self); } - let call_id = - attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def); + // Skip #[test]/#[bench] expansion, which would merely result in more memory usage // due to duplicating functions into macro expansions if matches!( diff --git a/crates/hir-expand/src/cfg_process.rs b/crates/hir-expand/src/cfg_process.rs index 7e9ff5c85b54..db3558a84e9e 100644 --- a/crates/hir-expand/src/cfg_process.rs +++ b/crates/hir-expand/src/cfg_process.rs @@ -10,7 +10,7 @@ use syntax::{ use tracing::{debug, warn}; use tt::SmolStr; -use crate::{db::ExpandDatabase, MacroCallKind, MacroCallLoc}; +use crate::{db::ExpandDatabase, proc_macro::ProcMacroKind, MacroCallLoc, MacroDefKind}; fn check_cfg_attr(attr: &Attr, loc: &MacroCallLoc, db: &dyn ExpandDatabase) -> Option { if !attr.simple_name().as_deref().map(|v| v == "cfg")? { @@ -180,7 +180,13 @@ pub(crate) fn process_cfg_attrs( db: &dyn ExpandDatabase, ) -> Option> { // FIXME: #[cfg_eval] is not implemented. But it is not stable yet - if !matches!(loc.kind, MacroCallKind::Derive { .. } | MacroCallKind::DeriveAttr { .. }) { + let is_derive = match loc.def.kind { + MacroDefKind::BuiltInDerive(..) + | MacroDefKind::ProcMacro(_, ProcMacroKind::CustomDerive, _) => true, + MacroDefKind::BuiltInAttr(expander, _) => expander.is_derive(), + _ => false, + }; + if !is_derive { return None; } let mut remove = FxHashSet::default(); diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs index ae2be4b71453..6c22f79c81a0 100644 --- a/crates/hir-expand/src/db.rs +++ b/crates/hir-expand/src/db.rs @@ -164,8 +164,7 @@ pub fn expand_speculative( SyntaxFixupUndoInfo::NONE, ), MacroCallKind::Derive { derive_attr_index: index, .. } - | MacroCallKind::Attr { invoc_attr_index: index, .. } - | MacroCallKind::DeriveAttr { invoc_attr_index: index, .. } => { + | MacroCallKind::Attr { invoc_attr_index: index, .. } => { let censor = if let MacroCallKind::Derive { .. } = loc.kind { censor_derive_input(index, &ast::Adt::cast(speculative_args.clone())?) } else { @@ -436,9 +435,9 @@ fn macro_arg(db: &dyn ExpandDatabase, id: MacroCallId) -> MacroArgResult { } return (Arc::new(tt), SyntaxFixupUndoInfo::NONE, span); } + // MacroCallKind::Derive should not be here. As we are getting the argument for the derive macro - MacroCallKind::Derive { ast_id, derive_attr_index, .. } - | MacroCallKind::DeriveAttr { ast_id, invoc_attr_index: derive_attr_index } => { + MacroCallKind::Derive { ast_id, derive_attr_index, .. } => { let node = ast_id.to_ptr(db).to_node(&root); let censor_derive_input = censor_derive_input(derive_attr_index, &node); let item_node = node.into(); @@ -454,13 +453,23 @@ fn macro_arg(db: &dyn ExpandDatabase, id: MacroCallId) -> MacroArgResult { MacroCallKind::Attr { ast_id, invoc_attr_index, .. } => { let node = ast_id.to_ptr(db).to_node(&root); let attr_source = attr_source(invoc_attr_index, &node); + let span = map.span_for_range( attr_source .as_ref() .and_then(|it| it.path()) .map_or_else(|| node.syntax().text_range(), |it| it.syntax().text_range()), ); - (attr_source.into_iter().map(|it| it.syntax().clone().into()).collect(), node, span) + // If derive attribute we need to censor the derive input + if matches!(loc.def.kind, MacroDefKind::BuiltInAttr(expander, ..) if expander.is_derive()) + && ast::Adt::can_cast(node.syntax().kind()) + { + let adt = ast::Adt::cast(node.syntax().clone()).unwrap(); + let censor_derive_input = censor_derive_input(invoc_attr_index, &adt); + (censor_derive_input, node, span) + } else { + (attr_source.into_iter().map(|it| it.syntax().clone().into()).collect(), node, span) + } } }; diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs index 870c3dc0aacf..e7a313c68cdd 100644 --- a/crates/hir-expand/src/lib.rs +++ b/crates/hir-expand/src/lib.rs @@ -228,10 +228,6 @@ pub enum MacroCallKind { /// We will resolve the same token tree for all derive macros in the same derive attribute. derive_macro_id: MacroCallId, }, - DeriveAttr { - ast_id: AstId, - invoc_attr_index: AttrId, - }, Attr { ast_id: AstId, // FIXME: This shouldn't be here, we can derive this from `invoc_attr_index` @@ -519,8 +515,7 @@ impl MacroCallLoc { MacroCallKind::FnLike { ast_id, .. } => { ast_id.with_value(ast_id.to_node(db).syntax().clone()) } - MacroCallKind::Derive { ast_id, derive_attr_index, .. } - | MacroCallKind::DeriveAttr { ast_id, invoc_attr_index: derive_attr_index } => { + MacroCallKind::Derive { ast_id, derive_attr_index, .. } => { // FIXME: handle `cfg_attr` ast_id.with_value(ast_id.to_node(db)).map(|it| { collect_attrs(&it) @@ -554,7 +549,7 @@ impl MacroCallLoc { fn expand_to(&self) -> ExpandTo { match self.kind { MacroCallKind::FnLike { expand_to, .. } => expand_to, - MacroCallKind::Derive { .. } | MacroCallKind::DeriveAttr { .. } => ExpandTo::Items, + MacroCallKind::Derive { .. } => ExpandTo::Items, MacroCallKind::Attr { .. } if self.def.is_attribute_derive() => ExpandTo::Items, MacroCallKind::Attr { .. } => { // FIXME(stmt_expr_attributes) @@ -588,7 +583,6 @@ impl MacroCallKind { MacroCallKind::FnLike { .. } => "macro call", MacroCallKind::Derive { .. } => "derive macro", MacroCallKind::Attr { .. } => "attribute macro", - MacroCallKind::DeriveAttr { .. } => "derive attribute", } } @@ -597,7 +591,6 @@ impl MacroCallKind { match *self { MacroCallKind::FnLike { ast_id: InFile { file_id, .. }, .. } | MacroCallKind::Derive { ast_id: InFile { file_id, .. }, .. } - | MacroCallKind::DeriveAttr { ast_id: InFile { file_id, .. }, .. } | MacroCallKind::Attr { ast_id: InFile { file_id, .. }, .. } => file_id, } } @@ -605,8 +598,7 @@ impl MacroCallKind { pub fn erased_ast_id(&self) -> ErasedFileAstId { match *self { MacroCallKind::FnLike { ast_id: InFile { value, .. }, .. } => value.erase(), - MacroCallKind::Derive { ast_id: InFile { value, .. }, .. } - | MacroCallKind::DeriveAttr { ast_id: InFile { value, .. }, .. } => value.erase(), + MacroCallKind::Derive { ast_id: InFile { value, .. }, .. } => value.erase(), MacroCallKind::Attr { ast_id: InFile { value, .. }, .. } => value.erase(), } } @@ -627,9 +619,7 @@ impl MacroCallKind { let range = match kind { MacroCallKind::FnLike { ast_id, .. } => ast_id.to_ptr(db).text_range(), - MacroCallKind::Derive { ast_id, .. } | MacroCallKind::DeriveAttr { 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(), }; @@ -675,15 +665,6 @@ impl MacroCallKind { .syntax() .text_range() } - MacroCallKind::DeriveAttr { ast_id, invoc_attr_index } => { - collect_attrs(&ast_id.to_node(db)) - .nth(invoc_attr_index.ast_index()) - .expect("missing attribute") - .1 - .expect_left("attribute macro is a doc comment?") - .syntax() - .text_range() - } }; FileRange { range, file_id } @@ -694,7 +675,7 @@ impl MacroCallKind { MacroCallKind::FnLike { ast_id, .. } => { ast_id.to_in_file_node(db).map(|it| Some(it.token_tree()?.syntax().clone())) } - MacroCallKind::Derive { ast_id, .. } | MacroCallKind::DeriveAttr { ast_id, .. } => { + MacroCallKind::Derive { ast_id, .. } => { ast_id.to_in_file_node(db).syntax().cloned().map(Some) } MacroCallKind::Attr { ast_id, .. } => { diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index acd53e19e8b0..5be54feb466d 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1022,26 +1022,6 @@ fn precise_macro_call_location( MacroKind::Attr, ) } - MacroCallKind::DeriveAttr { ast_id, invoc_attr_index } => { - let node = ast_id.to_node(db.upcast()); - let attr = collect_attrs(&node) - .nth(invoc_attr_index.ast_index()) - .and_then(|x| Either::left(x.1)) - .unwrap_or_else(|| { - panic!("cannot find attribute #{}", invoc_attr_index.ast_index()) - }); - - ( - ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&attr))), - Some(attr.syntax().text_range()), - attr.path() - .and_then(|path| path.segment()) - .and_then(|seg| seg.name_ref()) - .as_ref() - .map(ToString::to_string), - MacroKind::Attr, - ) - } } } From c381d0fc64e02898ec77617a14e16431595b8906 Mon Sep 17 00:00:00 2001 From: Wyatt Herkamp Date: Thu, 21 Mar 2024 13:41:46 -0400 Subject: [PATCH 8/8] Review Updates --- crates/hir-def/src/nameres/collector.rs | 9 +++------ crates/hir-expand/src/db.rs | 21 ++++----------------- 2 files changed, 7 insertions(+), 23 deletions(-) diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 2fae253d0a0a..adb6f11419a1 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -1232,6 +1232,7 @@ impl DefCollector<'_> { Some(def) if def.is_attribute() => def, _ => return Resolved::No, }; + let call_id = attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def); if let MacroDefId { @@ -1264,7 +1265,6 @@ impl DefCollector<'_> { }; let ast_id = ast_id.with_value(ast_adt_id); - // the call_id for the actual derive macro. This is used so all the derive proc macros can share a token tree match attr.parse_path_comma_token_tree(self.db.upcast()) { Some(derive_macros) => { @@ -2305,7 +2305,7 @@ impl ModCollector<'_, '_> { fn collect_macro_call( &mut self, - &MacroCall { ref path, ast_id, expand_to, ctxt, .. }: &MacroCall, + &MacroCall { ref path, ast_id, expand_to, ctxt }: &MacroCall, container: ItemContainerId, ) { let ast_id = AstIdWithPath::new(self.file_id(), ast_id, ModPath::clone(path)); @@ -2444,10 +2444,7 @@ mod tests { use base_db::SourceDatabase; use test_fixture::WithFixture; - use crate::{ - nameres::{DefMapCrateData, ModuleData, ModuleOrigin}, - test_db::TestDB, - }; + use crate::{nameres::DefMapCrateData, test_db::TestDB}; use super::*; diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs index 6c22f79c81a0..5461c1c49a32 100644 --- a/crates/hir-expand/src/db.rs +++ b/crates/hir-expand/src/db.rs @@ -151,7 +151,7 @@ pub fn expand_speculative( let span_map = RealSpanMap::absolute(FileId::BOGUS); let span_map = SpanMapRef::RealSpanMap(&span_map); - let (_, _, span) = db.macro_arg(actual_macro_call); + let (_, _, span) = db.macro_arg_considering_derives(actual_macro_call, &loc.kind); // Build the subtree and token mapping for the speculative args let (mut tt, undo_info) = match loc.kind { @@ -346,8 +346,6 @@ pub(crate) fn parse_with_map( } } -/// Imagine the word smart in quotes. -/// /// This resolves the [MacroCallId] to check if it is a derive macro if so get the [macro_arg] for the derive. /// Other wise return the [macro_arg] for the macro_call_id. /// @@ -435,20 +433,9 @@ fn macro_arg(db: &dyn ExpandDatabase, id: MacroCallId) -> MacroArgResult { } return (Arc::new(tt), SyntaxFixupUndoInfo::NONE, span); } - // MacroCallKind::Derive should not be here. As we are getting the argument for the derive macro - MacroCallKind::Derive { ast_id, derive_attr_index, .. } => { - let node = ast_id.to_ptr(db).to_node(&root); - let censor_derive_input = censor_derive_input(derive_attr_index, &node); - let item_node = node.into(); - let attr_source = attr_source(derive_attr_index, &item_node); - // FIXME: This is wrong, this should point to the path of the derive attribute` - let span = - map.span_for_range(attr_source.as_ref().and_then(|it| it.path()).map_or_else( - || item_node.syntax().text_range(), - |it| it.syntax().text_range(), - )); - (censor_derive_input, item_node, span) + MacroCallKind::Derive { .. } => { + unreachable!("`ExpandDatabase::macro_arg` called with `MacroCallKind::Derive`") } MacroCallKind::Attr { ast_id, invoc_attr_index, .. } => { let node = ast_id.to_ptr(db).to_node(&root); @@ -637,7 +624,7 @@ fn proc_macro_span(db: &dyn ExpandDatabase, ast: AstId) -> Span { fn expand_proc_macro(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandResult> { let loc = db.lookup_intern_macro_call(id); - let (macro_arg, undo_info, span) = db.macro_arg_considering_derives(id, &loc.kind.clone()); + let (macro_arg, undo_info, span) = db.macro_arg_considering_derives(id, &loc.kind); let (expander, ast) = match loc.def.kind { MacroDefKind::ProcMacro(expander, _, ast) => (expander, ast),