Skip to content

Commit 9bf974d

Browse files
Fix bug with attribute macro expansion
See the comment in the code. Previously it wasn't present because we were'nt stripping derives appearing before attribute macros, so we kept the derive and therefore the helper resolved as well. But we should strip the derives, as rustc does.
1 parent 3df6a32 commit 9bf974d

File tree

2 files changed

+74
-3
lines changed

2 files changed

+74
-3
lines changed

crates/hir-def/src/macro_expansion_tests/proc_macros.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,3 +316,28 @@ use proc_macros::disallow_cfg;
316316
expect![[r#""#]],
317317
);
318318
}
319+
320+
#[test]
321+
fn derive_helpers_are_ignored() {
322+
check(
323+
r#"
324+
//- proc_macros: identity, helper_should_be_ignored, helper_should_be_ignored_derive
325+
//- minicore: derive
326+
use proc_macros::{identity, helper_should_be_ignored, HelperShouldBeIgnoredDerive};
327+
328+
#[derive(HelperShouldBeIgnoredDerive)]
329+
#[helper_should_be_ignored]
330+
#[identity]
331+
struct Foo;
332+
"#,
333+
expect![[r#"
334+
use proc_macros::{identity, helper_should_be_ignored, HelperShouldBeIgnoredDerive};
335+
336+
#[derive(HelperShouldBeIgnoredDerive)]
337+
#[helper_should_be_ignored]
338+
#[identity]
339+
struct Foo;
340+
341+
#[helper_should_be_ignored] struct Foo;"#]],
342+
);
343+
}

crates/hir-def/src/nameres/collector.rs

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1238,7 +1238,17 @@ impl<'db> DefCollector<'db> {
12381238
let mut macros = mem::take(&mut self.unresolved_macros);
12391239
let mut resolved = Vec::new();
12401240
let mut push_resolved = |directive: &MacroDirective<'_>, call_id| {
1241-
resolved.push((directive.module_id, directive.depth, directive.container, call_id));
1241+
let attr_macro_item = match &directive.kind {
1242+
MacroDirectiveKind::Attr { ast_id, .. } => Some(ast_id.ast_id),
1243+
MacroDirectiveKind::FnLike { .. } | MacroDirectiveKind::Derive { .. } => None,
1244+
};
1245+
resolved.push((
1246+
directive.module_id,
1247+
directive.depth,
1248+
directive.container,
1249+
call_id,
1250+
attr_macro_item,
1251+
));
12421252
};
12431253

12441254
#[derive(PartialEq, Eq)]
@@ -1530,8 +1540,14 @@ impl<'db> DefCollector<'db> {
15301540
self.def_map.modules[module_id].scope.add_macro_invoc(ptr.map(|(_, it)| it), call_id);
15311541
}
15321542

1533-
for (module_id, depth, container, macro_call_id) in resolved {
1534-
self.collect_macro_expansion(module_id, macro_call_id, depth, container);
1543+
for (module_id, depth, container, macro_call_id, attr_macro_item) in resolved {
1544+
self.collect_macro_expansion(
1545+
module_id,
1546+
macro_call_id,
1547+
depth,
1548+
container,
1549+
attr_macro_item,
1550+
);
15351551
}
15361552

15371553
res
@@ -1543,6 +1559,7 @@ impl<'db> DefCollector<'db> {
15431559
macro_call_id: MacroCallId,
15441560
depth: usize,
15451561
container: ItemContainerId,
1562+
attr_macro_item: Option<AstId<ast::Item>>,
15461563
) {
15471564
if depth > self.def_map.recursion_limit() as usize {
15481565
cov_mark::hit!(macro_expansion_overflow);
@@ -1553,6 +1570,34 @@ impl<'db> DefCollector<'db> {
15531570

15541571
let item_tree = self.db.file_item_tree(file_id);
15551572

1573+
// Derive helpers that are in scope for an item are also in scope for attribute macro expansions
1574+
// of that item (but not derive or fn like macros).
1575+
// FIXME: This is a hack. The proper way to do this is by having a chain of derive helpers scope,
1576+
// where the next scope in the chain is the parent hygiene context of the span. Unfortunately
1577+
// it's difficult to implement with our current name resolution and hygiene system.
1578+
// This hack is also incorrect since it ignores item in blocks. But the main reason to bring derive
1579+
// helpers into scope in this case is to help with:
1580+
// ```
1581+
// #[derive(DeriveWithHelper)]
1582+
// #[helper]
1583+
// #[attr_macro]
1584+
// struct Foo;
1585+
// ```
1586+
// Where `attr_macro`'s input will include `#[helper]` but not the derive, and it will likely therefore
1587+
// also include it in its output. Therefore I hope not supporting blocks is fine at least for now.
1588+
if let Some(attr_macro_item) = attr_macro_item
1589+
&& let Some(derive_helpers) = self.def_map.derive_helpers_in_scope.get(&attr_macro_item)
1590+
{
1591+
let derive_helpers = derive_helpers.clone();
1592+
for item in item_tree.top_level_items() {
1593+
self.def_map
1594+
.derive_helpers_in_scope
1595+
.entry(InFile::new(file_id, item.ast_id()))
1596+
.or_default()
1597+
.extend(derive_helpers.iter().cloned());
1598+
}
1599+
}
1600+
15561601
let mod_dir = if macro_call_id.is_include_macro(self.db) {
15571602
ModDir::root()
15581603
} else {
@@ -2451,6 +2496,7 @@ impl ModCollector<'_, '_> {
24512496
call_id,
24522497
self.macro_depth + 1,
24532498
container,
2499+
None,
24542500
);
24552501
}
24562502

0 commit comments

Comments
 (0)