Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit 531e152

Browse files
VeykrilVeykril
committed
fix: Simplify macro statement expansion handling
1 parent f8c416e commit 531e152

File tree

10 files changed

+158
-113
lines changed

10 files changed

+158
-113
lines changed

‎crates/hir-def/src/body.rs‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ use la_arena::{Arena, ArenaMap};
1919
use limit::Limit;
2020
use profile::Count;
2121
use rustc_hash::FxHashMap;
22-
use smallvec::SmallVec;
2322
use syntax::{ast, AstNode, AstPtr, SyntaxNodePtr};
2423

2524
use crate::{
@@ -294,10 +293,6 @@ pub struct BodySourceMap {
294293
field_map: FxHashMap<InFile<AstPtr<ast::RecordExprField>>, ExprId>,
295294
field_map_back: FxHashMap<ExprId, InFile<AstPtr<ast::RecordExprField>>>,
296295

297-
/// Maps a macro call to its lowered expressions, a single one if it expands to an expression,
298-
/// or multiple if it expands to MacroStmts.
299-
macro_call_to_exprs: FxHashMap<InFile<AstPtr<ast::MacroCall>>, SmallVec<[ExprId; 1]>>,
300-
301296
expansions: FxHashMap<InFile<AstPtr<ast::MacroCall>>, HirFileId>,
302297

303298
/// Diagnostics accumulated during body lowering. These contain `AstPtr`s and so are stored in
@@ -466,9 +461,9 @@ impl BodySourceMap {
466461
self.field_map.get(&src).cloned()
467462
}
468463

469-
pub fn macro_expansion_expr(&self, node: InFile<&ast::MacroCall>) -> Option<&[ExprId]> {
470-
let src = node.map(AstPtr::new);
471-
self.macro_call_to_exprs.get(&src).map(|it| &**it)
464+
pub fn macro_expansion_expr(&self, node: InFile<&ast::MacroExpr>) -> Option<ExprId> {
465+
let src = node.map(AstPtr::new).map(AstPtr::upcast::<ast::MacroExpr>).map(AstPtr::upcast);
466+
self.expr_map.get(&src).copied()
472467
}
473468

474469
/// Get a reference to the body source map's diagnostics.

‎crates/hir-def/src/body/lower.rs‎

Lines changed: 61 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ use hir_expand::{
1313
use la_arena::Arena;
1414
use profile::Count;
1515
use rustc_hash::FxHashMap;
16-
use smallvec::smallvec;
1716
use syntax::{
1817
ast::{
1918
self, ArrayExprKind, AstChildren, HasArgList, HasLoopBody, HasName, LiteralKind,
@@ -97,7 +96,6 @@ pub(super) fn lower(
9796
or_pats: Default::default(),
9897
},
9998
expander,
100-
statements_in_scope: Vec::new(),
10199
name_to_pat_grouping: Default::default(),
102100
is_lowering_inside_or_pat: false,
103101
}
@@ -109,7 +107,6 @@ struct ExprCollector<'a> {
109107
expander: Expander,
110108
body: Body,
111109
source_map: BodySourceMap,
112-
statements_in_scope: Vec<Statement>,
113110
// a poor-mans union-find?
114111
name_to_pat_grouping: FxHashMap<Name, Vec<PatId>>,
115112
is_lowering_inside_or_pat: bool,
@@ -514,27 +511,25 @@ impl ExprCollector<'_> {
514511
ast::Expr::MacroExpr(e) => {
515512
let e = e.macro_call()?;
516513
let macro_ptr = AstPtr::new(&e);
517-
let id = self.collect_macro_call(e, macro_ptr.clone(), true, |this, expansion| {
514+
let id = self.collect_macro_call(e, macro_ptr, true, |this, expansion| {
518515
expansion.map(|it| this.collect_expr(it))
519516
});
520517
match id {
521518
Some(id) => {
522-
self.source_map
523-
.macro_call_to_exprs
524-
.insert(self.expander.to_source(macro_ptr), smallvec![id]);
519+
// Make the macro-call point to its expanded expression so we can query
520+
// semantics on syntax pointers to the macro
521+
let src = self.expander.to_source(syntax_ptr);
522+
self.source_map.expr_map.insert(src, id);
525523
id
526524
}
527-
None => self.alloc_expr(Expr::Missing, syntax_ptr.clone()),
525+
None => self.alloc_expr(Expr::Missing, syntax_ptr),
528526
}
529527
}
530528
ast::Expr::MacroStmts(e) => {
531-
e.statements().for_each(|s| self.collect_stmt(s));
532-
let tail = e
533-
.expr()
534-
.map(|e| self.collect_expr(e))
535-
.unwrap_or_else(|| self.alloc_expr(Expr::Missing, syntax_ptr.clone()));
529+
let statements = e.statements().filter_map(|s| self.collect_stmt(s)).collect();
530+
let tail = e.expr().map(|e| self.collect_expr(e));
536531

537-
self.alloc_expr(Expr::MacroStmts { tail }, syntax_ptr)
532+
self.alloc_expr(Expr::MacroStmts { tail, statements }, syntax_ptr)
538533
}
539534
ast::Expr::UnderscoreExpr(_) => self.alloc_expr(Expr::Underscore, syntax_ptr),
540535
})
@@ -607,11 +602,11 @@ impl ExprCollector<'_> {
607602
}
608603
}
609604

610-
fn collect_stmt(&mut self, s: ast::Stmt) {
605+
fn collect_stmt(&mut self, s: ast::Stmt) -> Option<Statement> {
611606
match s {
612607
ast::Stmt::LetStmt(stmt) => {
613608
if self.check_cfg(&stmt).is_none() {
614-
return;
609+
return None;
615610
}
616611
let pat = self.collect_pat_opt(stmt.pat());
617612
let type_ref =
@@ -621,70 +616,62 @@ impl ExprCollector<'_> {
621616
.let_else()
622617
.and_then(|let_else| let_else.block_expr())
623618
.map(|block| self.collect_block(block));
624-
self.statements_in_scope.push(Statement::Let {
625-
pat,
626-
type_ref,
627-
initializer,
628-
else_branch,
629-
});
619+
Some(Statement::Let { pat, type_ref, initializer, else_branch })
630620
}
631621
ast::Stmt::ExprStmt(stmt) => {
632-
if let Some(expr) = stmt.expr() {
633-
if self.check_cfg(&expr).is_none() {
634-
return;
622+
let expr = stmt.expr();
623+
if let Some(expr) = &expr {
624+
if self.check_cfg(expr).is_none() {
625+
return None;
635626
}
636627
}
637628
let has_semi = stmt.semicolon_token().is_some();
638-
// Note that macro could be expended to multiple statements
639-
if let Some(ast::Expr::MacroExpr(e)) = stmt.expr() {
640-
let m = match e.macro_call() {
641-
Some(it) => it,
642-
None => return,
643-
};
644-
let macro_ptr = AstPtr::new(&m);
645-
let syntax_ptr = AstPtr::new(&stmt.expr().unwrap());
646-
647-
let prev_stmt = self.statements_in_scope.len();
648-
self.collect_macro_call(m, macro_ptr.clone(), false, |this, expansion| {
649-
match expansion {
629+
// Note that macro could be expanded to multiple statements
630+
if let Some(expr @ ast::Expr::MacroExpr(mac)) = &expr {
631+
let mac_call = mac.macro_call()?;
632+
let syntax_ptr = AstPtr::new(expr);
633+
let macro_ptr = AstPtr::new(&mac_call);
634+
// let prev_stmt = self.statements_in_scope.len();
635+
let stmt = self.collect_macro_call(
636+
mac_call,
637+
macro_ptr,
638+
false,
639+
|this, expansion: Option<ast::MacroStmts>| match expansion {
650640
Some(expansion) => {
651-
let statements: ast::MacroStmts = expansion;
652-
653-
statements.statements().for_each(|stmt| this.collect_stmt(stmt));
654-
if let Some(expr) = statements.expr() {
655-
let expr = this.collect_expr(expr);
656-
this.statements_in_scope
657-
.push(Statement::Expr { expr, has_semi });
658-
}
641+
let statements = expansion
642+
.statements()
643+
.filter_map(|stmt| this.collect_stmt(stmt))
644+
.collect();
645+
let tail = expansion.expr().map(|expr| this.collect_expr(expr));
646+
647+
let mac_stmts = this.alloc_expr(
648+
Expr::MacroStmts { tail, statements },
649+
AstPtr::new(&ast::Expr::MacroStmts(expansion)),
650+
);
651+
652+
Some(mac_stmts)
659653
}
660-
None => {
661-
let expr = this.alloc_expr(Expr::Missing, syntax_ptr.clone());
662-
this.statements_in_scope.push(Statement::Expr { expr, has_semi });
663-
}
664-
}
665-
});
654+
None => None,
655+
},
656+
);
666657

667-
let mut macro_exprs = smallvec![];
668-
for stmt in &self.statements_in_scope[prev_stmt..] {
669-
match *stmt {
670-
Statement::Let { initializer, else_branch, .. } => {
671-
macro_exprs.extend(initializer);
672-
macro_exprs.extend(else_branch);
673-
}
674-
Statement::Expr { expr, .. } => macro_exprs.push(expr),
658+
let expr = match stmt {
659+
Some(expr) => {
660+
// Make the macro-call point to its expanded expression so we can query
661+
// semantics on syntax pointers to the macro
662+
let src = self.expander.to_source(syntax_ptr);
663+
self.source_map.expr_map.insert(src, expr);
664+
expr
675665
}
676-
}
677-
if !macro_exprs.is_empty() {
678-
self.source_map
679-
.macro_call_to_exprs
680-
.insert(self.expander.to_source(macro_ptr), macro_exprs);
681-
}
666+
None => self.alloc_expr(Expr::Missing, syntax_ptr),
667+
};
668+
Some(Statement::Expr { expr, has_semi })
682669
} else {
683-
let expr = self.collect_expr_opt(stmt.expr());
684-
self.statements_in_scope.push(Statement::Expr { expr, has_semi });
670+
let expr = self.collect_expr_opt(expr);
671+
Some(Statement::Expr { expr, has_semi })
685672
}
686673
}
687-
ast::Stmt::Item(_item) => {}
674+
ast::Stmt::Item(_item) => None,
688675
}
689676
}
690677

@@ -703,22 +690,10 @@ impl ExprCollector<'_> {
703690
};
704691
let prev_def_map = mem::replace(&mut self.expander.def_map, def_map);
705692
let prev_local_module = mem::replace(&mut self.expander.module, module);
706-
let prev_statements = std::mem::take(&mut self.statements_in_scope);
707-
708-
block.statements().for_each(|s| self.collect_stmt(s));
709-
block.tail_expr().and_then(|e| {
710-
let expr = self.maybe_collect_expr(e)?;
711-
self.statements_in_scope.push(Statement::Expr { expr, has_semi: false });
712-
Some(())
713-
});
714-
715-
let mut tail = None;
716-
if let Some(Statement::Expr { expr, has_semi: false }) = self.statements_in_scope.last() {
717-
tail = Some(*expr);
718-
self.statements_in_scope.pop();
719-
}
720-
let tail = tail;
721-
let statements = std::mem::replace(&mut self.statements_in_scope, prev_statements).into();
693+
694+
let statements = block.statements().filter_map(|s| self.collect_stmt(s)).collect();
695+
let tail = block.tail_expr().and_then(|e| self.maybe_collect_expr(e));
696+
722697
let syntax_node_ptr = AstPtr::new(&block.into());
723698
let expr_id = self.alloc_expr(
724699
Expr::Block { id: block_id, statements, tail, label: None },
@@ -903,10 +878,12 @@ impl ExprCollector<'_> {
903878
ast::Pat::MacroPat(mac) => match mac.macro_call() {
904879
Some(call) => {
905880
let macro_ptr = AstPtr::new(&call);
881+
let src = self.expander.to_source(Either::Left(AstPtr::new(&pat)));
906882
let pat =
907883
self.collect_macro_call(call, macro_ptr, true, |this, expanded_pat| {
908884
this.collect_pat_opt_(expanded_pat)
909885
});
886+
self.source_map.pat_map.insert(src, pat);
910887
return pat;
911888
}
912889
None => Pat::Missing,

‎crates/hir-def/src/expr.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,8 @@ pub enum Expr {
199199
body: ExprId,
200200
},
201201
MacroStmts {
202-
tail: ExprId,
202+
statements: Box<[Statement]>,
203+
tail: Option<ExprId>,
203204
},
204205
Array(Array),
205206
Literal(Literal),
@@ -254,7 +255,7 @@ impl Expr {
254255
Expr::Let { expr, .. } => {
255256
f(*expr);
256257
}
257-
Expr::Block { statements, tail, .. } => {
258+
Expr::MacroStmts { tail, statements } | Expr::Block { statements, tail, .. } => {
258259
for stmt in statements.iter() {
259260
match stmt {
260261
Statement::Let { initializer, .. } => {
@@ -344,7 +345,6 @@ impl Expr {
344345
f(*repeat)
345346
}
346347
},
347-
Expr::MacroStmts { tail } => f(*tail),
348348
Expr::Literal(_) => {}
349349
Expr::Underscore => {}
350350
}

‎crates/hir-ty/src/infer/expr.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -774,7 +774,9 @@ impl<'a> InferenceContext<'a> {
774774
None => self.table.new_float_var(),
775775
},
776776
},
777-
Expr::MacroStmts { tail } => self.infer_expr_inner(*tail, expected),
777+
Expr::MacroStmts { tail, statements } => {
778+
self.infer_block(tgt_expr, statements, *tail, expected)
779+
}
778780
Expr::Underscore => {
779781
// Underscore expressions may only appear in assignee expressions,
780782
// which are handled by `infer_assignee_expr()`, so any underscore

‎crates/hir/src/lib.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2223,6 +2223,7 @@ impl Local {
22232223
let src = source_map.pat_syntax(self.pat_id).unwrap(); // Hmm...
22242224
let root = src.file_syntax(db.upcast());
22252225
src.map(|ast| match ast {
2226+
// Suspicious unwrap
22262227
Either::Left(it) => Either::Left(it.cast().unwrap().to_node(&root)),
22272228
Either::Right(it) => Either::Right(it.to_node(&root)),
22282229
})

‎crates/hir/src/source_analyzer.rs‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -539,24 +539,26 @@ impl SourceAnalyzer {
539539
_ => (),
540540
}
541541
}
542+
let macro_expr = match macro_call
543+
.map(|it| it.syntax().parent().and_then(ast::MacroExpr::cast))
544+
.transpose()
545+
{
546+
Some(it) => it,
547+
None => return false,
548+
};
549+
542550
if let (Some((def, body, sm)), Some(infer)) = (&self.def, &self.infer) {
543-
if let Some(expr_ids) = sm.macro_expansion_expr(macro_call) {
551+
if let Some(expanded_expr) = sm.macro_expansion_expr(macro_expr.as_ref()) {
544552
let mut is_unsafe = false;
545-
for &expr_id in expr_ids {
546-
unsafe_expressions(
547-
db,
548-
infer,
549-
*def,
550-
body,
551-
expr_id,
552-
&mut |UnsafeExpr { inside_unsafe_block, .. }| {
553-
is_unsafe |= !inside_unsafe_block
554-
},
555-
);
556-
if is_unsafe {
557-
return true;
558-
}
559-
}
553+
unsafe_expressions(
554+
db,
555+
infer,
556+
*def,
557+
body,
558+
expanded_expr,
559+
&mut |UnsafeExpr { inside_unsafe_block, .. }| is_unsafe |= !inside_unsafe_block,
560+
);
561+
return is_unsafe;
560562
}
561563
}
562564
false

‎crates/ide-completion/src/tests/special.rs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -810,3 +810,33 @@ fn main() {
810810
"#]],
811811
)
812812
}
813+
814+
#[test]
815+
fn regression_12644() {
816+
check(
817+
r#"
818+
macro_rules! __rust_force_expr {
819+
($e:expr) => {
820+
$e
821+
};
822+
}
823+
macro_rules! vec {
824+
($elem:expr) => {
825+
__rust_force_expr!($elem)
826+
};
827+
}
828+
829+
struct Struct;
830+
impl Struct {
831+
fn foo(self) {}
832+
}
833+
834+
fn f() {
835+
vec![Struct].$0;
836+
}
837+
"#,
838+
expect![[r#"
839+
me foo() fn(self)
840+
"#]],
841+
);
842+
}

‎crates/ide/src/highlight_related.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,7 @@ fn foo() ->$0 u32 {
689689
never();
690690
// ^^^^^^^
691691
never!();
692-
// FIXME sema doesn't give us types for macrocalls
692+
// ^^^^^^^^
693693
694694
Never.never();
695695
// ^^^^^^^^^^^^^

0 commit comments

Comments
 (0)