From cf72b6232bbcabd06c094860f151332dde81c241 Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Thu, 20 Apr 2023 00:54:01 +0900 Subject: [PATCH 1/2] Resolve `$crate` in derive paths --- crates/hir-def/src/nameres/collector.rs | 19 +++++++------ crates/hir-def/src/nameres/tests/macros.rs | 23 ++++++++++++++++ crates/hir-expand/src/attrs.rs | 31 +++++++++++++++------- 3 files changed, 56 insertions(+), 17 deletions(-) diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 756a8f50490..e845cde73a2 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -14,6 +14,7 @@ builtin_attr_macro::find_builtin_attr, builtin_derive_macro::find_builtin_derive, builtin_fn_macro::find_builtin_macro, + hygiene::Hygiene, name::{name, AsName, Name}, proc_macro::ProcMacroExpander, ExpandResult, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroCallLoc, @@ -312,13 +313,14 @@ fn seed_with_top_level(&mut self) { } if *attr_name == hir_expand::name![feature] { - let features = - attr.parse_path_comma_token_tree().into_iter().flatten().filter_map( - |feat| match feat.segments() { - [name] => Some(name.to_smol_str()), - _ => None, - }, - ); + let features = attr + .parse_path_comma_token_tree(self.db.upcast(), Hygiene::new_unhygienic()) + .into_iter() + .flatten() + .filter_map(|feat| match feat.segments() { + [name] => Some(name.to_smol_str()), + _ => None, + }); self.def_map.unstable_features.extend(features); } @@ -1223,8 +1225,9 @@ fn resolve_macros(&mut self) -> ReachedFixedPoint { } }; let ast_id = ast_id.with_value(ast_adt_id); + let hygiene = Hygiene::new(self.db.upcast(), file_id); - match attr.parse_path_comma_token_tree() { + match attr.parse_path_comma_token_tree(self.db.upcast(), hygiene) { Some(derive_macros) => { let mut len = 0; for (idx, path) in derive_macros.enumerate() { diff --git a/crates/hir-def/src/nameres/tests/macros.rs b/crates/hir-def/src/nameres/tests/macros.rs index a4ccd14cbb4..6ee56c9368e 100644 --- a/crates/hir-def/src/nameres/tests/macros.rs +++ b/crates/hir-def/src/nameres/tests/macros.rs @@ -664,6 +664,29 @@ macro_rules! foo { ); } +#[test] +fn macro_dollar_crate_is_correct_in_derive_meta() { + let map = compute_crate_def_map( + r#" +//- minicore: derive, clone +//- /main.rs crate:main deps:lib +lib::foo!(); + +//- /lib.rs crate:lib +#[macro_export] +macro_rules! foo { + () => { + #[derive($crate::Clone)] + struct S; + } +} + +pub use core::clone::Clone; +"#, + ); + assert_eq!(map.modules[map.root].scope.impls().len(), 1); +} + #[test] fn expand_derive() { let map = compute_crate_def_map( diff --git a/crates/hir-expand/src/attrs.rs b/crates/hir-expand/src/attrs.rs index 7a61ca4f4d3..fd0254248ba 100644 --- a/crates/hir-expand/src/attrs.rs +++ b/crates/hir-expand/src/attrs.rs @@ -12,8 +12,7 @@ use crate::{ db::ExpandDatabase, hygiene::Hygiene, - mod_path::{ModPath, PathKind}, - name::AsName, + mod_path::ModPath, tt::{self, Subtree}, InFile, }; @@ -267,7 +266,11 @@ pub fn token_tree_value(&self) -> Option<&Subtree> { } /// Parses this attribute as a token tree consisting of comma separated paths. - pub fn parse_path_comma_token_tree(&self) -> Option + '_> { + pub fn parse_path_comma_token_tree<'a>( + &'a self, + db: &'a dyn ExpandDatabase, + hygiene: Hygiene, + ) -> Option + 'a> { let args = self.token_tree_value()?; if args.delimiter.kind != DelimiterKind::Parenthesis { @@ -276,15 +279,25 @@ pub fn parse_path_comma_token_tree(&self) -> Option Some(id.as_name()), - _ => None, - }); - Some(ModPath::from_segments(PathKind::Plain, segments)) + // FIXME: This is necessarily a hack. It'd be nice if we could avoid allocation here. + let subtree = tt::Subtree { + delimiter: tt::Delimiter::unspecified(), + token_trees: tts.into_iter().cloned().collect(), + }; + let (parse, _) = + mbe::token_tree_to_syntax_node(&subtree, mbe::TopEntryPoint::MetaItem); + let meta = ast::Meta::cast(parse.syntax_node())?; + // Only simple paths are allowed. + if meta.eq_token().is_some() || meta.expr().is_some() || meta.token_tree().is_some() + { + return None; + } + let path = meta.path()?; + ModPath::from_src(db, path, &hygiene) }); Some(paths) From 85e76542fe4302482ce91fcb1d193cd6f374676a Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Sat, 22 Apr 2023 18:22:29 +0900 Subject: [PATCH 2/2] Cache `Hygiene` in `DefCollector` --- crates/hir-def/src/nameres/collector.rs | 24 ++++++++++++++++++++++-- crates/hir-expand/src/attrs.rs | 4 ++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index e845cde73a2..ba93b2c70ef 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -123,6 +123,7 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, mut def_map: DefMap, tree_id: T from_glob_import: Default::default(), skip_attrs: Default::default(), is_proc_macro, + hygienes: FxHashMap::default(), }; if tree_id.is_block() { collector.seed_with_inner(tree_id); @@ -270,6 +271,12 @@ struct DefCollector<'a> { /// This also stores the attributes to skip when we resolve derive helpers and non-macro /// non-builtin attributes in general. skip_attrs: FxHashMap, AttrId>, + /// `Hygiene` cache, because `Hygiene` construction is expensive. + /// + /// Almost all paths should have been lowered to `ModPath` during `ItemTree` construction. + /// However, `DefCollector` still needs to lower paths in attributes, in particular those in + /// derive meta item list. + hygienes: FxHashMap, } impl DefCollector<'_> { @@ -313,8 +320,9 @@ fn seed_with_top_level(&mut self) { } if *attr_name == hir_expand::name![feature] { + let hygiene = &Hygiene::new_unhygienic(); let features = attr - .parse_path_comma_token_tree(self.db.upcast(), Hygiene::new_unhygienic()) + .parse_path_comma_token_tree(self.db.upcast(), hygiene) .into_iter() .flatten() .filter_map(|feat| match feat.segments() { @@ -1225,7 +1233,18 @@ fn resolve_macros(&mut self) -> ReachedFixedPoint { } }; let ast_id = ast_id.with_value(ast_adt_id); - let hygiene = Hygiene::new(self.db.upcast(), file_id); + + let extend_unhygenic; + let hygiene = if file_id.is_macro() { + self.hygienes + .entry(file_id) + .or_insert_with(|| Hygiene::new(self.db.upcast(), file_id)) + } else { + // Avoid heap allocation (`Hygiene` embraces `Arc`) and hash map entry + // when we're in an oridinary (non-macro) file. + extend_unhygenic = Hygiene::new_unhygienic(); + &extend_unhygenic + }; match attr.parse_path_comma_token_tree(self.db.upcast(), hygiene) { Some(derive_macros) => { @@ -2215,6 +2234,7 @@ fn do_collect_defs(db: &dyn DefDatabase, def_map: DefMap) -> DefMap { from_glob_import: Default::default(), skip_attrs: Default::default(), is_proc_macro: false, + hygienes: FxHashMap::default(), }; collector.seed_with_top_level(); collector.collect(); diff --git a/crates/hir-expand/src/attrs.rs b/crates/hir-expand/src/attrs.rs index fd0254248ba..17360090db1 100644 --- a/crates/hir-expand/src/attrs.rs +++ b/crates/hir-expand/src/attrs.rs @@ -269,7 +269,7 @@ pub fn token_tree_value(&self) -> Option<&Subtree> { pub fn parse_path_comma_token_tree<'a>( &'a self, db: &'a dyn ExpandDatabase, - hygiene: Hygiene, + hygiene: &'a Hygiene, ) -> Option + 'a> { let args = self.token_tree_value()?; @@ -297,7 +297,7 @@ pub fn parse_path_comma_token_tree<'a>( return None; } let path = meta.path()?; - ModPath::from_src(db, path, &hygiene) + ModPath::from_src(db, path, hygiene) }); Some(paths)