Skip to content

Commit cf87333

Browse files
borsbors
committed
Auto merge of rust-lang#16540 - Veykril:macro-arg, r=Veykril
internal: macro_arg query always returns a TokenTree
2 parents 35b0d66 + 2fa57d9 commit cf87333

File tree

12 files changed

+139
-131
lines changed

12 files changed

+139
-131
lines changed

‎crates/hir-def/src/body/lower.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,10 +1000,6 @@ impl ExprCollector<'_> {
10001000
krate: *krate,
10011001
});
10021002
}
1003-
Some(ExpandError::RecursionOverflowPoisoned) => {
1004-
// Recursion limit has been reached in the macro expansion tree, but not in
1005-
// this very macro call. Don't add diagnostics to avoid duplication.
1006-
}
10071003
Some(err) => {
10081004
self.source_map.diagnostics.push(BodyDiagnostic::MacroError {
10091005
node: InFile::new(outer_file, syntax_ptr),

‎crates/hir-def/src/expander.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,13 +140,11 @@ impl Expander {
140140
// The overflow error should have been reported when it occurred (see the next branch),
141141
// so don't return overflow error here to avoid diagnostics duplication.
142142
cov_mark::hit!(overflow_but_not_me);
143-
return ExpandResult::only_err(ExpandError::RecursionOverflowPoisoned);
143+
return ExpandResult::ok(None);
144144
} else if self.recursion_limit.check(self.recursion_depth as usize + 1).is_err() {
145145
self.recursion_depth = u32::MAX;
146146
cov_mark::hit!(your_stack_belongs_to_me);
147-
return ExpandResult::only_err(ExpandError::other(
148-
"reached recursion limit during macro expansion",
149-
));
147+
return ExpandResult::only_err(ExpandError::RecursionOverflow);
150148
}
151149

152150
let ExpandResult { value, err } = op(self);

‎crates/hir-def/src/macro_expansion_tests/mbe/matching.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ m!(&k");
3333
"#,
3434
expect![[r#"
3535
macro_rules! m { ($i:literal) => {}; }
36-
/* error: invalid token tree */"#]],
36+
/* error: mismatched delimiters */"#]],
3737
);
3838
}
3939

‎crates/hir-def/src/macro_expansion_tests/mbe/meta_syntax.rs

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -68,26 +68,26 @@ m2!();
6868
"#,
6969
expect![[r#"
7070
macro_rules! i1 { invalid }
71-
/* error: invalid macro definition: expected subtree */
71+
/* error: macro definition has parse errors */
7272
7373
macro_rules! e1 { $i:ident => () }
74-
/* error: invalid macro definition: expected subtree */
74+
/* error: macro definition has parse errors */
7575
macro_rules! e2 { ($i:ident) () }
76-
/* error: invalid macro definition: expected `=` */
76+
/* error: macro definition has parse errors */
7777
macro_rules! e3 { ($(i:ident)_) => () }
78-
/* error: invalid macro definition: invalid repeat */
78+
/* error: macro definition has parse errors */
7979
8080
macro_rules! f1 { ($i) => ($i) }
81-
/* error: invalid macro definition: missing fragment specifier */
81+
/* error: macro definition has parse errors */
8282
macro_rules! f2 { ($i:) => ($i) }
83-
/* error: invalid macro definition: missing fragment specifier */
83+
/* error: macro definition has parse errors */
8484
macro_rules! f3 { ($i:_) => () }
85-
/* error: invalid macro definition: missing fragment specifier */
85+
/* error: macro definition has parse errors */
8686
8787
macro_rules! m1 { ($$i) => () }
88-
/* error: invalid macro definition: `$$` is not allowed on the pattern side */
88+
/* error: macro definition has parse errors */
8989
macro_rules! m2 { () => ( ${invalid()} ) }
90-
/* error: invalid macro definition: invalid metavariable expression */
90+
/* error: macro definition has parse errors */
9191
"#]],
9292
)
9393
}
@@ -137,18 +137,18 @@ macro_rules! m9 { ($($($($i:ident)?)*)+) => {}; }
137137
macro_rules! mA { ($($($($i:ident)+)?)*) => {}; }
138138
macro_rules! mB { ($($($($i:ident)+)*)?) => {}; }
139139
140-
/* error: invalid macro definition: empty token tree in repetition */
141-
/* error: invalid macro definition: empty token tree in repetition */
142-
/* error: invalid macro definition: empty token tree in repetition */
143-
/* error: invalid macro definition: empty token tree in repetition */
144-
/* error: invalid macro definition: empty token tree in repetition */
145-
/* error: invalid macro definition: empty token tree in repetition */
146-
/* error: invalid macro definition: empty token tree in repetition */
147-
/* error: invalid macro definition: empty token tree in repetition */
148-
/* error: invalid macro definition: empty token tree in repetition */
149-
/* error: invalid macro definition: empty token tree in repetition */
150-
/* error: invalid macro definition: empty token tree in repetition */
151-
/* error: invalid macro definition: empty token tree in repetition */
140+
/* error: macro definition has parse errors */
141+
/* error: macro definition has parse errors */
142+
/* error: macro definition has parse errors */
143+
/* error: macro definition has parse errors */
144+
/* error: macro definition has parse errors */
145+
/* error: macro definition has parse errors */
146+
/* error: macro definition has parse errors */
147+
/* error: macro definition has parse errors */
148+
/* error: macro definition has parse errors */
149+
/* error: macro definition has parse errors */
150+
/* error: macro definition has parse errors */
151+
/* error: macro definition has parse errors */
152152
"#]],
153153
);
154154
}

‎crates/hir-def/src/macro_expansion_tests/mbe/metavar_expr.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,9 @@ macro_rules! depth_too_large {
275275
}
276276
277277
fn test() {
278-
/* error: invalid macro definition: invalid metavariable expression */;
279-
/* error: invalid macro definition: invalid metavariable expression */;
280-
/* error: invalid macro definition: invalid metavariable expression */;
278+
/* error: macro definition has parse errors */;
279+
/* error: macro definition has parse errors */;
280+
/* error: macro definition has parse errors */;
281281
}
282282
"#]],
283283
);

‎crates/hir-def/src/macro_expansion_tests/mbe/tt_conversion.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,8 @@ m2!(x
9797
macro_rules! m1 { ($x:ident) => { ($x } }
9898
macro_rules! m2 { ($x:ident) => {} }
9999
100-
/* error: invalid macro definition: expected subtree */
101-
/* error: invalid token tree */
100+
/* error: macro definition has parse errors */
101+
/* error: mismatched delimiters */
102102
"#]],
103103
)
104104
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,7 @@ fn compile_error_expand(
446446
) -> ExpandResult<tt::Subtree> {
447447
let err = match &*tt.token_trees {
448448
[tt::TokenTree::Leaf(tt::Leaf::Literal(it))] => match unquote_str(it) {
449-
Some(unquoted) => ExpandError::other(unquoted),
449+
Some(unquoted) => ExpandError::other(unquoted.into_boxed_str()),
450450
None => ExpandError::other("`compile_error!` argument must be a string"),
451451
},
452452
_ => ExpandError::other("`compile_error!` argument must be a string"),

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

Lines changed: 79 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ pub trait ExpandDatabase: SourceDatabase {
108108
fn macro_arg(
109109
&self,
110110
id: MacroCallId,
111-
) -> ValueResult<Option<(Arc<tt::Subtree>, SyntaxFixupUndoInfo)>, Arc<Box<[SyntaxError]>>>;
111+
) -> ValueResult<(Arc<tt::Subtree>, SyntaxFixupUndoInfo), Arc<Box<[SyntaxError]>>>;
112112
/// Fetches the expander for this macro.
113113
#[salsa::transparent]
114114
#[salsa::invoke(TokenExpander::macro_expander)]
@@ -326,58 +326,77 @@ fn macro_arg(
326326
db: &dyn ExpandDatabase,
327327
id: MacroCallId,
328328
// FIXME: consider the following by putting fixup info into eager call info args
329-
// ) -> ValueResult<Option<Arc<(tt::Subtree, SyntaxFixupUndoInfo)>>, Arc<Box<[SyntaxError]>>> {
330-
) -> ValueResult<Option<(Arc<tt::Subtree>, SyntaxFixupUndoInfo)>, Arc<Box<[SyntaxError]>>> {
331-
let mismatched_delimiters = |arg: &SyntaxNode| {
332-
let first = arg.first_child_or_token().map_or(T![.], |it| it.kind());
333-
let last = arg.last_child_or_token().map_or(T![.], |it| it.kind());
334-
let well_formed_tt =
335-
matches!((first, last), (T!['('], T![')']) | (T!['['], T![']']) | (T!['{'], T!['}']));
336-
if !well_formed_tt {
337-
// Don't expand malformed (unbalanced) macro invocations. This is
338-
// less than ideal, but trying to expand unbalanced macro calls
339-
// sometimes produces pathological, deeply nested code which breaks
340-
// all kinds of things.
341-
//
342-
// Some day, we'll have explicit recursion counters for all
343-
// recursive things, at which point this code might be removed.
344-
cov_mark::hit!(issue9358_bad_macro_stack_overflow);
345-
Some(Arc::new(Box::new([SyntaxError::new(
346-
"unbalanced token tree".to_owned(),
347-
arg.text_range(),
348-
)]) as Box<[_]>))
349-
} else {
350-
None
351-
}
352-
};
329+
// ) -> ValueResult<Arc<(tt::Subtree, SyntaxFixupUndoInfo)>, Arc<Box<[SyntaxError]>>> {
330+
) -> ValueResult<(Arc<tt::Subtree>, SyntaxFixupUndoInfo), Arc<Box<[SyntaxError]>>> {
353331
let loc = db.lookup_intern_macro_call(id);
354332
if let Some(EagerCallInfo { arg, .. }) = matches!(loc.def.kind, MacroDefKind::BuiltInEager(..))
355333
.then(|| loc.eager.as_deref())
356334
.flatten()
357335
{
358-
ValueResult::ok(Some((arg.clone(), SyntaxFixupUndoInfo::NONE)))
336+
ValueResult::ok((arg.clone(), SyntaxFixupUndoInfo::NONE))
359337
} else {
360338
let (parse, map) = parse_with_map(db, loc.kind.file_id());
361339
let root = parse.syntax_node();
362340

363341
let syntax = match loc.kind {
364342
MacroCallKind::FnLike { ast_id, .. } => {
343+
let dummy_tt = |kind| {
344+
(
345+
Arc::new(tt::Subtree {
346+
delimiter: tt::Delimiter {
347+
open: loc.call_site,
348+
close: loc.call_site,
349+
kind,
350+
},
351+
token_trees: Box::default(),
352+
}),
353+
SyntaxFixupUndoInfo::default(),
354+
)
355+
};
356+
365357
let node = &ast_id.to_ptr(db).to_node(&root);
366358
let offset = node.syntax().text_range().start();
367-
match node.token_tree() {
368-
Some(tt) => {
369-
let tt = tt.syntax();
370-
if let Some(e) = mismatched_delimiters(tt) {
371-
return ValueResult::only_err(e);
372-
}
373-
tt.clone()
374-
}
375-
None => {
376-
return ValueResult::only_err(Arc::new(Box::new([
377-
SyntaxError::new_at_offset("missing token tree".to_owned(), offset),
378-
])));
379-
}
359+
let Some(tt) = node.token_tree() else {
360+
return ValueResult::new(
361+
dummy_tt(tt::DelimiterKind::Invisible),
362+
Arc::new(Box::new([SyntaxError::new_at_offset(
363+
"missing token tree".to_owned(),
364+
offset,
365+
)])),
366+
);
367+
};
368+
let first = tt.left_delimiter_token().map(|it| it.kind()).unwrap_or(T!['(']);
369+
let last = tt.right_delimiter_token().map(|it| it.kind()).unwrap_or(T![.]);
370+
371+
let mismatched_delimiters = !matches!(
372+
(first, last),
373+
(T!['('], T![')']) | (T!['['], T![']']) | (T!['{'], T!['}'])
374+
);
375+
if mismatched_delimiters {
376+
// Don't expand malformed (unbalanced) macro invocations. This is
377+
// less than ideal, but trying to expand unbalanced macro calls
378+
// sometimes produces pathological, deeply nested code which breaks
379+
// all kinds of things.
380+
//
381+
// So instead, we'll return an empty subtree here
382+
cov_mark::hit!(issue9358_bad_macro_stack_overflow);
383+
384+
let kind = match first {
385+
_ if loc.def.is_proc_macro() => tt::DelimiterKind::Invisible,
386+
T!['('] => tt::DelimiterKind::Parenthesis,
387+
T!['['] => tt::DelimiterKind::Bracket,
388+
T!['{'] => tt::DelimiterKind::Brace,
389+
_ => tt::DelimiterKind::Invisible,
390+
};
391+
return ValueResult::new(
392+
dummy_tt(kind),
393+
Arc::new(Box::new([SyntaxError::new_at_offset(
394+
"mismatched delimiters".to_owned(),
395+
offset,
396+
)])),
397+
);
380398
}
399+
tt.syntax().clone()
381400
}
382401
MacroCallKind::Derive { ast_id, .. } => {
383402
ast_id.to_ptr(db).to_node(&root).syntax().clone()
@@ -427,15 +446,15 @@ fn macro_arg(
427446

428447
if matches!(loc.def.kind, MacroDefKind::BuiltInEager(..)) {
429448
match parse.errors() {
430-
[] => ValueResult::ok(Some((Arc::new(tt), undo_info))),
449+
[] => ValueResult::ok((Arc::new(tt), undo_info)),
431450
errors => ValueResult::new(
432-
Some((Arc::new(tt), undo_info)),
451+
(Arc::new(tt), undo_info),
433452
// Box::<[_]>::from(res.errors()), not stable yet
434453
Arc::new(errors.to_vec().into_boxed_slice()),
435454
),
436455
}
437456
} else {
438-
ValueResult::ok(Some((Arc::new(tt), undo_info)))
457+
ValueResult::ok((Arc::new(tt), undo_info))
439458
}
440459
}
441460
}
@@ -519,21 +538,20 @@ fn macro_expand(
519538
expander.expand(db, macro_call_id, &node, map.as_ref())
520539
}
521540
_ => {
522-
let ValueResult { value, err } = db.macro_arg(macro_call_id);
523-
let Some((macro_arg, undo_info)) = value else {
524-
return ExpandResult {
525-
value: CowArc::Owned(tt::Subtree {
526-
delimiter: tt::Delimiter::invisible_spanned(loc.call_site),
527-
token_trees: Box::new([]),
528-
}),
529-
// FIXME: We should make sure to enforce an invariant that invalid macro
530-
// calls do not reach this call path!
531-
err: Some(ExpandError::other("invalid token tree")),
532-
};
541+
let ValueResult { value: (macro_arg, undo_info), err } = db.macro_arg(macro_call_id);
542+
let format_parse_err = |err: Arc<Box<[SyntaxError]>>| {
543+
let mut buf = String::new();
544+
for err in &**err {
545+
use std::fmt::Write;
546+
_ = write!(buf, "{}, ", err);
547+
}
548+
buf.pop();
549+
buf.pop();
550+
ExpandError::other(buf)
533551
};
534552

535553
let arg = &*macro_arg;
536-
match loc.def.kind {
554+
let res = match loc.def.kind {
537555
MacroDefKind::Declarative(id) => {
538556
db.decl_macro_expander(loc.def.krate, id).expand(db, arg.clone(), macro_call_id)
539557
}
@@ -549,16 +567,7 @@ fn macro_expand(
549567
MacroDefKind::BuiltInEager(..) if loc.eager.is_none() => {
550568
return ExpandResult {
551569
value: CowArc::Arc(macro_arg.clone()),
552-
err: err.map(|err| {
553-
let mut buf = String::new();
554-
for err in &**err {
555-
use std::fmt::Write;
556-
_ = write!(buf, "{}, ", err);
557-
}
558-
buf.pop();
559-
buf.pop();
560-
ExpandError::other(buf)
561-
}),
570+
err: err.map(format_parse_err),
562571
};
563572
}
564573
MacroDefKind::BuiltInEager(it, _) => {
@@ -570,6 +579,11 @@ fn macro_expand(
570579
res
571580
}
572581
_ => unreachable!(),
582+
};
583+
ExpandResult {
584+
value: res.value,
585+
// if the arg had parse errors, show them instead of the expansion errors
586+
err: err.map(format_parse_err).or(res.err),
573587
}
574588
}
575589
};
@@ -597,17 +611,7 @@ fn macro_expand(
597611

598612
fn expand_proc_macro(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandResult<Arc<tt::Subtree>> {
599613
let loc = db.lookup_intern_macro_call(id);
600-
let Some((macro_arg, undo_info)) = db.macro_arg(id).value else {
601-
return ExpandResult {
602-
value: Arc::new(tt::Subtree {
603-
delimiter: tt::Delimiter::invisible_spanned(loc.call_site),
604-
token_trees: Box::new([]),
605-
}),
606-
// FIXME: We should make sure to enforce an invariant that invalid macro
607-
// calls do not reach this call path!
608-
err: Some(ExpandError::other("invalid token tree")),
609-
};
610-
};
614+
let (macro_arg, undo_info) = db.macro_arg(id).value;
611615

612616
let expander = match loc.def.kind {
613617
MacroDefKind::ProcMacro(expander, ..) => expander,

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ impl DeclarativeMacroExpander {
4444
)
4545
});
4646
match self.mac.err() {
47-
Some(e) => ExpandResult::new(
47+
Some(_) => ExpandResult::new(
4848
tt::Subtree::empty(tt::DelimSpan { open: loc.call_site, close: loc.call_site }),
49-
ExpandError::other(format!("invalid macro definition: {e}")),
49+
ExpandError::MacroDefinition,
5050
),
5151
None => self
5252
.mac
@@ -80,9 +80,9 @@ impl DeclarativeMacroExpander {
8080
)
8181
});
8282
match self.mac.err() {
83-
Some(e) => ExpandResult::new(
83+
Some(_) => ExpandResult::new(
8484
tt::Subtree::empty(tt::DelimSpan { open: call_site, close: call_site }),
85-
ExpandError::other(format!("invalid macro definition: {e}")),
85+
ExpandError::MacroDefinition,
8686
),
8787
None => self.mac.expand(&tt, |_| (), new_meta_vars, call_site).map_err(Into::into),
8888
}

0 commit comments

Comments
 (0)