Skip to content

Commit 91bbc55

Browse files
VeykrilVeykril
committed
Check for derive attributes by item path, not derive identifier
1 parent 32f425d commit 91bbc55

File tree

9 files changed

+173
-206
lines changed

9 files changed

+173
-206
lines changed

‎crates/hir_def/src/attr.rs‎

Lines changed: 27 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use either::Either;
88
use hir_expand::{hygiene::Hygiene, name::AsName, AstId, InFile};
99
use itertools::Itertools;
1010
use la_arena::ArenaMap;
11-
use mbe::{syntax_node_to_token_tree, DelimiterKind};
11+
use mbe::{syntax_node_to_token_tree, DelimiterKind, Punct};
1212
use smallvec::{smallvec, SmallVec};
1313
use syntax::{
1414
ast::{self, AstNode, HasAttrs, IsString},
@@ -722,41 +722,35 @@ impl Attr {
722722
/// Parses this attribute as a `#[derive]`, returns an iterator that yields all contained paths
723723
/// to derive macros.
724724
///
725-
/// Returns `None` when the attribute is not a well-formed `#[derive]` attribute.
725+
/// Returns `None` when the attribute does not have a well-formed `#[derive]` attribute input.
726726
pub(crate) fn parse_derive(&self) -> Option<impl Iterator<Item = ModPath>> {
727-
if self.path.as_ident() != Some(&hir_expand::name![derive]) {
728-
return None;
729-
}
730-
731-
match self.input.as_deref() {
732-
Some(AttrInput::TokenTree(args, _)) => {
733-
let mut counter = 0;
734-
let paths = args
735-
.token_trees
736-
.iter()
737-
.group_by(move |tt| {
738-
match tt {
739-
tt::TokenTree::Leaf(tt::Leaf::Punct(p)) if p.char == ',' => {
740-
counter += 1;
741-
}
742-
_ => {}
743-
}
744-
counter
745-
})
746-
.into_iter()
747-
.map(|(_, tts)| {
748-
let segments = tts.filter_map(|tt| match tt {
749-
tt::TokenTree::Leaf(tt::Leaf::Ident(id)) => Some(id.as_name()),
750-
_ => None,
751-
});
752-
ModPath::from_segments(PathKind::Plain, segments)
753-
})
754-
.collect::<Vec<_>>();
755-
756-
Some(paths.into_iter())
727+
if let Some(AttrInput::TokenTree(args, _)) = self.input.as_deref() {
728+
if args.delimiter_kind() != Some(DelimiterKind::Parenthesis) {
729+
return None;
757730
}
758-
_ => None,
731+
let mut counter = 0;
732+
let paths = args
733+
.token_trees
734+
.iter()
735+
.group_by(move |tt| {
736+
if let tt::TokenTree::Leaf(tt::Leaf::Punct(Punct { char: ',', .. })) = tt {
737+
counter += 1;
738+
}
739+
counter
740+
})
741+
.into_iter()
742+
.map(|(_, tts)| {
743+
let segments = tts.filter_map(|tt| match tt {
744+
tt::TokenTree::Leaf(tt::Leaf::Ident(id)) => Some(id.as_name()),
745+
_ => None,
746+
});
747+
ModPath::from_segments(PathKind::Plain, segments)
748+
})
749+
.collect::<Vec<_>>();
750+
751+
return Some(paths.into_iter());
759752
}
753+
None
760754
}
761755

762756
pub fn string_value(&self) -> Option<&SmolStr> {

‎crates/hir_def/src/macro_expansion_tests/builtin_derive_macro.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,16 @@ fn test_copy_expand_in_core() {
2626
check(
2727
r#"
2828
#[rustc_builtin_macro]
29+
macro derive {}
30+
#[rustc_builtin_macro]
2931
macro Copy {}
3032
#[derive(Copy)]
3133
struct Foo;
3234
"#,
3335
expect![[r##"
3436
#[rustc_builtin_macro]
37+
macro derive {}
38+
#[rustc_builtin_macro]
3539
macro Copy {}
3640
#[derive(Copy)]
3741
struct Foo;

‎crates/hir_def/src/macro_expansion_tests/proc_macros.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ fn derive_censoring() {
3131
check(
3232
r#"
3333
//- proc_macros: derive_identity
34+
//- minicore:derive
3435
#[attr1]
3536
#[derive(Foo)]
3637
#[derive(proc_macros::DeriveIdentity)]

‎crates/hir_def/src/nameres/collector.rs‎

Lines changed: 89 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use base_db::{CrateId, Edition, FileId, ProcMacroId};
99
use cfg::{CfgExpr, CfgOptions};
1010
use hir_expand::{
1111
ast_id_map::FileAstId,
12-
builtin_attr_macro::{find_builtin_attr, is_builtin_test_or_bench_attr},
12+
builtin_attr_macro::find_builtin_attr,
1313
builtin_derive_macro::find_builtin_derive,
1414
builtin_fn_macro::find_builtin_macro,
1515
name::{name, AsName, Name},
@@ -781,7 +781,7 @@ impl DefCollector<'_> {
781781
}
782782

783783
fn resolve_extern_crate(&self, name: &Name) -> PerNs {
784-
if name == &name!(self) {
784+
if *name == name!(self) {
785785
cov_mark::hit!(extern_crate_self_as);
786786
let root = match self.def_map.block {
787787
Some(_) => {
@@ -1105,7 +1105,7 @@ impl DefCollector<'_> {
11051105
let mod_dir = self.mod_dirs[&directive.module_id].clone();
11061106
self.skip_attrs.insert(InFile::new(file_id, *mod_item), attr.id);
11071107
ModCollector {
1108-
def_collector: &mut *self,
1108+
def_collector: self,
11091109
macro_depth: directive.depth,
11101110
module_id: directive.module_id,
11111111
tree_id: TreeId::new(file_id, None),
@@ -1121,6 +1121,65 @@ impl DefCollector<'_> {
11211121
}
11221122
}
11231123

1124+
let def = resolver(ast_id.path.clone()).filter(MacroDefId::is_attribute);
1125+
if matches!(
1126+
def,
1127+
Some(MacroDefId { kind:MacroDefKind::BuiltInAttr(expander, _),.. })
1128+
if expander.is_derive()
1129+
) {
1130+
// Resolved to derive
1131+
let file_id = ast_id.ast_id.file_id;
1132+
let item_tree = self.db.file_item_tree(file_id);
1133+
1134+
let ast_id: FileAstId<ast::Item> = match *mod_item {
1135+
ModItem::Struct(it) => item_tree[it].ast_id.upcast(),
1136+
ModItem::Union(it) => item_tree[it].ast_id.upcast(),
1137+
ModItem::Enum(it) => item_tree[it].ast_id.upcast(),
1138+
_ => {
1139+
// Cannot use derive on this item.
1140+
// FIXME: diagnose
1141+
res = ReachedFixedPoint::No;
1142+
return false;
1143+
}
1144+
};
1145+
1146+
match attr.parse_derive() {
1147+
Some(derive_macros) => {
1148+
for path in derive_macros {
1149+
let ast_id = AstIdWithPath::new(file_id, ast_id, path);
1150+
self.unresolved_macros.push(MacroDirective {
1151+
module_id: directive.module_id,
1152+
depth: directive.depth + 1,
1153+
kind: MacroDirectiveKind::Derive {
1154+
ast_id,
1155+
derive_attr: attr.id,
1156+
},
1157+
});
1158+
}
1159+
}
1160+
None => {
1161+
// FIXME: diagnose
1162+
tracing::debug!("malformed derive: {:?}", attr);
1163+
}
1164+
}
1165+
1166+
let mod_dir = self.mod_dirs[&directive.module_id].clone();
1167+
self.skip_attrs.insert(InFile::new(file_id, *mod_item), attr.id);
1168+
ModCollector {
1169+
def_collector: &mut *self,
1170+
macro_depth: directive.depth,
1171+
module_id: directive.module_id,
1172+
tree_id: TreeId::new(file_id, None),
1173+
item_tree: &item_tree,
1174+
mod_dir,
1175+
}
1176+
.collect(&[*mod_item]);
1177+
1178+
// Remove the original directive since we resolved it.
1179+
res = ReachedFixedPoint::No;
1180+
return false;
1181+
}
1182+
11241183
if !self.db.enable_proc_attr_macros() {
11251184
return true;
11261185
}
@@ -1138,7 +1197,11 @@ impl DefCollector<'_> {
11381197

11391198
// Skip #[test]/#[bench] expansion, which would merely result in more memory usage
11401199
// due to duplicating functions into macro expansions
1141-
if is_builtin_test_or_bench_attr(loc.def) {
1200+
if matches!(
1201+
loc.def.kind,
1202+
MacroDefKind::BuiltInAttr(expander, _)
1203+
if expander.is_test() || expander.is_bench()
1204+
) {
11421205
let file_id = ast_id.ast_id.file_id;
11431206
let item_tree = self.db.file_item_tree(file_id);
11441207
let mod_dir = self.mod_dirs[&directive.module_id].clone();
@@ -1281,7 +1344,7 @@ impl DefCollector<'_> {
12811344
for directive in &self.unresolved_macros {
12821345
match &directive.kind {
12831346
MacroDirectiveKind::FnLike { ast_id, expand_to } => {
1284-
match macro_call_as_call_id(
1347+
let macro_call_as_call_id = macro_call_as_call_id(
12851348
ast_id,
12861349
*expand_to,
12871350
self.db,
@@ -1297,15 +1360,13 @@ impl DefCollector<'_> {
12971360
resolved_res.resolved_def.take_macros()
12981361
},
12991362
&mut |_| (),
1300-
) {
1301-
Ok(_) => (),
1302-
Err(UnresolvedMacro { path }) => {
1303-
self.def_map.diagnostics.push(DefDiagnostic::unresolved_macro_call(
1304-
directive.module_id,
1305-
ast_id.ast_id,
1306-
path,
1307-
));
1308-
}
1363+
);
1364+
if let Err(UnresolvedMacro { path }) = macro_call_as_call_id {
1365+
self.def_map.diagnostics.push(DefDiagnostic::unresolved_macro_call(
1366+
directive.module_id,
1367+
ast_id.ast_id,
1368+
path,
1369+
));
13091370
}
13101371
}
13111372
MacroDirectiveKind::Derive { .. } | MacroDirectiveKind::Attr { .. } => {
@@ -1747,26 +1808,23 @@ impl ModCollector<'_, '_> {
17471808
});
17481809

17491810
for attr in iter {
1750-
if attr.path.as_ident() == Some(&hir_expand::name![derive]) {
1751-
self.collect_derive(attr, mod_item);
1752-
} else if self.is_builtin_or_registered_attr(&attr.path) {
1811+
if self.is_builtin_or_registered_attr(&attr.path) {
17531812
continue;
1754-
} else {
1755-
tracing::debug!("non-builtin attribute {}", attr.path);
1813+
}
1814+
tracing::debug!("non-builtin attribute {}", attr.path);
17561815

1757-
let ast_id = AstIdWithPath::new(
1758-
self.file_id(),
1759-
mod_item.ast_id(self.item_tree),
1760-
attr.path.as_ref().clone(),
1761-
);
1762-
self.def_collector.unresolved_macros.push(MacroDirective {
1763-
module_id: self.module_id,
1764-
depth: self.macro_depth + 1,
1765-
kind: MacroDirectiveKind::Attr { ast_id, attr: attr.clone(), mod_item },
1766-
});
1816+
let ast_id = AstIdWithPath::new(
1817+
self.file_id(),
1818+
mod_item.ast_id(self.item_tree),
1819+
attr.path.as_ref().clone(),
1820+
);
1821+
self.def_collector.unresolved_macros.push(MacroDirective {
1822+
module_id: self.module_id,
1823+
depth: self.macro_depth + 1,
1824+
kind: MacroDirectiveKind::Attr { ast_id, attr: attr.clone(), mod_item },
1825+
});
17671826

1768-
return Err(());
1769-
}
1827+
return Err(());
17701828
}
17711829

17721830
Ok(())
@@ -1800,36 +1858,6 @@ impl ModCollector<'_, '_> {
18001858
false
18011859
}
18021860

1803-
fn collect_derive(&mut self, attr: &Attr, mod_item: ModItem) {
1804-
let ast_id: FileAstId<ast::Item> = match mod_item {
1805-
ModItem::Struct(it) => self.item_tree[it].ast_id.upcast(),
1806-
ModItem::Union(it) => self.item_tree[it].ast_id.upcast(),
1807-
ModItem::Enum(it) => self.item_tree[it].ast_id.upcast(),
1808-
_ => {
1809-
// Cannot use derive on this item.
1810-
// FIXME: diagnose
1811-
return;
1812-
}
1813-
};
1814-
1815-
match attr.parse_derive() {
1816-
Some(derive_macros) => {
1817-
for path in derive_macros {
1818-
let ast_id = AstIdWithPath::new(self.file_id(), ast_id, path);
1819-
self.def_collector.unresolved_macros.push(MacroDirective {
1820-
module_id: self.module_id,
1821-
depth: self.macro_depth + 1,
1822-
kind: MacroDirectiveKind::Derive { ast_id, derive_attr: attr.id },
1823-
});
1824-
}
1825-
}
1826-
None => {
1827-
// FIXME: diagnose
1828-
tracing::debug!("malformed derive: {:?}", attr);
1829-
}
1830-
}
1831-
}
1832-
18331861
/// If `attrs` registers a procedural macro, collects its definition.
18341862
fn collect_proc_macro_def(&mut self, func_name: &Name, ast_id: AstId<ast::Fn>, attrs: &Attrs) {
18351863
// FIXME: this should only be done in the root module of `proc-macro` crates, not everywhere

‎crates/hir_def/src/nameres/tests/macros.rs‎

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -669,19 +669,20 @@ pub struct bar;
669669
fn expand_derive() {
670670
let map = compute_crate_def_map(
671671
r#"
672-
//- /main.rs crate:main deps:core
673-
use core::Copy;
674-
675-
#[derive(Copy, core::Clone)]
676-
struct Foo;
672+
//- /main.rs crate:main deps:core
673+
use core::Copy;
677674
678-
//- /core.rs crate:core
679-
#[rustc_builtin_macro]
680-
pub macro Copy {}
675+
#[core::derive(Copy, core::Clone)]
676+
struct Foo;
681677
682-
#[rustc_builtin_macro]
683-
pub macro Clone {}
684-
"#,
678+
//- /core.rs crate:core
679+
#[rustc_builtin_macro]
680+
pub macro derive($item:item) {}
681+
#[rustc_builtin_macro]
682+
pub macro Copy {}
683+
#[rustc_builtin_macro]
684+
pub macro Clone {}
685+
"#,
685686
);
686687
assert_eq!(map.modules[map.root].scope.impls().len(), 2);
687688
}
@@ -712,17 +713,19 @@ fn builtin_derive_with_unresolved_attributes_fall_back() {
712713
cov_mark::check!(unresolved_attribute_fallback);
713714
let map = compute_crate_def_map(
714715
r#"
715-
//- /main.rs crate:main deps:core
716-
use core::Clone;
716+
//- /main.rs crate:main deps:core
717+
use core::{Clone, derive};
717718
718-
#[derive(Clone)]
719-
#[unresolved]
720-
struct Foo;
719+
#[derive(Clone)]
720+
#[unresolved]
721+
struct Foo;
721722
722-
//- /core.rs crate:core
723-
#[rustc_builtin_macro]
724-
pub macro Clone {}
725-
"#,
723+
//- /core.rs crate:core
724+
#[rustc_builtin_macro]
725+
pub macro derive($item:item) {}
726+
#[rustc_builtin_macro]
727+
pub macro Clone {}
728+
"#,
726729
);
727730
assert_eq!(map.modules[map.root].scope.impls().len(), 1);
728731
}
@@ -799,6 +802,9 @@ fn resolves_derive_helper() {
799802
check(
800803
r#"
801804
//- /main.rs crate:main deps:proc
805+
#[rustc_builtin_macro]
806+
pub macro derive($item:item) {}
807+
802808
#[derive(proc::Derive)]
803809
#[helper]
804810
#[unresolved]
@@ -811,6 +817,7 @@ fn derive() {}
811817
expect![[r#"
812818
crate
813819
S: t v
820+
derive: m
814821
"#]],
815822
);
816823
}

0 commit comments

Comments
 (0)