aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2020-03-21 16:02:07 +0000
committerGitHub <[email protected]>2020-03-21 16:02:07 +0000
commit50c6a315ab2bef493b618742989744fb370a32ca (patch)
treefc4ee3a02310c55167d62d4bd95931d161178155
parent5e827bd948ea206328b126345ab8e909978b3b9b (diff)
parent9ff50d7e839c635632021d47e2dd3982916e4738 (diff)
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 <[email protected]>
-rw-r--r--crates/ra_hir_expand/src/db.rs24
-rw-r--r--crates/ra_hir_ty/src/tests/regression.rs31
-rw-r--r--crates/ra_syntax/src/algo.rs4
3 files changed, 56 insertions, 3 deletions
diff --git a/crates/ra_hir_expand/src/db.rs b/crates/ra_hir_expand/src/db.rs
index d171d2dfd..5a696542f 100644
--- a/crates/ra_hir_expand/src/db.rs
+++ b/crates/ra_hir_expand/src/db.rs
@@ -6,7 +6,7 @@ use mbe::{ExpandResult, MacroRules};
6use ra_db::{salsa, SourceDatabase}; 6use ra_db::{salsa, SourceDatabase};
7use ra_parser::FragmentKind; 7use ra_parser::FragmentKind;
8use ra_prof::profile; 8use ra_prof::profile;
9use ra_syntax::{AstNode, Parse, SyntaxKind::*, SyntaxNode}; 9use ra_syntax::{algo::diff, AstNode, Parse, SyntaxKind::*, SyntaxNode};
10 10
11use crate::{ 11use crate::{
12 ast_id_map::AstIdMap, BuiltinDeriveExpander, BuiltinFnLikeExpander, EagerCallLoc, EagerMacroId, 12 ast_id_map::AstIdMap, BuiltinDeriveExpander, BuiltinFnLikeExpander, EagerCallLoc, EagerMacroId,
@@ -238,7 +238,7 @@ pub fn parse_macro_with_arg(
238 } else { 238 } else {
239 db.macro_expand(macro_call_id) 239 db.macro_expand(macro_call_id)
240 }; 240 };
241 if let Some(err) = err { 241 if let Some(err) = &err {
242 // Note: 242 // Note:
243 // The final goal we would like to make all parse_macro success, 243 // The final goal we would like to make all parse_macro success,
244 // such that the following log will not call anyway. 244 // such that the following log will not call anyway.
@@ -272,7 +272,25 @@ pub fn parse_macro_with_arg(
272 let fragment_kind = to_fragment_kind(db, macro_call_id); 272 let fragment_kind = to_fragment_kind(db, macro_call_id);
273 273
274 let (parse, rev_token_map) = mbe::token_tree_to_syntax_node(&tt, fragment_kind).ok()?; 274 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 }
276} 294}
277 295
278/// Given a `MacroCallId`, return what `FragmentKind` it belongs to. 296/// Given a `MacroCallId`, return what `FragmentKind` it belongs to.
diff --git a/crates/ra_hir_ty/src/tests/regression.rs b/crates/ra_hir_ty/src/tests/regression.rs
index 14c8ed3a9..a02e3ee05 100644
--- a/crates/ra_hir_ty/src/tests/regression.rs
+++ b/crates/ra_hir_ty/src/tests/regression.rs
@@ -453,3 +453,34 @@ pub mod str {
453 // should be Option<char>, but currently not because of Chalk ambiguity problem 453 // should be Option<char>, but currently not because of Chalk ambiguity problem
454 assert_eq!("(Option<{unknown}>, Option<{unknown}>)", super::type_at_pos(&db, pos)); 454 assert_eq!("(Option<{unknown}>, Option<{unknown}>)", super::type_at_pos(&db, pos));
455} 455}
456
457#[test]
458fn issue_3642_bad_macro_stackover() {
459 let (db, pos) = TestDB::with_position(
460 r#"
461//- /main.rs
462#[macro_export]
463macro_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
475fn 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}
diff --git a/crates/ra_syntax/src/algo.rs b/crates/ra_syntax/src/algo.rs
index 344cf0fbe..ffdbdc767 100644
--- a/crates/ra_syntax/src/algo.rs
+++ b/crates/ra_syntax/src/algo.rs
@@ -95,6 +95,10 @@ impl TreeDiff {
95 builder.replace(from.text_range(), to.to_string()) 95 builder.replace(from.text_range(), to.to_string())
96 } 96 }
97 } 97 }
98
99 pub fn is_empty(&self) -> bool {
100 self.replacements.is_empty()
101 }
98} 102}
99 103
100/// Finds minimal the diff, which, applied to `from`, will result in `to`. 104/// Finds minimal the diff, which, applied to `from`, will result in `to`.