Skip to content

Commit 50c6a31

Browse files
Merge #3671
3671: Add identity expansion checking in ill-form expansion r=flodiebold a=edwin0cheng This PR try to add more checking code in error case in macro expansion. The bug in #3642 is introduced by #3580 , which allow ill-form macro expansion in *all* kind of macro expansions. In general we should separate hypothetical macro expansion and the actual macro expansion call. However, currently the `Semantic` workflow we are using only support single macro expansion type, we might want to review it and make it works in both ways. (Maybe add a field in `MacroCallLoc` for differentiation) Fix #3642 Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>
2 parents 5e827bd + 9ff50d7 commit 50c6a31

File tree

3 files changed

+56
-3
lines changed

3 files changed

+56
-3
lines changed

crates/ra_hir_expand/src/db.rs

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use mbe::{ExpandResult, MacroRules};
66
use ra_db::{salsa, SourceDatabase};
77
use ra_parser::FragmentKind;
88
use ra_prof::profile;
9-
use ra_syntax::{AstNode, Parse, SyntaxKind::*, SyntaxNode};
9+
use ra_syntax::{algo::diff, AstNode, Parse, SyntaxKind::*, SyntaxNode};
1010

1111
use crate::{
1212
ast_id_map::AstIdMap, BuiltinDeriveExpander, BuiltinFnLikeExpander, EagerCallLoc, EagerMacroId,
@@ -238,7 +238,7 @@ pub fn parse_macro_with_arg(
238238
} else {
239239
db.macro_expand(macro_call_id)
240240
};
241-
if let Some(err) = err {
241+
if let Some(err) = &err {
242242
// Note:
243243
// The final goal we would like to make all parse_macro success,
244244
// such that the following log will not call anyway.
@@ -272,7 +272,25 @@ pub fn parse_macro_with_arg(
272272
let fragment_kind = to_fragment_kind(db, macro_call_id);
273273

274274
let (parse, rev_token_map) = mbe::token_tree_to_syntax_node(&tt, fragment_kind).ok()?;
275-
Some((parse, Arc::new(rev_token_map)))
275+
276+
if err.is_none() {
277+
Some((parse, Arc::new(rev_token_map)))
278+
} else {
279+
// FIXME:
280+
// In future, we should propagate the actual error with recovery information
281+
// instead of ignore the error here.
282+
283+
// Safe check for recurisve identity macro
284+
let node = parse.syntax_node();
285+
let file: HirFileId = macro_file.into();
286+
let call_node = file.call_node(db)?;
287+
288+
if !diff(&node, &call_node.value).is_empty() {
289+
Some((parse, Arc::new(rev_token_map)))
290+
} else {
291+
None
292+
}
293+
}
276294
}
277295

278296
/// Given a `MacroCallId`, return what `FragmentKind` it belongs to.

crates/ra_hir_ty/src/tests/regression.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,3 +453,34 @@ pub mod str {
453453
// should be Option<char>, but currently not because of Chalk ambiguity problem
454454
assert_eq!("(Option<{unknown}>, Option<{unknown}>)", super::type_at_pos(&db, pos));
455455
}
456+
457+
#[test]
458+
fn issue_3642_bad_macro_stackover() {
459+
let (db, pos) = TestDB::with_position(
460+
r#"
461+
//- /main.rs
462+
#[macro_export]
463+
macro_rules! match_ast {
464+
(match $node:ident { $($tt:tt)* }) => { match_ast!(match ($node) { $($tt)* }) };
465+
466+
(match ($node:expr) {
467+
$( ast::$ast:ident($it:ident) => $res:expr, )*
468+
_ => $catch_all:expr $(,)?
469+
}) => {{
470+
$( if let Some($it) = ast::$ast::cast($node.clone()) { $res } else )*
471+
{ $catch_all }
472+
}};
473+
}
474+
475+
fn main() {
476+
let anchor<|> = match_ast! {
477+
match parent {
478+
as => {},
479+
_ => return None
480+
}
481+
};
482+
}"#,
483+
);
484+
485+
assert_eq!("()", super::type_at_pos(&db, pos));
486+
}

crates/ra_syntax/src/algo.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,10 @@ impl TreeDiff {
9595
builder.replace(from.text_range(), to.to_string())
9696
}
9797
}
98+
99+
pub fn is_empty(&self) -> bool {
100+
self.replacements.is_empty()
101+
}
98102
}
99103

100104
/// Finds minimal the diff, which, applied to `from`, will result in `to`.

0 commit comments

Comments
 (0)