Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit 16c8dee

Browse files
committed
Auto merge of rust-lang#16835 - wyatt-herkamp:use_one_tt_for_a_derive, r=Veykril
Have Derive Attribute share a token tree with it's proc macros. The goal of this PR is to stop creating a token tree for each derive proc macro. This is done by giving the derive proc macros an id to its parent derive element. From running the analysis stat on the rust analyzer project I did see a small memory decrease. ``` Inference: 42.80s, 362ginstr, 591mb MIR lowering: 8.67s, 67ginstr, 291mb Mir failed bodies: 18 (0%) Data layouts: 85.81ms, 609minstr, 8mb Failed data layouts: 135 (6%) Const evaluation: 440.57ms, 5235minstr, 13mb Failed const evals: 1 (0%) Total: 64.16s, 552ginstr, 1731mb ``` After Change ``` Inference: 40.32s, 340ginstr, 593mb MIR lowering: 7.95s, 62ginstr, 292mb Mir failed bodies: 18 (0%) Data layouts: 87.97ms, 591minstr, 8mb Failed data layouts: 135 (6%) Const evaluation: 433.38ms, 5226minstr, 14mb Failed const evals: 1 (0%) Total: 60.49s, 523ginstr, 1680mb ``` Currently this breaks the expansion for the actual derive attribute. ## TODO - [x] Pick a better name for the function `smart_macro_arg`
2 parents a3d9625 + c381d0f commit 16c8dee

File tree

6 files changed

+80
-32
lines changed

6 files changed

+80
-32
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ pub(super) fn derive_macro_as_call_id(
136136
call_site: SyntaxContextId,
137137
krate: CrateId,
138138
resolver: impl Fn(path::ModPath) -> Option<(MacroId, MacroDefId)>,
139+
derive_macro_id: MacroCallId,
139140
) -> Result<(MacroId, MacroDefId, MacroCallId), UnresolvedMacro> {
140141
let (macro_id, def_id) = resolver(item_attr.path.clone())
141142
.filter(|(_, def_id)| def_id.is_derive())
@@ -147,6 +148,7 @@ pub(super) fn derive_macro_as_call_id(
147148
ast_id: item_attr.ast_id,
148149
derive_index: derive_pos,
149150
derive_attr_index,
151+
derive_macro_id,
150152
},
151153
call_site,
152154
);

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

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,8 @@ enum MacroDirectiveKind {
237237
derive_attr: AttrId,
238238
derive_pos: usize,
239239
ctxt: SyntaxContextId,
240+
/// The "parent" macro it is resolved to.
241+
derive_macro_id: MacroCallId,
240242
},
241243
Attr {
242244
ast_id: AstIdWithPath<ast::Item>,
@@ -1146,7 +1148,13 @@ impl DefCollector<'_> {
11461148
return Resolved::Yes;
11471149
}
11481150
}
1149-
MacroDirectiveKind::Derive { ast_id, derive_attr, derive_pos, ctxt: call_site } => {
1151+
MacroDirectiveKind::Derive {
1152+
ast_id,
1153+
derive_attr,
1154+
derive_pos,
1155+
ctxt: call_site,
1156+
derive_macro_id,
1157+
} => {
11501158
let id = derive_macro_as_call_id(
11511159
self.db,
11521160
ast_id,
@@ -1155,6 +1163,7 @@ impl DefCollector<'_> {
11551163
*call_site,
11561164
self.def_map.krate,
11571165
resolver,
1166+
*derive_macro_id,
11581167
);
11591168

11601169
if let Ok((macro_id, def_id, call_id)) = id {
@@ -1224,6 +1233,8 @@ impl DefCollector<'_> {
12241233
_ => return Resolved::No,
12251234
};
12261235

1236+
let call_id =
1237+
attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def);
12271238
if let MacroDefId {
12281239
kind:
12291240
MacroDefKind::BuiltInAttr(
@@ -1252,6 +1263,7 @@ impl DefCollector<'_> {
12521263
return recollect_without(self);
12531264
}
12541265
};
1266+
12551267
let ast_id = ast_id.with_value(ast_adt_id);
12561268

12571269
match attr.parse_path_comma_token_tree(self.db.upcast()) {
@@ -1267,6 +1279,7 @@ impl DefCollector<'_> {
12671279
derive_attr: attr.id,
12681280
derive_pos: idx,
12691281
ctxt: call_site.ctx,
1282+
derive_macro_id: call_id,
12701283
},
12711284
container: directive.container,
12721285
});
@@ -1301,10 +1314,6 @@ impl DefCollector<'_> {
13011314
return recollect_without(self);
13021315
}
13031316

1304-
// Not resolved to a derive helper or the derive attribute, so try to treat as a normal attribute.
1305-
let call_id =
1306-
attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def);
1307-
13081317
// Skip #[test]/#[bench] expansion, which would merely result in more memory usage
13091318
// due to duplicating functions into macro expansions
13101319
if matches!(
@@ -1460,13 +1469,20 @@ impl DefCollector<'_> {
14601469
));
14611470
}
14621471
}
1463-
MacroDirectiveKind::Derive { ast_id, derive_attr, derive_pos, ctxt: _ } => {
1472+
MacroDirectiveKind::Derive {
1473+
ast_id,
1474+
derive_attr,
1475+
derive_pos,
1476+
derive_macro_id,
1477+
..
1478+
} => {
14641479
self.def_map.diagnostics.push(DefDiagnostic::unresolved_macro_call(
14651480
directive.module_id,
14661481
MacroCallKind::Derive {
14671482
ast_id: ast_id.ast_id,
14681483
derive_attr_index: *derive_attr,
14691484
derive_index: *derive_pos as u32,
1485+
derive_macro_id: *derive_macro_id,
14701486
},
14711487
ast_id.path.clone(),
14721488
));

crates/hir-expand/src/cfg_process.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use syntax::{
1010
use tracing::{debug, warn};
1111
use tt::SmolStr;
1212

13-
use crate::{db::ExpandDatabase, MacroCallKind, MacroCallLoc};
13+
use crate::{db::ExpandDatabase, proc_macro::ProcMacroKind, MacroCallLoc, MacroDefKind};
1414

1515
fn check_cfg_attr(attr: &Attr, loc: &MacroCallLoc, db: &dyn ExpandDatabase) -> Option<bool> {
1616
if !attr.simple_name().as_deref().map(|v| v == "cfg")? {
@@ -180,7 +180,13 @@ pub(crate) fn process_cfg_attrs(
180180
db: &dyn ExpandDatabase,
181181
) -> Option<FxHashSet<SyntaxElement>> {
182182
// FIXME: #[cfg_eval] is not implemented. But it is not stable yet
183-
if !matches!(loc.kind, MacroCallKind::Derive { .. }) {
183+
let is_derive = match loc.def.kind {
184+
MacroDefKind::BuiltInDerive(..)
185+
| MacroDefKind::ProcMacro(_, ProcMacroKind::CustomDerive, _) => true,
186+
MacroDefKind::BuiltInAttr(expander, _) => expander.is_derive(),
187+
_ => false,
188+
};
189+
if !is_derive {
184190
return None;
185191
}
186192
let mut remove = FxHashSet::default();

crates/hir-expand/src/db.rs

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ use crate::{
2424
HirFileId, HirFileIdRepr, MacroCallId, MacroCallKind, MacroCallLoc, MacroDefId, MacroDefKind,
2525
MacroFileId,
2626
};
27-
27+
/// This is just to ensure the types of smart_macro_arg and macro_arg are the same
28+
type MacroArgResult = (Arc<tt::Subtree>, SyntaxFixupUndoInfo, Span);
2829
/// Total limit on the number of tokens produced by any macro invocation.
2930
///
3031
/// If an invocation produces more tokens than this limit, it will not be stored in the database and
@@ -98,7 +99,13 @@ pub trait ExpandDatabase: SourceDatabase {
9899
/// Lowers syntactic macro call to a token tree representation. That's a firewall
99100
/// query, only typing in the macro call itself changes the returned
100101
/// subtree.
101-
fn macro_arg(&self, id: MacroCallId) -> (Arc<tt::Subtree>, SyntaxFixupUndoInfo, Span);
102+
fn macro_arg(&self, id: MacroCallId) -> MacroArgResult;
103+
#[salsa::transparent]
104+
fn macro_arg_considering_derives(
105+
&self,
106+
id: MacroCallId,
107+
kind: &MacroCallKind,
108+
) -> MacroArgResult;
102109
/// Fetches the expander for this macro.
103110
#[salsa::transparent]
104111
#[salsa::invoke(TokenExpander::macro_expander)]
@@ -144,7 +151,7 @@ pub fn expand_speculative(
144151
let span_map = RealSpanMap::absolute(FileId::BOGUS);
145152
let span_map = SpanMapRef::RealSpanMap(&span_map);
146153

147-
let (_, _, span) = db.macro_arg(actual_macro_call);
154+
let (_, _, span) = db.macro_arg_considering_derives(actual_macro_call, &loc.kind);
148155

149156
// Build the subtree and token mapping for the speculative args
150157
let (mut tt, undo_info) = match loc.kind {
@@ -339,12 +346,24 @@ pub(crate) fn parse_with_map(
339346
}
340347
}
341348

342-
// FIXME: for derive attributes, this will return separate copies of the same structures! Though
343-
// they may differ in spans due to differing call sites...
344-
fn macro_arg(
349+
/// This resolves the [MacroCallId] to check if it is a derive macro if so get the [macro_arg] for the derive.
350+
/// Other wise return the [macro_arg] for the macro_call_id.
351+
///
352+
/// This is not connected to the database so it does not cached the result. However, the inner [macro_arg] query is
353+
fn macro_arg_considering_derives(
345354
db: &dyn ExpandDatabase,
346355
id: MacroCallId,
347-
) -> (Arc<tt::Subtree>, SyntaxFixupUndoInfo, Span) {
356+
kind: &MacroCallKind,
357+
) -> MacroArgResult {
358+
match kind {
359+
// Get the macro arg for the derive macro
360+
MacroCallKind::Derive { derive_macro_id, .. } => db.macro_arg(*derive_macro_id),
361+
// Normal macro arg
362+
_ => db.macro_arg(id),
363+
}
364+
}
365+
366+
fn macro_arg(db: &dyn ExpandDatabase, id: MacroCallId) -> MacroArgResult {
348367
let loc = db.lookup_intern_macro_call(id);
349368

350369
if let MacroCallLoc {
@@ -414,29 +433,30 @@ fn macro_arg(
414433
}
415434
return (Arc::new(tt), SyntaxFixupUndoInfo::NONE, span);
416435
}
417-
MacroCallKind::Derive { ast_id, derive_attr_index, .. } => {
418-
let node = ast_id.to_ptr(db).to_node(&root);
419-
let censor_derive_input = censor_derive_input(derive_attr_index, &node);
420-
let item_node = node.into();
421-
let attr_source = attr_source(derive_attr_index, &item_node);
422-
// FIXME: This is wrong, this should point to the path of the derive attribute`
423-
let span =
424-
map.span_for_range(attr_source.as_ref().and_then(|it| it.path()).map_or_else(
425-
|| item_node.syntax().text_range(),
426-
|it| it.syntax().text_range(),
427-
));
428-
(censor_derive_input, item_node, span)
436+
// MacroCallKind::Derive should not be here. As we are getting the argument for the derive macro
437+
MacroCallKind::Derive { .. } => {
438+
unreachable!("`ExpandDatabase::macro_arg` called with `MacroCallKind::Derive`")
429439
}
430440
MacroCallKind::Attr { ast_id, invoc_attr_index, .. } => {
431441
let node = ast_id.to_ptr(db).to_node(&root);
432442
let attr_source = attr_source(invoc_attr_index, &node);
443+
433444
let span = map.span_for_range(
434445
attr_source
435446
.as_ref()
436447
.and_then(|it| it.path())
437448
.map_or_else(|| node.syntax().text_range(), |it| it.syntax().text_range()),
438449
);
439-
(attr_source.into_iter().map(|it| it.syntax().clone().into()).collect(), node, span)
450+
// If derive attribute we need to censor the derive input
451+
if matches!(loc.def.kind, MacroDefKind::BuiltInAttr(expander, ..) if expander.is_derive())
452+
&& ast::Adt::can_cast(node.syntax().kind())
453+
{
454+
let adt = ast::Adt::cast(node.syntax().clone()).unwrap();
455+
let censor_derive_input = censor_derive_input(invoc_attr_index, &adt);
456+
(censor_derive_input, node, span)
457+
} else {
458+
(attr_source.into_iter().map(|it| it.syntax().clone().into()).collect(), node, span)
459+
}
440460
}
441461
};
442462

@@ -526,7 +546,8 @@ fn macro_expand(
526546
let (ExpandResult { value: tt, err }, span) = match loc.def.kind {
527547
MacroDefKind::ProcMacro(..) => return db.expand_proc_macro(macro_call_id).map(CowArc::Arc),
528548
_ => {
529-
let (macro_arg, undo_info, span) = db.macro_arg(macro_call_id);
549+
let (macro_arg, undo_info, span) =
550+
db.macro_arg_considering_derives(macro_call_id, &loc.kind);
530551

531552
let arg = &*macro_arg;
532553
let res =
@@ -603,7 +624,7 @@ fn proc_macro_span(db: &dyn ExpandDatabase, ast: AstId<ast::Fn>) -> Span {
603624

604625
fn expand_proc_macro(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandResult<Arc<tt::Subtree>> {
605626
let loc = db.lookup_intern_macro_call(id);
606-
let (macro_arg, undo_info, span) = db.macro_arg(id);
627+
let (macro_arg, undo_info, span) = db.macro_arg_considering_derives(id, &loc.kind);
607628

608629
let (expander, ast) = match loc.def.kind {
609630
MacroDefKind::ProcMacro(expander, _, ast) => (expander, ast),

crates/hir-expand/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,9 @@ pub enum MacroCallKind {
224224
derive_attr_index: AttrId,
225225
/// Index of the derive macro in the derive attribute
226226
derive_index: u32,
227+
/// The "parent" macro call.
228+
/// We will resolve the same token tree for all derive macros in the same derive attribute.
229+
derive_macro_id: MacroCallId,
227230
},
228231
Attr {
229232
ast_id: AstId<ast::Item>,

crates/hir/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -972,7 +972,7 @@ fn precise_macro_call_location(
972972
MacroKind::ProcMacro,
973973
)
974974
}
975-
MacroCallKind::Derive { ast_id, derive_attr_index, derive_index } => {
975+
MacroCallKind::Derive { ast_id, derive_attr_index, derive_index, .. } => {
976976
let node = ast_id.to_node(db.upcast());
977977
// Compute the precise location of the macro name's token in the derive
978978
// list.
@@ -3708,7 +3708,7 @@ impl Impl {
37083708
let macro_file = src.file_id.macro_file()?;
37093709
let loc = macro_file.macro_call_id.lookup(db.upcast());
37103710
let (derive_attr, derive_index) = match loc.kind {
3711-
MacroCallKind::Derive { ast_id, derive_attr_index, derive_index } => {
3711+
MacroCallKind::Derive { ast_id, derive_attr_index, derive_index, .. } => {
37123712
let module_id = self.id.lookup(db.upcast()).container;
37133713
(
37143714
db.crate_def_map(module_id.krate())[module_id.local_id]

0 commit comments

Comments
 (0)