From cf54b8c3a4d7aee0bcc57e7a509c74ee0b451faf Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Mon, 7 Nov 2022 21:24:17 +0900 Subject: [PATCH 1/2] Parse and collect derive helpers for builtin derive macros --- crates/hir-def/src/nameres/collector.rs | 28 +++++++- crates/hir-def/src/nameres/proc_macro.rs | 78 ++++++++++++---------- crates/hir-def/src/nameres/tests/macros.rs | 22 ++++++ crates/test-utils/src/minicore.rs | 2 +- 4 files changed, 93 insertions(+), 37 deletions(-) diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index b0dd01f9dbe..f21d674f20d 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -40,7 +40,7 @@ diagnostics::DefDiagnostic, mod_resolution::ModDir, path_resolution::ReachedFixedPoint, - proc_macro::{ProcMacroDef, ProcMacroKind}, + proc_macro::{parse_macro_name_and_helper_attrs, ProcMacroDef, ProcMacroKind}, BuiltinShadowMode, DefMap, ModuleData, ModuleOrigin, ResolveMode, }, path::{ImportAlias, ModPath, PathKind}, @@ -2005,6 +2005,7 @@ fn collect_macro_def(&mut self, id: FileItemTreeId, module: ModuleId) let ast_id = InFile::new(self.file_id(), mac.ast_id.upcast()); // Case 1: builtin macros + let mut helpers_opt = None; let attrs = self.item_tree.attrs(self.def_collector.db, krate, ModItem::from(id).into()); let expander = if attrs.by_key("rustc_builtin_macro").exists() { if let Some(expander) = find_builtin_macro(&mac.name) { @@ -2013,6 +2014,25 @@ fn collect_macro_def(&mut self, id: FileItemTreeId, module: ModuleId) Either::Right(it) => MacroExpander::BuiltInEager(it), } } else if let Some(expander) = find_builtin_derive(&mac.name) { + if let Some(attr) = attrs.by_key("rustc_builtin_macro").tt_values().next() { + // NOTE: The item *may* have both `#[rustc_builtin_macro]` and `#[proc_macro_derive]`, + // in which case rustc ignores the helper attributes from the latter, but it + // "doesn't make sense in practice" (see rust-lang/rust#87027). + if let Some((name, helpers)) = + parse_macro_name_and_helper_attrs(&attr.token_trees) + { + // NOTE: rustc overrides the name if the macro name if it's different from the + // macro name, but we assume it isn't as there's no such case yet. FIXME if + // the following assertion fails. + stdx::always!( + name == mac.name, + "built-in macro {} has #[rustc_builtin_macro] which declares different name {}", + mac.name, + name + ); + helpers_opt = Some(helpers); + } + } MacroExpander::BuiltInDerive(expander) } else if let Some(expander) = find_builtin_attr(&mac.name) { MacroExpander::BuiltInAttr(expander) @@ -2037,6 +2057,12 @@ fn collect_macro_def(&mut self, id: FileItemTreeId, module: ModuleId) macro_id, &self.item_tree[mac.visibility], ); + if let Some(helpers) = helpers_opt { + self.def_collector + .def_map + .exported_derives + .insert(macro_id_to_def_id(self.def_collector.db, macro_id.into()), helpers); + } } fn collect_macro_call(&mut self, mac: &MacroCall, container: ItemContainerId) { diff --git a/crates/hir-def/src/nameres/proc_macro.rs b/crates/hir-def/src/nameres/proc_macro.rs index 52b79cd0fdd..06b23392cfe 100644 --- a/crates/hir-def/src/nameres/proc_macro.rs +++ b/crates/hir-def/src/nameres/proc_macro.rs @@ -37,45 +37,53 @@ pub fn parse_proc_macro_decl(&self, func_name: &Name) -> Option { Some(ProcMacroDef { name: func_name.clone(), kind: ProcMacroKind::Attr }) } else if self.by_key("proc_macro_derive").exists() { let derive = self.by_key("proc_macro_derive").tt_values().next()?; + let def = parse_macro_name_and_helper_attrs(&derive.token_trees) + .map(|(name, helpers)| ProcMacroDef { name, kind: ProcMacroKind::CustomDerive { helpers } }); - match &*derive.token_trees { - // `#[proc_macro_derive(Trait)]` - [TokenTree::Leaf(Leaf::Ident(trait_name))] => Some(ProcMacroDef { - name: trait_name.as_name(), - kind: ProcMacroKind::CustomDerive { helpers: Box::new([]) }, - }), - - // `#[proc_macro_derive(Trait, attributes(helper1, helper2, ...))]` - [ - TokenTree::Leaf(Leaf::Ident(trait_name)), - TokenTree::Leaf(Leaf::Punct(comma)), - TokenTree::Leaf(Leaf::Ident(attributes)), - TokenTree::Subtree(helpers) - ] if comma.char == ',' && attributes.text == "attributes" => - { - let helpers = helpers.token_trees.iter() - .filter(|tt| !matches!(tt, TokenTree::Leaf(Leaf::Punct(comma)) if comma.char == ',')) - .map(|tt| { - match tt { - TokenTree::Leaf(Leaf::Ident(helper)) => Some(helper.as_name()), - _ => None - } - }) - .collect::>>()?; - - Some(ProcMacroDef { - name: trait_name.as_name(), - kind: ProcMacroKind::CustomDerive { helpers }, - }) - } - - _ => { - tracing::trace!("malformed `#[proc_macro_derive]`: {}", derive); - None - } + if def.is_none() { + tracing::trace!("malformed `#[proc_macro_derive]`: {}", derive); } + + def } else { None } } } + +// This fn is intended for `#[proc_macro_derive(..)]` and `#[rustc_builtin_macro(..)]`, which have +// the same strucuture. +#[rustfmt::skip] +pub(crate) fn parse_macro_name_and_helper_attrs(tt: &[TokenTree]) -> Option<(Name, Box<[Name]>)> { + match tt { + // `#[proc_macro_derive(Trait)]` + // `#[rustc_builtin_macro(Trait)]` + [TokenTree::Leaf(Leaf::Ident(trait_name))] => Some((trait_name.as_name(), Box::new([]))), + + // `#[proc_macro_derive(Trait, attributes(helper1, helper2, ...))]` + // `#[rustc_builtin_macro(Trait, attributes(helper1, helper2, ...))]` + [ + TokenTree::Leaf(Leaf::Ident(trait_name)), + TokenTree::Leaf(Leaf::Punct(comma)), + TokenTree::Leaf(Leaf::Ident(attributes)), + TokenTree::Subtree(helpers) + ] if comma.char == ',' && attributes.text == "attributes" => + { + let helpers = helpers + .token_trees + .iter() + .filter( + |tt| !matches!(tt, TokenTree::Leaf(Leaf::Punct(comma)) if comma.char == ','), + ) + .map(|tt| match tt { + TokenTree::Leaf(Leaf::Ident(helper)) => Some(helper.as_name()), + _ => None, + }) + .collect::>>()?; + + Some((trait_name.as_name(), helpers)) + } + + _ => None, + } +} diff --git a/crates/hir-def/src/nameres/tests/macros.rs b/crates/hir-def/src/nameres/tests/macros.rs index 3ece1379ad7..fe0ad4f3863 100644 --- a/crates/hir-def/src/nameres/tests/macros.rs +++ b/crates/hir-def/src/nameres/tests/macros.rs @@ -822,6 +822,28 @@ fn derive() {} ); } +#[test] +fn resolves_derive_helper_rustc_builtin_macro() { + cov_mark::check!(resolved_derive_helper); + // This is NOT the correct usage of `default` helper attribute, but we don't resolve helper + // attributes on non mod items in hir nameres. + check( + r#" +//- minicore: derive, default +#[derive(Default)] +#[default] +enum E { + A, + B, +} +"#, + expect![[r#" + crate + E: t + "#]], + ); +} + #[test] fn unresolved_attr_with_cfg_attr_hang() { // Another regression test for https://github.com/rust-lang/rust-analyzer/issues/8905 diff --git a/crates/test-utils/src/minicore.rs b/crates/test-utils/src/minicore.rs index 40a330a6bd1..013fd6b4fdf 100644 --- a/crates/test-utils/src/minicore.rs +++ b/crates/test-utils/src/minicore.rs @@ -112,7 +112,7 @@ pub trait Default: Sized { fn default() -> Self; } // region:derive - #[rustc_builtin_macro] + #[rustc_builtin_macro(Default, attributes(default))] pub macro Default($item:item) {} // endregion:derive } From 051c6598bedadcb65710276d1ee853e3743730a3 Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Tue, 6 Dec 2022 15:53:03 +0900 Subject: [PATCH 2/2] Resolve macro2's derive helpers in IDE layer Macro2's generally don't have derive helpers, but currently builtin derive macros are declared with macro2 syntax, which can have derive helpers. --- crates/hir-def/src/data.rs | 17 ++++++++++++++++- crates/hir/src/lib.rs | 6 ++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs index 0e7acda4a75..b78ab71ef21 100644 --- a/crates/hir-def/src/data.rs +++ b/crates/hir-def/src/data.rs @@ -13,7 +13,9 @@ intern::Interned, item_tree::{self, AssocItem, FnFlags, ItemTree, ItemTreeId, ModItem, Param, TreeId}, nameres::{ - attr_resolution::ResolvedAttr, diagnostics::DefDiagnostic, proc_macro::ProcMacroKind, + attr_resolution::ResolvedAttr, + diagnostics::DefDiagnostic, + proc_macro::{parse_macro_name_and_helper_attrs, ProcMacroKind}, DefMap, }, type_ref::{TraitRef, TypeBound, TypeRef}, @@ -348,6 +350,10 @@ pub fn attribute_calls(&self) -> impl Iterator, MacroCa pub struct Macro2Data { pub name: Name, pub visibility: RawVisibility, + // It's a bit wasteful as currently this is only for builtin `Default` derive macro, but macro2 + // are rarely used in practice so I think it's okay for now. + /// Derive helpers, if this is a derive rustc_builtin_macro + pub helpers: Option>, } impl Macro2Data { @@ -356,9 +362,18 @@ pub(crate) fn macro2_data_query(db: &dyn DefDatabase, makro: Macro2Id) -> Arc Macro { pub fn name(&self, db: &dyn HirDatabase) -> Name { match self.derive { - MacroId::Macro2Id(_) => None, + MacroId::Macro2Id(it) => { + db.macro2_data(it).helpers.as_deref().and_then(|it| it.get(self.idx)).cloned() + } MacroId::MacroRulesId(_) => None, MacroId::ProcMacroId(proc_macro) => db .proc_macro_data(proc_macro) .helpers - .as_ref() + .as_deref() .and_then(|it| it.get(self.idx)) .cloned(), }