Skip to content

Commit

Permalink
Check for derive attributes by item path, not derive identifier
Browse files Browse the repository at this point in the history
  • Loading branch information
@Veykril
Veykril committed Nov 17, 2021
1 parent 32f425d commit 91bbc55
Show file tree
Hide file tree
Showing 9 changed files with 173 additions and 206 deletions.
60 changes: 27 additions & 33 deletions crates/hir_def/src/attr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use either::Either;
use hir_expand::{hygiene::Hygiene, name::AsName, AstId, InFile};
use itertools::Itertools;
use la_arena::ArenaMap;
use mbe::{syntax_node_to_token_tree, DelimiterKind};
use mbe::{syntax_node_to_token_tree, DelimiterKind, Punct};
use smallvec::{smallvec, SmallVec};
use syntax::{
ast::{self, AstNode, HasAttrs, IsString},
Expand Down Expand Up @@ -722,41 +722,35 @@ impl Attr {
/// Parses this attribute as a `#[derive]`, returns an iterator that yields all contained paths
/// to derive macros.
///
/// Returns `None` when the attribute is not a well-formed `#[derive]` attribute.
/// Returns `None` when the attribute does not have a well-formed `#[derive]` attribute input.
pub(crate) fn parse_derive(&self) -> Option<impl Iterator<Item = ModPath>> {
if self.path.as_ident() != Some(&hir_expand::name![derive]) {
return None;
}

match self.input.as_deref() {
Some(AttrInput::TokenTree(args, _)) => {
let mut counter = 0;
let paths = args
.token_trees
.iter()
.group_by(move |tt| {
match tt {
tt::TokenTree::Leaf(tt::Leaf::Punct(p)) if p.char == ',' => {
counter += 1;
}
_ => {}
}
counter
})
.into_iter()
.map(|(_, tts)| {
let segments = tts.filter_map(|tt| match tt {
tt::TokenTree::Leaf(tt::Leaf::Ident(id)) => Some(id.as_name()),
_ => None,
});
ModPath::from_segments(PathKind::Plain, segments)
})
.collect::<Vec<_>>();

Some(paths.into_iter())
if let Some(AttrInput::TokenTree(args, _)) = self.input.as_deref() {
if args.delimiter_kind() != Some(DelimiterKind::Parenthesis) {
return None;
}
_ => None,
let mut counter = 0;
let paths = args
.token_trees
.iter()
.group_by(move |tt| {
if let tt::TokenTree::Leaf(tt::Leaf::Punct(Punct { char: ',', .. })) = tt {
counter += 1;
}
counter
})
.into_iter()
.map(|(_, tts)| {
let segments = tts.filter_map(|tt| match tt {
tt::TokenTree::Leaf(tt::Leaf::Ident(id)) => Some(id.as_name()),
_ => None,
});
ModPath::from_segments(PathKind::Plain, segments)
})
.collect::<Vec<_>>();

return Some(paths.into_iter());
}
None
}

pub fn string_value(&self) -> Option<&SmolStr> {
Expand Down
4 changes: 4 additions & 0 deletions crates/hir_def/src/macro_expansion_tests/builtin_derive_macro.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,16 @@ fn test_copy_expand_in_core() {
check(
r#"
#[rustc_builtin_macro]
macro derive {}
#[rustc_builtin_macro]
macro Copy {}
#[derive(Copy)]
struct Foo;
"#,
expect![[r##"
#[rustc_builtin_macro]
macro derive {}
#[rustc_builtin_macro]
macro Copy {}
#[derive(Copy)]
struct Foo;
Expand Down
1 change: 1 addition & 0 deletions crates/hir_def/src/macro_expansion_tests/proc_macros.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ fn derive_censoring() {
check(
r#"
//- proc_macros: derive_identity
//- minicore:derive
#[attr1]
#[derive(Foo)]
#[derive(proc_macros::DeriveIdentity)]
Expand Down
150 changes: 89 additions & 61 deletions crates/hir_def/src/nameres/collector.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use base_db::{CrateId, Edition, FileId, ProcMacroId};
use cfg::{CfgExpr, CfgOptions};
use hir_expand::{
ast_id_map::FileAstId,
builtin_attr_macro::{find_builtin_attr, is_builtin_test_or_bench_attr},
builtin_attr_macro::find_builtin_attr,
builtin_derive_macro::find_builtin_derive,
builtin_fn_macro::find_builtin_macro,
name::{name, AsName, Name},
Expand Down Expand Up @@ -781,7 +781,7 @@ impl DefCollector<'_> {
}

fn resolve_extern_crate(&self, name: &Name) -> PerNs {
if name == &name!(self) {
if *name == name!(self) {
cov_mark::hit!(extern_crate_self_as);
let root = match self.def_map.block {
Some(_) => {
Expand Down Expand Up @@ -1105,7 +1105,7 @@ impl DefCollector<'_> {
let mod_dir = self.mod_dirs[&directive.module_id].clone();
self.skip_attrs.insert(InFile::new(file_id, *mod_item), attr.id);
ModCollector {
def_collector: &mut *self,
def_collector: self,
macro_depth: directive.depth,
module_id: directive.module_id,
tree_id: TreeId::new(file_id, None),
Expand All @@ -1121,6 +1121,65 @@ impl DefCollector<'_> {
}
}

let def = resolver(ast_id.path.clone()).filter(MacroDefId::is_attribute);
if matches!(
def,
Some(MacroDefId { kind:MacroDefKind::BuiltInAttr(expander, _),.. })
if expander.is_derive()
) {
// Resolved to derive
let file_id = ast_id.ast_id.file_id;
let item_tree = self.db.file_item_tree(file_id);

let ast_id: FileAstId<ast::Item> = match *mod_item {
ModItem::Struct(it) => item_tree[it].ast_id.upcast(),
ModItem::Union(it) => item_tree[it].ast_id.upcast(),
ModItem::Enum(it) => item_tree[it].ast_id.upcast(),
_ => {
// Cannot use derive on this item.
// FIXME: diagnose
res = ReachedFixedPoint::No;
return false;
}
};

match attr.parse_derive() {
Some(derive_macros) => {
for path in derive_macros {
let ast_id = AstIdWithPath::new(file_id, ast_id, path);
self.unresolved_macros.push(MacroDirective {
module_id: directive.module_id,
depth: directive.depth + 1,
kind: MacroDirectiveKind::Derive {
ast_id,
derive_attr: attr.id,
},
});
}
}
None => {
// FIXME: diagnose
tracing::debug!("malformed derive: {:?}", attr);
}
}

let mod_dir = self.mod_dirs[&directive.module_id].clone();
self.skip_attrs.insert(InFile::new(file_id, *mod_item), attr.id);
ModCollector {
def_collector: &mut *self,
macro_depth: directive.depth,
module_id: directive.module_id,
tree_id: TreeId::new(file_id, None),
item_tree: &item_tree,
mod_dir,
}
.collect(&[*mod_item]);

// Remove the original directive since we resolved it.
res = ReachedFixedPoint::No;
return false;
}

if !self.db.enable_proc_attr_macros() {
return true;
}
Expand All @@ -1138,7 +1197,11 @@ impl DefCollector<'_> {

// Skip #[test]/#[bench] expansion, which would merely result in more memory usage
// due to duplicating functions into macro expansions
if is_builtin_test_or_bench_attr(loc.def) {
if matches!(
loc.def.kind,
MacroDefKind::BuiltInAttr(expander, _)
if expander.is_test() || expander.is_bench()
) {
let file_id = ast_id.ast_id.file_id;
let item_tree = self.db.file_item_tree(file_id);
let mod_dir = self.mod_dirs[&directive.module_id].clone();
Expand Down Expand Up @@ -1281,7 +1344,7 @@ impl DefCollector<'_> {
for directive in &self.unresolved_macros {
match &directive.kind {
MacroDirectiveKind::FnLike { ast_id, expand_to } => {
match macro_call_as_call_id(
let macro_call_as_call_id = macro_call_as_call_id(
ast_id,
*expand_to,
self.db,
Expand All @@ -1297,15 +1360,13 @@ impl DefCollector<'_> {
resolved_res.resolved_def.take_macros()
},
&mut |_| (),
) {
Ok(_) => (),
Err(UnresolvedMacro { path }) => {
self.def_map.diagnostics.push(DefDiagnostic::unresolved_macro_call(
directive.module_id,
ast_id.ast_id,
path,
));
}
);
if let Err(UnresolvedMacro { path }) = macro_call_as_call_id {
self.def_map.diagnostics.push(DefDiagnostic::unresolved_macro_call(
directive.module_id,
ast_id.ast_id,
path,
));
}
}
MacroDirectiveKind::Derive { .. } | MacroDirectiveKind::Attr { .. } => {
Expand Down Expand Up @@ -1747,26 +1808,23 @@ impl ModCollector<'_, '_> {
});

for attr in iter {
if attr.path.as_ident() == Some(&hir_expand::name![derive]) {
self.collect_derive(attr, mod_item);
} else if self.is_builtin_or_registered_attr(&attr.path) {
if self.is_builtin_or_registered_attr(&attr.path) {
continue;
} else {
tracing::debug!("non-builtin attribute {}", attr.path);
}
tracing::debug!("non-builtin attribute {}", attr.path);

let ast_id = AstIdWithPath::new(
self.file_id(),
mod_item.ast_id(self.item_tree),
attr.path.as_ref().clone(),
);
self.def_collector.unresolved_macros.push(MacroDirective {
module_id: self.module_id,
depth: self.macro_depth + 1,
kind: MacroDirectiveKind::Attr { ast_id, attr: attr.clone(), mod_item },
});
let ast_id = AstIdWithPath::new(
self.file_id(),
mod_item.ast_id(self.item_tree),
attr.path.as_ref().clone(),
);
self.def_collector.unresolved_macros.push(MacroDirective {
module_id: self.module_id,
depth: self.macro_depth + 1,
kind: MacroDirectiveKind::Attr { ast_id, attr: attr.clone(), mod_item },
});

return Err(());
}
return Err(());
}

Ok(())
Expand Down Expand Up @@ -1800,36 +1858,6 @@ impl ModCollector<'_, '_> {
false
}

fn collect_derive(&mut self, attr: &Attr, mod_item: ModItem) {
let ast_id: FileAstId<ast::Item> = match mod_item {
ModItem::Struct(it) => self.item_tree[it].ast_id.upcast(),
ModItem::Union(it) => self.item_tree[it].ast_id.upcast(),
ModItem::Enum(it) => self.item_tree[it].ast_id.upcast(),
_ => {
// Cannot use derive on this item.
// FIXME: diagnose
return;
}
};

match attr.parse_derive() {
Some(derive_macros) => {
for path in derive_macros {
let ast_id = AstIdWithPath::new(self.file_id(), ast_id, path);
self.def_collector.unresolved_macros.push(MacroDirective {
module_id: self.module_id,
depth: self.macro_depth + 1,
kind: MacroDirectiveKind::Derive { ast_id, derive_attr: attr.id },
});
}
}
None => {
// FIXME: diagnose
tracing::debug!("malformed derive: {:?}", attr);
}
}
}

/// If `attrs` registers a procedural macro, collects its definition.
fn collect_proc_macro_def(&mut self, func_name: &Name, ast_id: AstId<ast::Fn>, attrs: &Attrs) {
// FIXME: this should only be done in the root module of `proc-macro` crates, not everywhere
Expand Down
47 changes: 27 additions & 20 deletions crates/hir_def/src/nameres/tests/macros.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -669,19 +669,20 @@ pub struct bar;
fn expand_derive() {
let map = compute_crate_def_map(
r#"
//- /main.rs crate:main deps:core
use core::Copy;
#[derive(Copy, core::Clone)]
struct Foo;
//- /main.rs crate:main deps:core
use core::Copy;
//- /core.rs crate:core
#[rustc_builtin_macro]
pub macro Copy {}
#[core::derive(Copy, core::Clone)]
struct Foo;
#[rustc_builtin_macro]
pub macro Clone {}
"#,
//- /core.rs crate:core
#[rustc_builtin_macro]
pub macro derive($item:item) {}
#[rustc_builtin_macro]
pub macro Copy {}
#[rustc_builtin_macro]
pub macro Clone {}
"#,
);
assert_eq!(map.modules[map.root].scope.impls().len(), 2);
}
Expand Down Expand Up @@ -712,17 +713,19 @@ fn builtin_derive_with_unresolved_attributes_fall_back() {
cov_mark::check!(unresolved_attribute_fallback);
let map = compute_crate_def_map(
r#"
//- /main.rs crate:main deps:core
use core::Clone;
//- /main.rs crate:main deps:core
use core::{Clone, derive};
#[derive(Clone)]
#[unresolved]
struct Foo;
#[derive(Clone)]
#[unresolved]
struct Foo;
//- /core.rs crate:core
#[rustc_builtin_macro]
pub macro Clone {}
"#,
//- /core.rs crate:core
#[rustc_builtin_macro]
pub macro derive($item:item) {}
#[rustc_builtin_macro]
pub macro Clone {}
"#,
);
assert_eq!(map.modules[map.root].scope.impls().len(), 1);
}
Expand Down Expand Up @@ -799,6 +802,9 @@ fn resolves_derive_helper() {
check(
r#"
//- /main.rs crate:main deps:proc
#[rustc_builtin_macro]
pub macro derive($item:item) {}
#[derive(proc::Derive)]
#[helper]
#[unresolved]
Expand All @@ -811,6 +817,7 @@ fn derive() {}
expect![[r#"
crate
S: t v
derive: m
"#]],
);
}
Expand Down
Loading

0 comments on commit 91bbc55

Please sign in to comment.