Skip to content

Commit

Permalink
Fix another bug when reaching macro expansion limit caused a stack ov…
Browse files Browse the repository at this point in the history
…erflow

This time without missing bindings.

Solve it by returning to the old ways, i.e. just throw the extra nodes away.

In other words, I acknowledge defeat.
  • Loading branch information
ChayimFriedman2 committed Jan 13, 2025
1 parent 8364ef2 commit 8ab1892
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 0 deletions.
26 changes: 26 additions & 0 deletions crates/ide-diagnostics/src/handlers/macro_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,4 +291,30 @@ mod prim_never {}
"#,
);
}

#[test]
fn no_stack_overflow_for_missing_binding() {
check_diagnostics(
r#"
#[macro_export]
macro_rules! boom {
(
$($code:literal),+,
$(param: $param:expr,)?
) => {{
let _ = $crate::boom!(@param $($param)*);
}};
(@param) => { () };
(@param $param:expr) => { $param };
}
fn it_works() {
// NOTE: there is an error, but RA crashes before showing it
boom!("RAND", param: c7.clone());
// ^^^^^ error: expected literal
}
"#,
);
}
}
5 changes: 5 additions & 0 deletions crates/mbe/src/expander/transcriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,7 @@ fn expand_repeat(
let mut counter = 0;
let mut err = None;

let initial_restore_point = builder.restore_point();
let mut restore_point = builder.restore_point();
loop {
let ExpandResult { value: (), err: e } =
Expand All @@ -465,6 +466,10 @@ fn expand_repeat(

counter += 1;
if counter == limit {
// FIXME: This is a bug here, we get here when we shouldn't, see https://github.com/rust-lang/rust-analyzer/issues/18910.
// If we don't restore we emit a lot of nodes which causes a stack overflow down the road. For now just ignore them,
// there is always an error here anyway.
builder.restore(initial_restore_point);
err = Some(ExpandError::new(ctx.call_site, ExpandErrorKind::LimitExceeded));
break;
}
Expand Down

0 comments on commit 8ab1892

Please sign in to comment.