From 8ab18927ce7fe04e4fffedb864ba608f1bce9279 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Sun, 12 Jan 2025 23:28:30 +0200 Subject: [PATCH] Fix another bug when reaching macro expansion limit caused a stack overflow 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. --- .../src/handlers/macro_error.rs | 26 +++++++++++++++++++ crates/mbe/src/expander/transcriber.rs | 5 ++++ 2 files changed, 31 insertions(+) diff --git a/crates/ide-diagnostics/src/handlers/macro_error.rs b/crates/ide-diagnostics/src/handlers/macro_error.rs index edf656ed04..99894fefef 100644 --- a/crates/ide-diagnostics/src/handlers/macro_error.rs +++ b/crates/ide-diagnostics/src/handlers/macro_error.rs @@ -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 +} + + "#, + ); + } } diff --git a/crates/mbe/src/expander/transcriber.rs b/crates/mbe/src/expander/transcriber.rs index acab989437..7710ea7938 100644 --- a/crates/mbe/src/expander/transcriber.rs +++ b/crates/mbe/src/expander/transcriber.rs @@ -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 } = @@ -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; }