diff --git a/Cargo.lock b/Cargo.lock index 4d42c1c7be88..61180fcb9a89 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -508,6 +508,7 @@ version = "0.0.0" dependencies = [ "base_db", "cfg", + "cov-mark", "either", "expect-test", "la-arena", diff --git a/crates/cfg/src/tests.rs b/crates/cfg/src/tests.rs index d8736c893d7b..ea04114d3a3b 100644 --- a/crates/cfg/src/tests.rs +++ b/crates/cfg/src/tests.rs @@ -1,5 +1,5 @@ use expect_test::{expect, Expect}; -use mbe::ast_to_token_tree; +use mbe::syntax_node_to_token_tree; use syntax::{ast, AstNode}; use crate::{CfgAtom, CfgExpr, CfgOptions, DnfExpr}; @@ -8,7 +8,7 @@ fn assert_parse_result(input: &str, expected: CfgExpr) { let (tt, _) = { let source_file = ast::SourceFile::parse(input).ok().unwrap(); let tt = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap(); - ast_to_token_tree(&tt) + syntax_node_to_token_tree(tt.syntax()) }; let cfg = CfgExpr::parse(&tt); assert_eq!(cfg, expected); @@ -18,7 +18,7 @@ fn check_dnf(input: &str, expect: Expect) { let (tt, _) = { let source_file = ast::SourceFile::parse(input).ok().unwrap(); let tt = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap(); - ast_to_token_tree(&tt) + syntax_node_to_token_tree(tt.syntax()) }; let cfg = CfgExpr::parse(&tt); let actual = format!("#![cfg({})]", DnfExpr::new(cfg)); @@ -29,7 +29,7 @@ fn check_why_inactive(input: &str, opts: &CfgOptions, expect: Expect) { let (tt, _) = { let source_file = ast::SourceFile::parse(input).ok().unwrap(); let tt = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap(); - ast_to_token_tree(&tt) + syntax_node_to_token_tree(tt.syntax()) }; let cfg = CfgExpr::parse(&tt); let dnf = DnfExpr::new(cfg); @@ -42,7 +42,7 @@ fn check_enable_hints(input: &str, opts: &CfgOptions, expected_hints: &[&str]) { let (tt, _) = { let source_file = ast::SourceFile::parse(input).ok().unwrap(); let tt = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap(); - ast_to_token_tree(&tt) + syntax_node_to_token_tree(tt.syntax()) }; let cfg = CfgExpr::parse(&tt); let dnf = DnfExpr::new(cfg); diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs index 0ef046e46909..b2f8e4d20d90 100644 --- a/crates/hir_def/src/attr.rs +++ b/crates/hir_def/src/attr.rs @@ -12,7 +12,7 @@ use either::Either; use hir_expand::{hygiene::Hygiene, name::AsName, AstId, InFile}; use itertools::Itertools; use la_arena::ArenaMap; -use mbe::{ast_to_token_tree, DelimiterKind}; +use mbe::{syntax_node_to_token_tree, DelimiterKind}; use smallvec::{smallvec, SmallVec}; use syntax::{ ast::{self, AstNode, AttrsOwner}, @@ -679,7 +679,7 @@ impl Attr { }; Some(Interned::new(AttrInput::Literal(value))) } else if let Some(tt) = ast.token_tree() { - Some(Interned::new(AttrInput::TokenTree(ast_to_token_tree(&tt).0))) + Some(Interned::new(AttrInput::TokenTree(syntax_node_to_token_tree(tt.syntax()).0))) } else { None }; diff --git a/crates/hir_def/src/nameres/tests/macros.rs b/crates/hir_def/src/nameres/tests/macros.rs index 45448ba81b34..763dff5a2142 100644 --- a/crates/hir_def/src/nameres/tests/macros.rs +++ b/crates/hir_def/src/nameres/tests/macros.rs @@ -1,4 +1,5 @@ use super::*; + use crate::nameres::proc_macro::{ProcMacroDef, ProcMacroKind}; #[test] @@ -1021,3 +1022,20 @@ pub mod prelude { "#]], ) } + +#[test] +fn issue9358_bad_macro_stack_overflow() { + cov_mark::check!(issue9358_bad_macro_stack_overflow); + check( + r#" +macro_rules! m { + ($cond:expr) => { m!($cond, stringify!($cond)) }; + ($cond:expr, $($arg:tt)*) => { $cond }; +} +m!( +"#, + expect![[r#" + crate + "#]], + ) +} diff --git a/crates/hir_expand/Cargo.toml b/crates/hir_expand/Cargo.toml index 92d6c3e96a10..4645970266cc 100644 --- a/crates/hir_expand/Cargo.toml +++ b/crates/hir_expand/Cargo.toml @@ -9,6 +9,7 @@ edition = "2018" doctest = false [dependencies] +cov-mark = "2.0.0-pre.1" log = "0.4.8" either = "1.5.3" rustc-hash = "1.0.0" diff --git a/crates/hir_expand/src/builtin_macro.rs b/crates/hir_expand/src/builtin_macro.rs index f556bc751dbd..c9149f27a997 100644 --- a/crates/hir_expand/src/builtin_macro.rs +++ b/crates/hir_expand/src/builtin_macro.rs @@ -610,7 +610,7 @@ mod tests { let fragment = crate::to_fragment_kind(¯o_call); let args = macro_call.token_tree().unwrap(); - let parsed_args = mbe::ast_to_token_tree(&args).0; + let parsed_args = mbe::syntax_node_to_token_tree(args.syntax()).0; let call_id = AstId::new(file_id.into(), ast_id_map.ast_id(¯o_call)); let arg_id = db.intern_macro(MacroCallLoc { diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs index ee64564430e5..a9bf6db476f3 100644 --- a/crates/hir_expand/src/db.rs +++ b/crates/hir_expand/src/db.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use base_db::{salsa, SourceDatabase}; use limit::Limit; use mbe::{ExpandError, ExpandResult}; -use parser::FragmentKind; +use parser::{FragmentKind, T}; use syntax::{ algo::diff, ast::{self, NameOwner}, @@ -273,6 +273,23 @@ fn macro_arg_text(db: &dyn AstDatabase, id: MacroCallId) -> Option { let loc = db.lookup_intern_macro(id); let arg = loc.kind.arg(db)?; let arg = process_macro_input(db, arg, id); + if matches!(loc.kind, MacroCallKind::FnLike { .. }) { + let first = arg.first_child_or_token().map_or(T![.], |it| it.kind()); + let last = arg.last_child_or_token().map_or(T![.], |it| it.kind()); + let well_formed_tt = + matches!((first, last), (T!['('], T![')']) | (T!['['], T![']']) | (T!['{'], T!['}'])); + if !well_formed_tt { + // Don't expand malformed (unbalanced) macro invocations. This is + // less than ideal, but trying to expand unbalanced macro calls + // sometimes produces pathological, deeply nested code which breaks + // all kinds of things. + // + // Some day, we'll have explicit recursion counters for all + // recursive things, at which point this code might be removed. + cov_mark::hit!(issue9358_bad_macro_stack_overflow); + return None; + } + } Some(arg.green().into()) } @@ -281,7 +298,7 @@ fn macro_def(db: &dyn AstDatabase, id: MacroDefId) -> Option> MacroDefKind::Declarative(ast_id) => match ast_id.to_node(db) { ast::Macro::MacroRules(macro_rules) => { let arg = macro_rules.token_tree()?; - let (tt, def_site_token_map) = mbe::ast_to_token_tree(&arg); + let (tt, def_site_token_map) = mbe::syntax_node_to_token_tree(arg.syntax()); let mac = match mbe::MacroRules::parse(&tt) { Ok(it) => it, Err(err) => { @@ -294,7 +311,7 @@ fn macro_def(db: &dyn AstDatabase, id: MacroDefId) -> Option> } ast::Macro::MacroDef(macro_def) => { let arg = macro_def.body()?; - let (tt, def_site_token_map) = mbe::ast_to_token_tree(&arg); + let (tt, def_site_token_map) = mbe::syntax_node_to_token_tree(arg.syntax()); let mac = match mbe::MacroDef::parse(&tt) { Ok(it) => it, Err(err) => { diff --git a/crates/hir_expand/src/eager.rs b/crates/hir_expand/src/eager.rs index 66601f4df37d..052f1bf20e5d 100644 --- a/crates/hir_expand/src/eager.rs +++ b/crates/hir_expand/src/eager.rs @@ -107,7 +107,7 @@ pub fn expand_eager_macro( mut diagnostic_sink: &mut dyn FnMut(mbe::ExpandError), ) -> Result { let parsed_args = diagnostic_sink.option_with( - || Some(mbe::ast_to_token_tree(¯o_call.value.token_tree()?).0), + || Some(mbe::syntax_node_to_token_tree(¯o_call.value.token_tree()?.syntax()).0), || err("malformed macro invocation"), )?; diff --git a/crates/ide_completion/src/tests/expression.rs b/crates/ide_completion/src/tests/expression.rs index 95aaff01ac3e..9a42dd7b2751 100644 --- a/crates/ide_completion/src/tests/expression.rs +++ b/crates/ide_completion/src/tests/expression.rs @@ -308,27 +308,7 @@ fn quux(x: i32) { m!(x$0 } "#, - expect![[r#" - kw unsafe - kw match - kw while - kw while let - kw loop - kw if - kw if let - kw for - kw true - kw false - kw return - kw self - kw super - kw crate - lc y i32 - bt u32 - lc x i32 - fn quux(…) fn(i32) - ma m!(…) macro_rules! m - "#]], + expect![[r#""#]], ); } diff --git a/crates/ide_ssr/src/tests.rs b/crates/ide_ssr/src/tests.rs index 444c6b0afd0e..0b0c1111c466 100644 --- a/crates/ide_ssr/src/tests.rs +++ b/crates/ide_ssr/src/tests.rs @@ -921,13 +921,13 @@ fn preserves_whitespace_within_macro_expansion() { macro_rules! macro1 { ($a:expr) => {$a} } - fn f() {macro1!(1 * 2 + 3 + 4} + fn f() {macro1!(1 * 2 + 3 + 4)} "#, expect![[r#" macro_rules! macro1 { ($a:expr) => {$a} } - fn f() {macro1!(4 - (3 - 1 * 2)} + fn f() {macro1!(4 - (3 - 1 * 2))} "#]], ) } diff --git a/crates/mbe/src/benchmark.rs b/crates/mbe/src/benchmark.rs index 18eb97f0d58a..ffa316dd24c1 100644 --- a/crates/mbe/src/benchmark.rs +++ b/crates/mbe/src/benchmark.rs @@ -8,9 +8,8 @@ use syntax::{ use test_utils::{bench, bench_fixture, skip_slow_tests}; use crate::{ - ast_to_token_tree, parser::{Op, RepeatKind, Separator}, - MacroRules, + syntax_node_to_token_tree, MacroRules, }; #[test] @@ -65,7 +64,7 @@ fn macro_rules_fixtures_tt() -> FxHashMap { .filter_map(ast::MacroRules::cast) .map(|rule| { let id = rule.name().unwrap().to_string(); - let (def_tt, _) = ast_to_token_tree(&rule.token_tree().unwrap()); + let (def_tt, _) = syntax_node_to_token_tree(rule.token_tree().unwrap().syntax()); (id, def_tt) }) .collect() diff --git a/crates/mbe/src/expander.rs b/crates/mbe/src/expander.rs index bfef7f73d504..e9ef8ccecde2 100644 --- a/crates/mbe/src/expander.rs +++ b/crates/mbe/src/expander.rs @@ -120,7 +120,7 @@ mod tests { use syntax::{ast, AstNode}; use super::*; - use crate::ast_to_token_tree; + use crate::syntax_node_to_token_tree; #[test] fn test_expand_rule() { @@ -159,7 +159,8 @@ mod tests { let macro_definition = source_file.syntax().descendants().find_map(ast::MacroRules::cast).unwrap(); - let (definition_tt, _) = ast_to_token_tree(¯o_definition.token_tree().unwrap()); + let (definition_tt, _) = + syntax_node_to_token_tree(macro_definition.token_tree().unwrap().syntax()); crate::MacroRules::parse(&definition_tt).unwrap() } @@ -168,7 +169,8 @@ mod tests { let macro_invocation = source_file.syntax().descendants().find_map(ast::MacroCall::cast).unwrap(); - let (invocation_tt, _) = ast_to_token_tree(¯o_invocation.token_tree().unwrap()); + let (invocation_tt, _) = + syntax_node_to_token_tree(macro_invocation.token_tree().unwrap().syntax()); expand_rules(&rules.rules, &invocation_tt) } diff --git a/crates/mbe/src/lib.rs b/crates/mbe/src/lib.rs index fcc596756ca7..e0bbb825b93b 100644 --- a/crates/mbe/src/lib.rs +++ b/crates/mbe/src/lib.rs @@ -66,7 +66,7 @@ impl fmt::Display for ExpandError { pub use crate::{ syntax_bridge::{ - ast_to_token_tree, parse_exprs_with_sep, parse_to_token_tree, syntax_node_to_token_tree, + parse_exprs_with_sep, parse_to_token_tree, syntax_node_to_token_tree, token_tree_to_syntax_node, }, token_map::TokenMap, diff --git a/crates/mbe/src/syntax_bridge.rs b/crates/mbe/src/syntax_bridge.rs index ae6058cbc889..2aaa440a7124 100644 --- a/crates/mbe/src/syntax_bridge.rs +++ b/crates/mbe/src/syntax_bridge.rs @@ -13,12 +13,6 @@ use tt::buffer::{Cursor, TokenBuffer}; use crate::{subtree_source::SubtreeTokenSource, tt_iter::TtIter}; use crate::{ExpandError, TokenMap}; -/// Convert the syntax tree (what user has written) to a `TokenTree` (what macro -/// will consume). -pub fn ast_to_token_tree(ast: &impl ast::AstNode) -> (tt::Subtree, TokenMap) { - syntax_node_to_token_tree(ast.syntax()) -} - /// Convert the syntax node to a `TokenTree` (what macro /// will consume). pub fn syntax_node_to_token_tree(node: &SyntaxNode) -> (tt::Subtree, TokenMap) { @@ -812,7 +806,7 @@ mod tests { // - T!['}'] // - WHITE_SPACE let token_tree = ast::TokenTree::cast(token_tree).unwrap(); - let tt = ast_to_token_tree(&token_tree).0; + let tt = syntax_node_to_token_tree(token_tree.syntax()).0; assert_eq!(tt.delimiter_kind(), Some(tt::DelimiterKind::Brace)); } @@ -821,7 +815,7 @@ mod tests { fn test_token_tree_multi_char_punct() { let source_file = ast::SourceFile::parse("struct Foo { a: x::Y }").ok().unwrap(); let struct_def = source_file.syntax().descendants().find_map(ast::Struct::cast).unwrap(); - let tt = ast_to_token_tree(&struct_def).0; + let tt = syntax_node_to_token_tree(struct_def.syntax()).0; token_tree_to_syntax_node(&tt, FragmentKind::Item).unwrap(); } @@ -829,7 +823,7 @@ mod tests { fn test_missing_closing_delim() { let source_file = ast::SourceFile::parse("m!(x").tree(); let node = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap(); - let tt = ast_to_token_tree(&node).0.to_string(); + let tt = syntax_node_to_token_tree(node.syntax()).0.to_string(); assert_eq_text!(&*tt, "( x"); } } diff --git a/crates/mbe/src/tests.rs b/crates/mbe/src/tests.rs index 3698ff3f0f43..705cf5a2b16a 100644 --- a/crates/mbe/src/tests.rs +++ b/crates/mbe/src/tests.rs @@ -29,7 +29,8 @@ macro_rules! impl_fixture { let macro_invocation = source_file.syntax().descendants().find_map(ast::MacroCall::cast).unwrap(); - let (invocation_tt, _) = ast_to_token_tree(¯o_invocation.token_tree().unwrap()); + let (invocation_tt, _) = + syntax_node_to_token_tree(macro_invocation.token_tree().unwrap().syntax()); self.rules.expand(&invocation_tt).result() } @@ -100,7 +101,7 @@ macro_rules! impl_fixture { .descendants() .find_map(ast::TokenTree::cast) .unwrap(); - let mut wrapped = ast_to_token_tree(&wrapped).0; + let mut wrapped = syntax_node_to_token_tree(wrapped.syntax()).0; wrapped.delimiter = None; wrapped }; @@ -163,7 +164,8 @@ fn parse_macro_rules_to_tt(ra_fixture: &str) -> tt::Subtree { let macro_definition = source_file.syntax().descendants().find_map(ast::MacroRules::cast).unwrap(); - let (definition_tt, _) = ast_to_token_tree(¯o_definition.token_tree().unwrap()); + let (definition_tt, _) = + syntax_node_to_token_tree(macro_definition.token_tree().unwrap().syntax()); let parsed = parse_to_token_tree( &ra_fixture[macro_definition.token_tree().unwrap().syntax().text_range()], @@ -180,7 +182,7 @@ fn parse_macro_def_to_tt(ra_fixture: &str) -> tt::Subtree { let macro_definition = source_file.syntax().descendants().find_map(ast::MacroDef::cast).unwrap(); - let (definition_tt, _) = ast_to_token_tree(¯o_definition.body().unwrap()); + let (definition_tt, _) = syntax_node_to_token_tree(macro_definition.body().unwrap().syntax()); let parsed = parse_to_token_tree(&ra_fixture[macro_definition.body().unwrap().syntax().text_range()]) diff --git a/crates/mbe/src/tests/rule.rs b/crates/mbe/src/tests/rule.rs index 5c61a98fdf96..691e359e4d7a 100644 --- a/crates/mbe/src/tests/rule.rs +++ b/crates/mbe/src/tests/rule.rs @@ -1,7 +1,5 @@ use syntax::{ast, AstNode}; -use crate::ast_to_token_tree; - use super::*; #[test] @@ -44,6 +42,7 @@ fn parse_macro_arm(arm_definition: &str) -> Result