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: Resolve $crate in derive paths #14610

Merged
merged 2 commits into from
Apr 22, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
39 changes: 31 additions & 8 deletions crates/hir-def/src/nameres/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use hir_expand::{
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,
Expand Down Expand Up @@ -122,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);
Expand Down Expand Up @@ -269,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<InFile<ModItem>, 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<HirFileId, Hygiene>,
}

impl DefCollector<'_> {
Expand Down Expand Up @@ -312,13 +320,15 @@ impl DefCollector<'_> {
}

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 hygiene = &Hygiene::new_unhygienic();
let features = attr
.parse_path_comma_token_tree(self.db.upcast(), hygiene)
.into_iter()
.flatten()
.filter_map(|feat| match feat.segments() {
[name] => Some(name.to_smol_str()),
_ => None,
});
self.def_map.unstable_features.extend(features);
}

Expand Down Expand Up @@ -1224,7 +1234,19 @@ impl DefCollector<'_> {
};
let ast_id = ast_id.with_value(ast_adt_id);

match attr.parse_path_comma_token_tree() {
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) => {
let mut len = 0;
for (idx, path) in derive_macros.enumerate() {
Expand Down Expand Up @@ -2212,6 +2234,7 @@ mod tests {
from_glob_import: Default::default(),
skip_attrs: Default::default(),
is_proc_macro: false,
hygienes: FxHashMap::default(),
};
collector.seed_with_top_level();
collector.collect();
Expand Down
23 changes: 23 additions & 0 deletions crates/hir-def/src/nameres/tests/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,29 @@ pub struct bar;
);
}

#[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(
Expand Down
31 changes: 22 additions & 9 deletions crates/hir-expand/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ use syntax::{ast, match_ast, AstNode, SmolStr, SyntaxNode};
use crate::{
db::ExpandDatabase,
hygiene::Hygiene,
mod_path::{ModPath, PathKind},
name::AsName,
mod_path::ModPath,
tt::{self, Subtree},
InFile,
};
Expand Down Expand Up @@ -267,7 +266,11 @@ impl Attr {
}

/// Parses this attribute as a token tree consisting of comma separated paths.
pub fn parse_path_comma_token_tree(&self) -> Option<impl Iterator<Item = ModPath> + '_> {
pub fn parse_path_comma_token_tree<'a>(
&'a self,
db: &'a dyn ExpandDatabase,
hygiene: &'a Hygiene,
) -> Option<impl Iterator<Item = ModPath> + 'a> {
let args = self.token_tree_value()?;

if args.delimiter.kind != DelimiterKind::Parenthesis {
Expand All @@ -276,15 +279,25 @@ impl Attr {
let paths = args
.token_trees
.split(|tt| matches!(tt, tt::TokenTree::Leaf(tt::Leaf::Punct(Punct { char: ',', .. }))))
.filter_map(|tts| {
.filter_map(move |tts| {
if tts.is_empty() {
return None;
}
let segments = tts.iter().filter_map(|tt| match tt {
tt::TokenTree::Leaf(tt::Leaf::Ident(id)) => 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)
Expand Down