Skip to content

Commit b5eaf56

Browse files
bors[bot]Jonas Schievink
bors[bot]
and
Jonas Schievink
authored
Merge #11904
11904: internal: Wrap macros in expr position in `MacroExpr` node r=jonas-schievink a=jonas-schievink This lets us distinguish them from macros in item position just by looking at the syntax tree. Fixes #11854 bors r+ Co-authored-by: Jonas Schievink <jonas.schievink@ferrous-systems.com>
2 parents 5a6918f + 872b7b9 commit b5eaf56

File tree

31 files changed

+636
-542
lines changed

31 files changed

+636
-542
lines changed

‎crates/hir/src/source_analyzer.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,8 @@ impl SourceAnalyzer {
110110

111111
fn expr_id(&self, db: &dyn HirDatabase, expr: &ast::Expr) -> Option<ExprId> {
112112
let src = match expr {
113-
ast::Expr::MacroCall(call) => {
114-
self.expand_expr(db, InFile::new(self.file_id, call.clone()))?
113+
ast::Expr::MacroExpr(expr) => {
114+
self.expand_expr(db, InFile::new(self.file_id, expr.macro_call()?.clone()))?
115115
}
116116
_ => InFile::new(self.file_id, expr.clone()),
117117
};

‎crates/hir_def/src/body/lower.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,8 @@ impl ExprCollector<'_> {
506506
None => self.alloc_expr(Expr::Missing, syntax_ptr),
507507
}
508508
}
509-
ast::Expr::MacroCall(e) => {
509+
ast::Expr::MacroExpr(e) => {
510+
let e = e.macro_call()?;
510511
let macro_ptr = AstPtr::new(&e);
511512
let id = self.collect_macro_call(e, macro_ptr.clone(), true, |this, expansion| {
512513
expansion.map(|it| this.collect_expr(it))
@@ -629,7 +630,11 @@ impl ExprCollector<'_> {
629630
}
630631
let has_semi = stmt.semicolon_token().is_some();
631632
// Note that macro could be expended to multiple statements
632-
if let Some(ast::Expr::MacroCall(m)) = stmt.expr() {
633+
if let Some(ast::Expr::MacroExpr(e)) = stmt.expr() {
634+
let m = match e.macro_call() {
635+
Some(it) => it,
636+
None => return,
637+
};
633638
let macro_ptr = AstPtr::new(&m);
634639
let syntax_ptr = AstPtr::new(&stmt.expr().unwrap());
635640

‎crates/hir_def/src/body/tests.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,34 @@ fn main() { n_nuple!(1,2,3); }
7979
);
8080
}
8181

82+
#[test]
83+
fn issue_3642_bad_macro_stackover() {
84+
lower(
85+
r#"
86+
#[macro_export]
87+
macro_rules! match_ast {
88+
(match $node:ident { $($tt:tt)* }) => { match_ast!(match ($node) { $($tt)* }) };
89+
90+
(match ($node:expr) {
91+
$( ast::$ast:ident($it:ident) => $res:expr, )*
92+
_ => $catch_all:expr $(,)?
93+
}) => {{
94+
$( if let Some($it) = ast::$ast::cast($node.clone()) { $res } else )*
95+
{ $catch_all }
96+
}};
97+
}
98+
99+
fn main() {
100+
let anchor = match_ast! {
101+
match parent {
102+
as => {},
103+
_ => return None
104+
}
105+
};
106+
}"#,
107+
);
108+
}
109+
82110
#[test]
83111
fn macro_resolve() {
84112
// Regression test for a path resolution bug introduced with inner item handling.

‎crates/hir_def/src/body/tests/block.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,3 +371,27 @@ fn outer() {
371371
"#]],
372372
);
373373
}
374+
375+
#[test]
376+
fn stmt_macro_expansion_with_trailing_expr() {
377+
cov_mark::check!(macro_stmt_with_trailing_macro_expr);
378+
check_at(
379+
r#"
380+
macro_rules! mac {
381+
() => { mac!($) };
382+
($x:tt) => { fn inner() {} };
383+
}
384+
fn foo() {
385+
mac!();
386+
$0
387+
}
388+
"#,
389+
expect![[r#"
390+
block scope
391+
inner: v
392+
393+
crate
394+
foo: v
395+
"#]],
396+
)
397+
}

‎crates/hir_def/src/item_tree/lower.rs

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,22 +48,33 @@ impl<'a> Ctx<'a> {
4848
pub(super) fn lower_macro_stmts(mut self, stmts: ast::MacroStmts) -> ItemTree {
4949
self.tree.top_level = stmts
5050
.statements()
51-
.filter_map(|stmt| match stmt {
52-
ast::Stmt::Item(item) => Some(item),
53-
// Macro calls can be both items and expressions. The syntax library always treats
54-
// them as expressions here, so we undo that.
55-
ast::Stmt::ExprStmt(es) => match es.expr()? {
56-
ast::Expr::MacroCall(call) => {
57-
cov_mark::hit!(macro_call_in_macro_stmts_is_added_to_item_tree);
58-
Some(call.into())
59-
}
51+
.filter_map(|stmt| {
52+
match stmt {
53+
ast::Stmt::Item(item) => Some(item),
54+
// Macro calls can be both items and expressions. The syntax library always treats
55+
// them as expressions here, so we undo that.
56+
ast::Stmt::ExprStmt(es) => match es.expr()? {
57+
ast::Expr::MacroExpr(expr) => {
58+
cov_mark::hit!(macro_call_in_macro_stmts_is_added_to_item_tree);
59+
Some(expr.macro_call()?.into())
60+
}
61+
_ => None,
62+
},
6063
_ => None,
61-
},
62-
_ => None,
64+
}
6365
})
6466
.flat_map(|item| self.lower_mod_item(&item))
6567
.collect();
6668

69+
if let Some(ast::Expr::MacroExpr(tail_macro)) = stmts.expr() {
70+
if let Some(call) = tail_macro.macro_call() {
71+
cov_mark::hit!(macro_stmt_with_trailing_macro_expr);
72+
if let Some(mod_item) = self.lower_mod_item(&call.into()) {
73+
self.tree.top_level.push(mod_item);
74+
}
75+
}
76+
}
77+
6778
self.tree
6879
}
6980

@@ -75,7 +86,7 @@ impl<'a> Ctx<'a> {
7586
// Macro calls can be both items and expressions. The syntax library always treats
7687
// them as expressions here, so we undo that.
7788
ast::Stmt::ExprStmt(es) => match es.expr()? {
78-
ast::Expr::MacroCall(call) => self.lower_mod_item(&call.into()),
89+
ast::Expr::MacroExpr(expr) => self.lower_mod_item(&expr.macro_call()?.into()),
7990
_ => None,
8091
},
8192
_ => None,

‎crates/hir_def/src/macro_expansion_tests/builtin_fn_macro.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,8 +249,7 @@ macro_rules! format_args {
249249
250250
fn main() {
251251
let _ =
252-
// +errors
253-
format_args!("{} {:?}", a.);
252+
format_args!/*+errors*/("{} {:?}", a.);
254253
}
255254
"#,
256255
expect![[r##"

‎crates/hir_def/src/macro_expansion_tests/mbe.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -530,8 +530,7 @@ macro_rules! m {
530530
}
531531
532532
fn f() -> i32 {
533-
// +tree
534-
m!{}
533+
m!/*+tree*/{}
535534
}
536535
"#,
537536
expect![[r#"

‎crates/hir_expand/src/lib.rs

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -885,6 +885,16 @@ impl ExpandTo {
885885
None => return ExpandTo::Statements,
886886
};
887887

888+
// FIXME: macros in statement position are treated as expression statements, they should
889+
// probably be their own statement kind. The *grand*parent indicates what's valid.
890+
if parent.kind() == MACRO_EXPR
891+
&& parent
892+
.parent()
893+
.map_or(true, |p| matches!(p.kind(), EXPR_STMT | STMT_LIST | MACRO_STMTS))
894+
{
895+
return ExpandTo::Statements;
896+
}
897+
888898
match parent.kind() {
889899
MACRO_ITEMS | SOURCE_FILE | ITEM_LIST => ExpandTo::Items,
890900
MACRO_STMTS | EXPR_STMT | STMT_LIST => ExpandTo::Statements,
@@ -895,23 +905,10 @@ impl ExpandTo {
895905
| CLOSURE_EXPR | FIELD_EXPR | FOR_EXPR | IF_EXPR | INDEX_EXPR | LET_EXPR
896906
| MATCH_ARM | MATCH_EXPR | MATCH_GUARD | METHOD_CALL_EXPR | PAREN_EXPR | PATH_EXPR
897907
| PREFIX_EXPR | RANGE_EXPR | RECORD_EXPR_FIELD | REF_EXPR | RETURN_EXPR | TRY_EXPR
898-
| TUPLE_EXPR | WHILE_EXPR => ExpandTo::Expr,
908+
| TUPLE_EXPR | WHILE_EXPR | MACRO_EXPR => ExpandTo::Expr,
899909
_ => {
900-
match ast::LetStmt::cast(parent) {
901-
Some(let_stmt) => {
902-
if let Some(true) = let_stmt.initializer().map(|it| it.syntax() == syn) {
903-
ExpandTo::Expr
904-
} else if let Some(true) = let_stmt.ty().map(|it| it.syntax() == syn) {
905-
ExpandTo::Type
906-
} else {
907-
ExpandTo::Pattern
908-
}
909-
}
910-
None => {
911-
// Unknown , Just guess it is `Items`
912-
ExpandTo::Items
913-
}
914-
}
910+
// Unknown , Just guess it is `Items`
911+
ExpandTo::Items
915912
}
916913
}
917914
}

‎crates/hir_ty/src/tests/regression.rs

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -444,34 +444,6 @@ fn test() {
444444
);
445445
}
446446

447-
#[test]
448-
fn issue_3642_bad_macro_stackover() {
449-
check_no_mismatches(
450-
r#"
451-
#[macro_export]
452-
macro_rules! match_ast {
453-
(match $node:ident { $($tt:tt)* }) => { match_ast!(match ($node) { $($tt)* }) };
454-
455-
(match ($node:expr) {
456-
$( ast::$ast:ident($it:ident) => $res:expr, )*
457-
_ => $catch_all:expr $(,)?
458-
}) => {{
459-
$( if let Some($it) = ast::$ast::cast($node.clone()) { $res } else )*
460-
{ $catch_all }
461-
}};
462-
}
463-
464-
fn main() {
465-
let anchor = match_ast! {
466-
match parent {
467-
as => {},
468-
_ => return None
469-
}
470-
};
471-
}"#,
472-
);
473-
}
474-
475447
#[test]
476448
fn issue_3999_slice() {
477449
check_infer(

‎crates/ide/src/highlight_related.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ fn highlight_exit_points(
156156
highlights.push(HighlightedRange { category: None, range: token.text_range() });
157157
}
158158
}
159-
ast::Expr::MethodCallExpr(_) | ast::Expr::CallExpr(_) | ast::Expr::MacroCall(_) => {
159+
ast::Expr::MethodCallExpr(_) | ast::Expr::CallExpr(_) | ast::Expr::MacroExpr(_) => {
160160
if sema.type_of_expr(&expr).map_or(false, |ty| ty.original.is_never()) {
161161
highlights.push(HighlightedRange {
162162
category: None,

0 commit comments

Comments
 (0)