Skip to content

Commit 87e0bbc

Browse files
VeykrilVeykril
committed
Stronger typing for macro_arg query
1 parent e8182a5 commit 87e0bbc

File tree

2 files changed

+150
-163
lines changed

2 files changed

+150
-163
lines changed

‎crates/hir-expand/src/db.rs

Lines changed: 149 additions & 163 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,16 @@ use limit::Limit;
66
use mbe::{syntax_node_to_token_tree, ValueResult};
77
use rustc_hash::FxHashSet;
88
use span::{AstIdMap, SyntaxContextData, SyntaxContextId};
9-
use syntax::{
10-
ast::{self, HasAttrs},
11-
AstNode, Parse, SyntaxElement, SyntaxError, SyntaxNode, SyntaxToken, T,
12-
};
9+
use syntax::{ast, AstNode, Parse, SyntaxElement, SyntaxError, SyntaxNode, SyntaxToken, T};
1310
use triomphe::Arc;
1411

1512
use crate::{
16-
attrs::collect_attrs,
13+
attrs::{collect_attrs, AttrId},
1714
builtin_attr_macro::pseudo_derive_attr_expansion,
1815
builtin_fn_macro::EagerExpander,
1916
cfg_process,
2017
declarative::DeclarativeMacroExpander,
21-
fixup::{self, reverse_fixups, SyntaxFixupUndoInfo},
18+
fixup::{self, SyntaxFixupUndoInfo},
2219
hygiene::{span_with_call_site_ctxt, span_with_def_site_ctxt, span_with_mixed_site_ctxt},
2320
proc_macro::ProcMacros,
2421
span_map::{RealSpanMap, SpanMap, SpanMapRef},
@@ -149,8 +146,14 @@ pub fn expand_speculative(
149146
mbe::syntax_node_to_token_tree(speculative_args, span_map, loc.call_site),
150147
SyntaxFixupUndoInfo::NONE,
151148
),
152-
MacroCallKind::Derive { .. } | MacroCallKind::Attr { .. } => {
153-
let censor = censor_for_macro_input(&loc, speculative_args);
149+
MacroCallKind::Derive { derive_attr_index: index, .. }
150+
| MacroCallKind::Attr { invoc_attr_index: index, .. } => {
151+
let censor = if let MacroCallKind::Derive { .. } = loc.kind {
152+
censor_derive_input(index, &ast::Adt::cast(speculative_args.clone())?)
153+
} else {
154+
censor_attr_input(index, &ast::Item::cast(speculative_args.clone())?)
155+
};
156+
154157
let censor_cfg =
155158
cfg_process::process_cfg_attrs(speculative_args, &loc, db).unwrap_or_default();
156159
let mut fixups = fixup::fixup_syntax(span_map, speculative_args, loc.call_site);
@@ -324,7 +327,8 @@ pub(crate) fn parse_with_map(
324327
}
325328
}
326329

327-
// FIXME: for derive attributes, this will return separate copies of the same structures!
330+
// FIXME: for derive attributes, this will return separate copies of the same structures! Though
331+
// they may differ in spans due to differing call sites...
328332
fn macro_arg(
329333
db: &dyn ExpandDatabase,
330334
id: MacroCallId,
@@ -336,173 +340,155 @@ fn macro_arg(
336340
.then(|| loc.eager.as_deref())
337341
.flatten()
338342
{
339-
ValueResult::ok((arg.clone(), SyntaxFixupUndoInfo::NONE))
340-
} else {
341-
let (parse, map) = parse_with_map(db, loc.kind.file_id());
342-
let root = parse.syntax_node();
343-
344-
let syntax = match loc.kind {
345-
MacroCallKind::FnLike { ast_id, .. } => {
346-
let dummy_tt = |kind| {
347-
(
348-
Arc::new(tt::Subtree {
349-
delimiter: tt::Delimiter {
350-
open: loc.call_site,
351-
close: loc.call_site,
352-
kind,
353-
},
354-
token_trees: Box::default(),
355-
}),
356-
SyntaxFixupUndoInfo::default(),
357-
)
358-
};
343+
return ValueResult::ok((arg.clone(), SyntaxFixupUndoInfo::NONE));
344+
}
359345

360-
let node = &ast_id.to_ptr(db).to_node(&root);
361-
let offset = node.syntax().text_range().start();
362-
let Some(tt) = node.token_tree() else {
363-
return ValueResult::new(
364-
dummy_tt(tt::DelimiterKind::Invisible),
365-
Arc::new(Box::new([SyntaxError::new_at_offset(
366-
"missing token tree".to_owned(),
367-
offset,
368-
)])),
369-
);
370-
};
371-
let first = tt.left_delimiter_token().map(|it| it.kind()).unwrap_or(T!['(']);
372-
let last = tt.right_delimiter_token().map(|it| it.kind()).unwrap_or(T![.]);
346+
let (parse, map) = parse_with_map(db, loc.kind.file_id());
347+
let root = parse.syntax_node();
373348

374-
let mismatched_delimiters = !matches!(
375-
(first, last),
376-
(T!['('], T![')']) | (T!['['], T![']']) | (T!['{'], T!['}'])
377-
);
378-
if mismatched_delimiters {
379-
// Don't expand malformed (unbalanced) macro invocations. This is
380-
// less than ideal, but trying to expand unbalanced macro calls
381-
// sometimes produces pathological, deeply nested code which breaks
382-
// all kinds of things.
383-
//
384-
// So instead, we'll return an empty subtree here
385-
cov_mark::hit!(issue9358_bad_macro_stack_overflow);
386-
387-
let kind = match first {
388-
_ if loc.def.is_proc_macro() => tt::DelimiterKind::Invisible,
389-
T!['('] => tt::DelimiterKind::Parenthesis,
390-
T!['['] => tt::DelimiterKind::Bracket,
391-
T!['{'] => tt::DelimiterKind::Brace,
392-
_ => tt::DelimiterKind::Invisible,
393-
};
394-
return ValueResult::new(
395-
dummy_tt(kind),
396-
Arc::new(Box::new([SyntaxError::new_at_offset(
397-
"mismatched delimiters".to_owned(),
398-
offset,
399-
)])),
400-
);
401-
}
402-
tt.syntax().clone()
403-
}
404-
MacroCallKind::Derive { ast_id, .. } => {
405-
ast_id.to_ptr(db).to_node(&root).syntax().clone()
406-
}
407-
MacroCallKind::Attr { ast_id, .. } => ast_id.to_ptr(db).to_node(&root).syntax().clone(),
408-
};
409-
let (mut tt, undo_info) = match loc.kind {
410-
MacroCallKind::FnLike { .. } => (
411-
mbe::syntax_node_to_token_tree(&syntax, map.as_ref(), loc.call_site),
412-
SyntaxFixupUndoInfo::NONE,
413-
),
414-
MacroCallKind::Derive { .. } | MacroCallKind::Attr { .. } => {
415-
let censor = censor_for_macro_input(&loc, &syntax);
416-
let censor_cfg =
417-
cfg_process::process_cfg_attrs(&syntax, &loc, db).unwrap_or_default();
418-
let mut fixups = fixup::fixup_syntax(map.as_ref(), &syntax, loc.call_site);
419-
fixups.append.retain(|it, _| match it {
420-
syntax::NodeOrToken::Token(_) => true,
421-
it => !censor.contains(it) && !censor_cfg.contains(it),
422-
});
423-
fixups.remove.extend(censor);
424-
fixups.remove.extend(censor_cfg);
425-
426-
{
427-
let mut tt = mbe::syntax_node_to_token_tree_modified(
428-
&syntax,
429-
map.as_ref(),
430-
fixups.append.clone(),
431-
fixups.remove.clone(),
432-
loc.call_site,
433-
);
434-
reverse_fixups(&mut tt, &fixups.undo_info);
435-
}
349+
let (censor, item_node) = match loc.kind {
350+
MacroCallKind::FnLike { ast_id, .. } => {
351+
let dummy_tt = |kind| {
436352
(
437-
mbe::syntax_node_to_token_tree_modified(
438-
&syntax,
439-
map,
440-
fixups.append,
441-
fixups.remove,
442-
loc.call_site,
443-
),
444-
fixups.undo_info,
353+
Arc::new(tt::Subtree {
354+
delimiter: tt::Delimiter {
355+
open: loc.call_site,
356+
close: loc.call_site,
357+
kind,
358+
},
359+
token_trees: Box::default(),
360+
}),
361+
SyntaxFixupUndoInfo::default(),
445362
)
363+
};
364+
365+
let node = &ast_id.to_ptr(db).to_node(&root);
366+
let offset = node.syntax().text_range().start();
367+
let Some(tt) = node.token_tree() else {
368+
return ValueResult::new(
369+
dummy_tt(tt::DelimiterKind::Invisible),
370+
Arc::new(Box::new([SyntaxError::new_at_offset(
371+
"missing token tree".to_owned(),
372+
offset,
373+
)])),
374+
);
375+
};
376+
let first = tt.left_delimiter_token().map(|it| it.kind()).unwrap_or(T!['(']);
377+
let last = tt.right_delimiter_token().map(|it| it.kind()).unwrap_or(T![.]);
378+
379+
let mismatched_delimiters = !matches!(
380+
(first, last),
381+
(T!['('], T![')']) | (T!['['], T![']']) | (T!['{'], T!['}'])
382+
);
383+
if mismatched_delimiters {
384+
// Don't expand malformed (unbalanced) macro invocations. This is
385+
// less than ideal, but trying to expand unbalanced macro calls
386+
// sometimes produces pathological, deeply nested code which breaks
387+
// all kinds of things.
388+
//
389+
// So instead, we'll return an empty subtree here
390+
cov_mark::hit!(issue9358_bad_macro_stack_overflow);
391+
392+
let kind = match first {
393+
_ if loc.def.is_proc_macro() => tt::DelimiterKind::Invisible,
394+
T!['('] => tt::DelimiterKind::Parenthesis,
395+
T!['['] => tt::DelimiterKind::Bracket,
396+
T!['{'] => tt::DelimiterKind::Brace,
397+
_ => tt::DelimiterKind::Invisible,
398+
};
399+
return ValueResult::new(
400+
dummy_tt(kind),
401+
Arc::new(Box::new([SyntaxError::new_at_offset(
402+
"mismatched delimiters".to_owned(),
403+
offset,
404+
)])),
405+
);
446406
}
447-
};
448407

449-
if loc.def.is_proc_macro() {
450-
// proc macros expect their inputs without parentheses, MBEs expect it with them included
451-
tt.delimiter.kind = tt::DelimiterKind::Invisible;
408+
let tt = mbe::syntax_node_to_token_tree(tt.syntax(), map.as_ref(), loc.call_site);
409+
let val = (Arc::new(tt), SyntaxFixupUndoInfo::NONE);
410+
return if matches!(loc.def.kind, MacroDefKind::BuiltInEager(..)) {
411+
match parse.errors() {
412+
errors if errors.is_empty() => ValueResult::ok(val),
413+
errors => ValueResult::new(
414+
val,
415+
// Box::<[_]>::from(res.errors()), not stable yet
416+
Arc::new(errors.to_vec().into_boxed_slice()),
417+
),
418+
}
419+
} else {
420+
ValueResult::ok(val)
421+
};
452422
}
453-
454-
if matches!(loc.def.kind, MacroDefKind::BuiltInEager(..)) {
455-
match parse.errors() {
456-
errors if errors.is_empty() => ValueResult::ok((Arc::new(tt), undo_info)),
457-
errors => ValueResult::new(
458-
(Arc::new(tt), undo_info),
459-
// Box::<[_]>::from(res.errors()), not stable yet
460-
Arc::new(errors.to_vec().into_boxed_slice()),
461-
),
462-
}
463-
} else {
464-
ValueResult::ok((Arc::new(tt), undo_info))
423+
MacroCallKind::Derive { ast_id, derive_attr_index, .. } => {
424+
let node = ast_id.to_ptr(db).to_node(&root);
425+
(censor_derive_input(derive_attr_index, &node), node.into())
426+
}
427+
MacroCallKind::Attr { ast_id, invoc_attr_index, .. } => {
428+
let node = ast_id.to_ptr(db).to_node(&root);
429+
(censor_attr_input(invoc_attr_index, &node), node)
465430
}
431+
};
432+
433+
let (mut tt, undo_info) = {
434+
let syntax = item_node.syntax();
435+
let censor_cfg = cfg_process::process_cfg_attrs(syntax, &loc, db).unwrap_or_default();
436+
let mut fixups = fixup::fixup_syntax(map.as_ref(), syntax, loc.call_site);
437+
fixups.append.retain(|it, _| match it {
438+
syntax::NodeOrToken::Token(_) => true,
439+
it => !censor.contains(it) && !censor_cfg.contains(it),
440+
});
441+
fixups.remove.extend(censor);
442+
fixups.remove.extend(censor_cfg);
443+
444+
(
445+
mbe::syntax_node_to_token_tree_modified(
446+
syntax,
447+
map,
448+
fixups.append,
449+
fixups.remove,
450+
loc.call_site,
451+
),
452+
fixups.undo_info,
453+
)
454+
};
455+
456+
if loc.def.is_proc_macro() {
457+
// proc macros expect their inputs without parentheses, MBEs expect it with them included
458+
tt.delimiter.kind = tt::DelimiterKind::Invisible;
466459
}
460+
461+
ValueResult::ok((Arc::new(tt), undo_info))
467462
}
468463

469464
// FIXME: Censoring info should be calculated by the caller! Namely by name resolution
470-
/// Certain macro calls expect some nodes in the input to be preprocessed away, namely:
471-
/// - derives expect all `#[derive(..)]` invocations up to the currently invoked one to be stripped
472-
/// - attributes expect the invoking attribute to be stripped
473-
fn censor_for_macro_input(loc: &MacroCallLoc, node: &SyntaxNode) -> FxHashSet<SyntaxElement> {
465+
/// Derives expect all `#[derive(..)]` invocations up to the currently invoked one to be stripped
466+
fn censor_derive_input(derive_attr_index: AttrId, node: &ast::Adt) -> FxHashSet<SyntaxElement> {
474467
// FIXME: handle `cfg_attr`
475-
(|| {
476-
let censor = match loc.kind {
477-
MacroCallKind::FnLike { .. } => return None,
478-
MacroCallKind::Derive { derive_attr_index, .. } => {
479-
cov_mark::hit!(derive_censoring);
480-
ast::Item::cast(node.clone())?
481-
.attrs()
482-
.take(derive_attr_index.ast_index() + 1)
483-
// FIXME, this resolution should not be done syntactically
484-
// derive is a proper macro now, no longer builtin
485-
// But we do not have resolution at this stage, this means
486-
// we need to know about all macro calls for the given ast item here
487-
// so we require some kind of mapping...
488-
.filter(|attr| attr.simple_name().as_deref() == Some("derive"))
489-
.map(|it| it.syntax().clone().into())
490-
.collect()
491-
}
492-
MacroCallKind::Attr { .. } if loc.def.is_attribute_derive() => return None,
493-
MacroCallKind::Attr { invoc_attr_index, .. } => {
494-
cov_mark::hit!(attribute_macro_attr_censoring);
495-
collect_attrs(&ast::Item::cast(node.clone())?)
496-
.nth(invoc_attr_index.ast_index())
497-
.and_then(|x| Either::left(x.1))
498-
.map(|attr| attr.syntax().clone().into())
499-
.into_iter()
500-
.collect()
501-
}
502-
};
503-
Some(censor)
504-
})()
505-
.unwrap_or_default()
468+
cov_mark::hit!(derive_censoring);
469+
collect_attrs(node)
470+
.take(derive_attr_index.ast_index() + 1)
471+
.filter_map(|(_, attr)| Either::left(attr))
472+
// FIXME, this resolution should not be done syntactically
473+
// derive is a proper macro now, no longer builtin
474+
// But we do not have resolution at this stage, this means
475+
// we need to know about all macro calls for the given ast item here
476+
// so we require some kind of mapping...
477+
.filter(|attr| attr.simple_name().as_deref() == Some("derive"))
478+
.map(|it| it.syntax().clone().into())
479+
.collect()
480+
}
481+
482+
/// Attributes expect the invoking attribute to be stripped\
483+
fn censor_attr_input(invoc_attr_index: AttrId, node: &ast::Item) -> FxHashSet<SyntaxElement> {
484+
// FIXME: handle `cfg_attr`
485+
cov_mark::hit!(attribute_macro_attr_censoring);
486+
collect_attrs(node)
487+
.nth(invoc_attr_index.ast_index())
488+
.and_then(|(_, attr)| Either::left(attr))
489+
.map(|attr| attr.syntax().clone().into())
490+
.into_iter()
491+
.collect()
506492
}
507493

508494
impl TokenExpander {

‎crates/hir-expand/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ pub struct MacroCallLoc {
176176
// leakage problems here
177177
eager: Option<Arc<EagerCallInfo>>,
178178
pub kind: MacroCallKind,
179+
// FIXME: Spans while relative to an anchor, are still rather unstable
179180
pub call_site: Span,
180181
}
181182
impl_intern_value_trivial!(MacroCallLoc);

0 commit comments

Comments
 (0)