From 278cbf12cd0f76fc191d5ce7f130e6245596a578 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Sun, 24 May 2020 16:24:36 -0400 Subject: Track unsafe blocks, don't trigger missing unsafe diagnostic when unsafe exprs within unsafe block --- crates/ra_hir_def/src/body/lower.rs | 6 +++++- crates/ra_hir_def/src/expr.rs | 5 ++++- 2 files changed, 9 insertions(+), 2 deletions(-) (limited to 'crates/ra_hir_def/src') diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index a7e2e0982..174bbf9f4 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -218,8 +218,12 @@ impl ExprCollector<'_> { let body = self.collect_block_opt(e.block_expr()); self.alloc_expr(Expr::TryBlock { body }, syntax_ptr) } + ast::Effect::Unsafe(_) => { + let body = self.collect_block_opt(e.block_expr()); + self.alloc_expr(Expr::UnsafeBlock { body }, syntax_ptr) + } // FIXME: we need to record these effects somewhere... - ast::Effect::Async(_) | ast::Effect::Label(_) | ast::Effect::Unsafe(_) => { + ast::Effect::Async(_) | ast::Effect::Label(_) => { self.collect_block_opt(e.block_expr()) } }, diff --git a/crates/ra_hir_def/src/expr.rs b/crates/ra_hir_def/src/expr.rs index ca49b26d1..f5e3e74fb 100644 --- a/crates/ra_hir_def/src/expr.rs +++ b/crates/ra_hir_def/src/expr.rs @@ -150,6 +150,9 @@ pub enum Expr { Tuple { exprs: Vec, }, + UnsafeBlock { + body: ExprId, + }, Array(Array), Literal(Literal), } @@ -247,7 +250,7 @@ impl Expr { f(*expr); } } - Expr::TryBlock { body } => f(*body), + Expr::TryBlock { body } | Expr::UnsafeBlock { body } => f(*body), Expr::Loop { body, .. } => f(*body), Expr::While { condition, body, .. } => { f(*condition); -- cgit v1.2.3 From b9569886a915f2411adaecbcad28eda8b163c3e3 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Sun, 24 May 2020 16:28:02 -0400 Subject: Rename Expr::UnsafeBlock to Expr::Unsafe --- crates/ra_hir_def/src/body/lower.rs | 2 +- crates/ra_hir_def/src/expr.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'crates/ra_hir_def/src') diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index 174bbf9f4..6eb72e950 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -220,7 +220,7 @@ impl ExprCollector<'_> { } ast::Effect::Unsafe(_) => { let body = self.collect_block_opt(e.block_expr()); - self.alloc_expr(Expr::UnsafeBlock { body }, syntax_ptr) + self.alloc_expr(Expr::Unsafe { body }, syntax_ptr) } // FIXME: we need to record these effects somewhere... ast::Effect::Async(_) | ast::Effect::Label(_) => { diff --git a/crates/ra_hir_def/src/expr.rs b/crates/ra_hir_def/src/expr.rs index f5e3e74fb..e41cfc16b 100644 --- a/crates/ra_hir_def/src/expr.rs +++ b/crates/ra_hir_def/src/expr.rs @@ -150,7 +150,7 @@ pub enum Expr { Tuple { exprs: Vec, }, - UnsafeBlock { + Unsafe { body: ExprId, }, Array(Array), @@ -250,7 +250,7 @@ impl Expr { f(*expr); } } - Expr::TryBlock { body } | Expr::UnsafeBlock { body } => f(*body), + Expr::TryBlock { body } | Expr::Unsafe { body } => f(*body), Expr::Loop { body, .. } => f(*body), Expr::While { condition, body, .. } => { f(*condition); -- cgit v1.2.3 From 7f2219dc76d6cddc46eadb2b2dd674795e0efce8 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Wed, 27 May 2020 22:21:20 -0400 Subject: Track expr parents during lowering, use parent map when checking if unsafe exprs are within unsafe blocks --- crates/ra_hir_def/src/body.rs | 1 + crates/ra_hir_def/src/body/lower.rs | 252 +++++++++++++++++++++++------------- 2 files changed, 165 insertions(+), 88 deletions(-) (limited to 'crates/ra_hir_def/src') diff --git a/crates/ra_hir_def/src/body.rs b/crates/ra_hir_def/src/body.rs index 4f2350915..076d1a4fa 100644 --- a/crates/ra_hir_def/src/body.rs +++ b/crates/ra_hir_def/src/body.rs @@ -184,6 +184,7 @@ pub struct Body { /// The `ExprId` of the actual body expression. pub body_expr: ExprId, pub item_scope: ItemScope, + pub parent_map: FxHashMap, } pub type ExprPtr = AstPtr; diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index 6eb72e950..a1678adeb 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -15,6 +15,7 @@ use ra_syntax::{ }, AstNode, AstPtr, }; +use rustc_hash::FxHashMap; use test_utils::mark; use crate::{ @@ -74,6 +75,7 @@ pub(super) fn lower( params: Vec::new(), body_expr: dummy_expr_id(), item_scope: Default::default(), + parent_map: FxHashMap::default(), }, item_trees: { let mut map = FxHashMap::default(); @@ -171,11 +173,28 @@ impl ExprCollector<'_> { id } + fn update_parent_map( + &mut self, + (parent_expr, children_exprs): (ExprId, Vec), + ) -> ExprId { + for child_expr in children_exprs { + self.body.parent_map.insert(child_expr, parent_expr); + } + + parent_expr + } + fn collect_expr(&mut self, expr: ast::Expr) -> ExprId { + let parent_and_children = self.collect_expr_inner(expr); + self.update_parent_map(parent_and_children) + } + + fn collect_expr_inner(&mut self, expr: ast::Expr) -> (ExprId, Vec) { let syntax_ptr = AstPtr::new(&expr); if !self.expander.is_cfg_enabled(&expr) { - return self.missing_expr(); + return (self.missing_expr(), vec![]); } + match expr { ast::Expr::IfExpr(e) => { let then_branch = self.collect_block_opt(e.then_branch()); @@ -205,32 +224,48 @@ impl ExprCollector<'_> { guard: None, }, ]; - return self - .alloc_expr(Expr::Match { expr: match_expr, arms }, syntax_ptr); + let children_exprs = if let Some(else_branch) = else_branch { + vec![match_expr, then_branch, else_branch] + } else { + vec![match_expr, then_branch] + }; + return ( + self.alloc_expr(Expr::Match { expr: match_expr, arms }, syntax_ptr), + children_exprs, + ); } }, }; - self.alloc_expr(Expr::If { condition, then_branch, else_branch }, syntax_ptr) + let children_exprs = if let Some(else_branch) = else_branch { + vec![then_branch, else_branch, condition] + } else { + vec![then_branch, condition] + }; + + ( + self.alloc_expr(Expr::If { condition, then_branch, else_branch }, syntax_ptr), + children_exprs, + ) } ast::Expr::EffectExpr(e) => match e.effect() { ast::Effect::Try(_) => { let body = self.collect_block_opt(e.block_expr()); - self.alloc_expr(Expr::TryBlock { body }, syntax_ptr) + (self.alloc_expr(Expr::TryBlock { body }, syntax_ptr), vec![body]) } ast::Effect::Unsafe(_) => { let body = self.collect_block_opt(e.block_expr()); - self.alloc_expr(Expr::Unsafe { body }, syntax_ptr) + (self.alloc_expr(Expr::Unsafe { body }, syntax_ptr), vec![body]) } // FIXME: we need to record these effects somewhere... ast::Effect::Async(_) | ast::Effect::Label(_) => { - self.collect_block_opt(e.block_expr()) + (self.collect_block_opt(e.block_expr()), vec![]) } }, - ast::Expr::BlockExpr(e) => self.collect_block(e), + ast::Expr::BlockExpr(e) => (self.collect_block(e), vec![]), ast::Expr::LoopExpr(e) => { let body = self.collect_block_opt(e.loop_body()); - self.alloc_expr( + (self.alloc_expr( Expr::Loop { body, label: e @@ -239,7 +274,7 @@ impl ExprCollector<'_> { .map(|l| Name::new_lifetime(&l)), }, syntax_ptr, - ) + ), vec![body]) } ast::Expr::WhileExpr(e) => { let body = self.collect_block_opt(e.loop_body()); @@ -250,6 +285,7 @@ impl ExprCollector<'_> { None => self.collect_expr_opt(condition.expr()), // if let -- desugar to match Some(pat) => { + // FIXME(pfaria) track the break and arms parents here? mark::hit!(infer_resolve_while_let); let pat = self.collect_pat(pat); let match_expr = self.collect_expr_opt(condition.expr()); @@ -262,7 +298,7 @@ impl ExprCollector<'_> { ]; let match_expr = self.alloc_expr_desugared(Expr::Match { expr: match_expr, arms }); - return self.alloc_expr( + return (self.alloc_expr( Expr::Loop { body: match_expr, label: e @@ -271,12 +307,12 @@ impl ExprCollector<'_> { .map(|l| Name::new_lifetime(&l)), }, syntax_ptr, - ); + ), vec![match_expr]); } }, }; - self.alloc_expr( + (self.alloc_expr( Expr::While { condition, body, @@ -286,13 +322,13 @@ impl ExprCollector<'_> { .map(|l| Name::new_lifetime(&l)), }, syntax_ptr, - ) + ), vec![body, condition]) } ast::Expr::ForExpr(e) => { let iterable = self.collect_expr_opt(e.iterable()); let pat = self.collect_pat_opt(e.pat()); let body = self.collect_block_opt(e.loop_body()); - self.alloc_expr( + (self.alloc_expr( Expr::For { iterable, pat, @@ -303,7 +339,7 @@ impl ExprCollector<'_> { .map(|l| Name::new_lifetime(&l)), }, syntax_ptr, - ) + ), vec![iterable, body]) } ast::Expr::CallExpr(e) => { let callee = self.collect_expr_opt(e.expr()); @@ -312,41 +348,56 @@ impl ExprCollector<'_> { } else { Vec::new() }; - self.alloc_expr(Expr::Call { callee, args }, syntax_ptr) + let mut children_exprs = args.clone(); + children_exprs.push(callee); + (self.alloc_expr(Expr::Call { callee, args }, syntax_ptr), children_exprs) } ast::Expr::MethodCallExpr(e) => { let receiver = self.collect_expr_opt(e.expr()); let args = if let Some(arg_list) = e.arg_list() { arg_list.args().map(|e| self.collect_expr(e)).collect() } else { - Vec::new() + vec![] }; let method_name = e.name_ref().map(|nr| nr.as_name()).unwrap_or_else(Name::missing); let generic_args = e.type_arg_list().and_then(|it| GenericArgs::from_ast(&self.ctx(), it)); - self.alloc_expr( - Expr::MethodCall { receiver, method_name, args, generic_args }, - syntax_ptr, + let mut children_exprs = args.clone(); + children_exprs.push(receiver); + ( + self.alloc_expr( + Expr::MethodCall { receiver, method_name, args, generic_args }, + syntax_ptr, + ), + children_exprs, ) } ast::Expr::MatchExpr(e) => { let expr = self.collect_expr_opt(e.expr()); - let arms = if let Some(match_arm_list) = e.match_arm_list() { - match_arm_list - .arms() - .map(|arm| MatchArm { - pat: self.collect_pat_opt(arm.pat()), - expr: self.collect_expr_opt(arm.expr()), - guard: arm - .guard() - .and_then(|guard| guard.expr()) - .map(|e| self.collect_expr(e)), - }) - .collect() - } else { - Vec::new() - }; - self.alloc_expr(Expr::Match { expr, arms }, syntax_ptr) + let (arms, mut children_exprs): (Vec<_>, Vec<_>) = + if let Some(match_arm_list) = e.match_arm_list() { + match_arm_list + .arms() + .map(|arm| { + let expr = self.collect_expr_opt(arm.expr()); + ( + MatchArm { + pat: self.collect_pat_opt(arm.pat()), + expr, + guard: arm + .guard() + .and_then(|guard| guard.expr()) + .map(|e| self.collect_expr(e)), + }, + expr, + ) + }) + .unzip() + } else { + (vec![], vec![]) + }; + children_exprs.push(expr); + (self.alloc_expr(Expr::Match { expr, arms }, syntax_ptr), children_exprs) } ast::Expr::PathExpr(e) => { let path = e @@ -354,35 +405,35 @@ impl ExprCollector<'_> { .and_then(|path| self.expander.parse_path(path)) .map(Expr::Path) .unwrap_or(Expr::Missing); - self.alloc_expr(path, syntax_ptr) + (self.alloc_expr(path, syntax_ptr), vec![]) } - ast::Expr::ContinueExpr(e) => self.alloc_expr( + ast::Expr::ContinueExpr(e) => (self.alloc_expr( Expr::Continue { label: e.lifetime_token().map(|l| Name::new_lifetime(&l)) }, syntax_ptr, - ), + ), vec![]), ast::Expr::BreakExpr(e) => { let expr = e.expr().map(|e| self.collect_expr(e)); - self.alloc_expr( + (self.alloc_expr( Expr::Break { expr, label: e.lifetime_token().map(|l| Name::new_lifetime(&l)) }, syntax_ptr, - ) + ), expr.into_iter().collect()) } ast::Expr::ParenExpr(e) => { let inner = self.collect_expr_opt(e.expr()); // make the paren expr point to the inner expression as well let src = self.expander.to_source(syntax_ptr); self.source_map.expr_map.insert(src, inner); - inner + (inner, vec![]) } ast::Expr::ReturnExpr(e) => { let expr = e.expr().map(|e| self.collect_expr(e)); - self.alloc_expr(Expr::Return { expr }, syntax_ptr) + (self.alloc_expr(Expr::Return { expr }, syntax_ptr), expr.into_iter().collect()) } ast::Expr::RecordLit(e) => { let path = e.path().and_then(|path| self.expander.parse_path(path)); let mut field_ptrs = Vec::new(); - let record_lit = if let Some(nfl) = e.record_field_list() { - let fields = nfl + let (record_lit, children) = if let Some(nfl) = e.record_field_list() { + let (fields, children): (Vec<_>, Vec<_>) = nfl .fields() .inspect(|field| field_ptrs.push(AstPtr::new(field))) .filter_map(|field| { @@ -391,19 +442,20 @@ impl ExprCollector<'_> { } let name = field.field_name()?.as_name(); - Some(RecordLitField { - name, - expr: match field.expr() { - Some(e) => self.collect_expr(e), - None => self.missing_expr(), - }, - }) + let expr = match field.expr() { + Some(e) => self.collect_expr(e), + None => self.missing_expr(), + }; + Some((RecordLitField { name, expr }, expr)) }) - .collect(); + .unzip(); let spread = nfl.spread().map(|s| self.collect_expr(s)); - Expr::RecordLit { path, fields, spread } + ( + Expr::RecordLit { path, fields, spread: spread }, + children.into_iter().chain(spread.into_iter()).collect(), + ) } else { - Expr::RecordLit { path, fields: Vec::new(), spread: None } + (Expr::RecordLit { path, fields: Vec::new(), spread: None }, vec![]) }; let res = self.alloc_expr(record_lit, syntax_ptr); @@ -411,7 +463,7 @@ impl ExprCollector<'_> { let src = self.expander.to_source(ptr); self.source_map.field_map.insert((res, i), src); } - res + (res, children) } ast::Expr::FieldExpr(e) => { let expr = self.collect_expr_opt(e.expr()); @@ -419,20 +471,20 @@ impl ExprCollector<'_> { Some(kind) => kind.as_name(), _ => Name::missing(), }; - self.alloc_expr(Expr::Field { expr, name }, syntax_ptr) + (self.alloc_expr(Expr::Field { expr, name }, syntax_ptr), vec![expr]) } ast::Expr::AwaitExpr(e) => { let expr = self.collect_expr_opt(e.expr()); - self.alloc_expr(Expr::Await { expr }, syntax_ptr) + (self.alloc_expr(Expr::Await { expr }, syntax_ptr), vec![expr]) } ast::Expr::TryExpr(e) => { let expr = self.collect_expr_opt(e.expr()); - self.alloc_expr(Expr::Try { expr }, syntax_ptr) + (self.alloc_expr(Expr::Try { expr }, syntax_ptr), vec![expr]) } ast::Expr::CastExpr(e) => { let expr = self.collect_expr_opt(e.expr()); let type_ref = TypeRef::from_ast_opt(&self.ctx(), e.type_ref()); - self.alloc_expr(Expr::Cast { expr, type_ref }, syntax_ptr) + (self.alloc_expr(Expr::Cast { expr, type_ref }, syntax_ptr), vec![expr]) } ast::Expr::RefExpr(e) => { let expr = self.collect_expr_opt(e.expr()); @@ -455,9 +507,9 @@ impl ExprCollector<'_> { ast::Expr::PrefixExpr(e) => { let expr = self.collect_expr_opt(e.expr()); if let Some(op) = e.op_kind() { - self.alloc_expr(Expr::UnaryOp { expr, op }, syntax_ptr) + (self.alloc_expr(Expr::UnaryOp { expr, op }, syntax_ptr), vec![expr]) } else { - self.alloc_expr(Expr::Missing, syntax_ptr) + (self.alloc_expr(Expr::Missing, syntax_ptr), vec![]) } } ast::Expr::LambdaExpr(e) => { @@ -477,21 +529,24 @@ impl ExprCollector<'_> { .and_then(|r| r.type_ref()) .map(|it| TypeRef::from_ast(&self.ctx(), it)); let body = self.collect_expr_opt(e.body()); - self.alloc_expr(Expr::Lambda { args, arg_types, ret_type, body }, syntax_ptr) + ( + self.alloc_expr(Expr::Lambda { args, arg_types, ret_type, body }, syntax_ptr), + vec![body], + ) } ast::Expr::BinExpr(e) => { let lhs = self.collect_expr_opt(e.lhs()); let rhs = self.collect_expr_opt(e.rhs()); let op = e.op_kind().map(BinaryOp::from); - self.alloc_expr(Expr::BinaryOp { lhs, rhs, op }, syntax_ptr) + (self.alloc_expr(Expr::BinaryOp { lhs, rhs, op }, syntax_ptr), vec![lhs, rhs]) } ast::Expr::TupleExpr(e) => { - let exprs = e.exprs().map(|expr| self.collect_expr(expr)).collect(); - self.alloc_expr(Expr::Tuple { exprs }, syntax_ptr) + let exprs = e.exprs().map(|expr| self.collect_expr(expr)).collect::>(); + (self.alloc_expr(Expr::Tuple { exprs: exprs.clone() }, syntax_ptr), exprs) } ast::Expr::BoxExpr(e) => { let expr = self.collect_expr_opt(e.expr()); - self.alloc_expr(Expr::Box { expr }, syntax_ptr) + (self.alloc_expr(Expr::Box { expr }, syntax_ptr), vec![expr]) } ast::Expr::ArrayExpr(e) => { @@ -499,34 +554,46 @@ impl ExprCollector<'_> { match kind { ArrayExprKind::ElementList(e) => { - let exprs = e.map(|expr| self.collect_expr(expr)).collect(); - self.alloc_expr(Expr::Array(Array::ElementList(exprs)), syntax_ptr) + let exprs = e.map(|expr| self.collect_expr(expr)).collect::>(); + ( + self.alloc_expr( + Expr::Array(Array::ElementList(exprs.clone())), + syntax_ptr, + ), + exprs, + ) } ArrayExprKind::Repeat { initializer, repeat } => { let initializer = self.collect_expr_opt(initializer); let repeat = self.collect_expr_opt(repeat); - self.alloc_expr( - Expr::Array(Array::Repeat { initializer, repeat }), - syntax_ptr, + ( + self.alloc_expr( + Expr::Array(Array::Repeat { initializer, repeat }), + syntax_ptr, + ), + vec![initializer, repeat], ) } } } - ast::Expr::Literal(e) => self.alloc_expr(Expr::Literal(e.kind().into()), syntax_ptr), + ast::Expr::Literal(e) => { + (self.alloc_expr(Expr::Literal(e.kind().into()), syntax_ptr), vec![]) + } ast::Expr::IndexExpr(e) => { let base = self.collect_expr_opt(e.base()); let index = self.collect_expr_opt(e.index()); - self.alloc_expr(Expr::Index { base, index }, syntax_ptr) + (self.alloc_expr(Expr::Index { base, index }, syntax_ptr), vec![base, index]) } ast::Expr::RangeExpr(e) => { let lhs = e.start().map(|lhs| self.collect_expr(lhs)); let rhs = e.end().map(|rhs| self.collect_expr(rhs)); match e.op_kind() { - Some(range_type) => { - self.alloc_expr(Expr::Range { lhs, rhs, range_type }, syntax_ptr) - } - None => self.alloc_expr(Expr::Missing, syntax_ptr), + Some(range_type) => ( + self.alloc_expr(Expr::Range { lhs, rhs, range_type }, syntax_ptr), + lhs.into_iter().chain(rhs.into_iter()).collect(), + ), + None => (self.alloc_expr(Expr::Missing, syntax_ptr), vec![]), } } ast::Expr::MacroCall(e) => { @@ -540,7 +607,7 @@ impl ExprCollector<'_> { self.body.item_scope.define_legacy_macro(name, mac); // FIXME: do we still need to allocate this as missing ? - self.alloc_expr(Expr::Missing, syntax_ptr) + (self.alloc_expr(Expr::Missing, syntax_ptr), vec![]) } else { let macro_call = self.expander.to_source(AstPtr::new(&e)); match self.expander.enter_expand(self.db, Some(&self.body.item_scope), e) { @@ -553,15 +620,15 @@ impl ExprCollector<'_> { self.item_trees.insert(self.expander.current_file_id, item_tree); let id = self.collect_expr(expansion); self.expander.exit(self.db, mark); - id + (id, vec![]) } - None => self.alloc_expr(Expr::Missing, syntax_ptr), + None => (self.alloc_expr(Expr::Missing, syntax_ptr), vec![]), } } } // FIXME implement HIR for these: - ast::Expr::Label(_e) => self.alloc_expr(Expr::Missing, syntax_ptr), + ast::Expr::Label(_e) => (self.alloc_expr(Expr::Missing, syntax_ptr), vec![]), } } @@ -600,9 +667,14 @@ impl ExprCollector<'_> { } fn collect_block(&mut self, block: ast::BlockExpr) -> ExprId { + let parent_and_children = self.collect_block_inner(block); + self.update_parent_map(parent_and_children) + } + + fn collect_block_inner(&mut self, block: ast::BlockExpr) -> (ExprId, Vec) { let syntax_node_ptr = AstPtr::new(&block.clone().into()); self.collect_block_items(&block); - let statements = block + let (statements, children_exprs): (Vec<_>, Vec<_>) = block .statements() .map(|s| match s { ast::Stmt::LetStmt(stmt) => { @@ -610,14 +682,18 @@ impl ExprCollector<'_> { let type_ref = stmt.ascribed_type().map(|it| TypeRef::from_ast(&self.ctx(), it)); let initializer = stmt.initializer().map(|e| self.collect_expr(e)); - Statement::Let { pat, type_ref, initializer } + (Statement::Let { pat, type_ref, initializer }, initializer) + } + ast::Stmt::ExprStmt(stmt) => { + let expr = self.collect_expr_opt(stmt.expr()); + (Statement::Expr(expr), Some(expr)) } - ast::Stmt::ExprStmt(stmt) => Statement::Expr(self.collect_expr_opt(stmt.expr())), }) - .collect(); + .unzip(); let tail = block.expr().map(|e| self.collect_expr(e)); let label = block.label().and_then(|l| l.lifetime_token()).map(|t| Name::new_lifetime(&t)); - self.alloc_expr(Expr::Block { statements, tail, label }, syntax_node_ptr) + let children_exprs = children_exprs.into_iter().flatten().chain(tail.into_iter()).collect(); + (self.alloc_expr(Expr::Block { statements, tail, label }, syntax_node_ptr), children_exprs) } fn collect_block_items(&mut self, block: &ast::BlockExpr) { -- cgit v1.2.3 From 2608a6fd3a024206d4776cc391e46ef28c018434 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Fri, 29 May 2020 08:55:47 -0400 Subject: unsafe: Clean up, improve tracking, add debug_assert Move unsafe_expressions to unsafe_validation.rs, replace vec tracking of child exprs with inline macro, add debug assert to ensure tracked children match walked children exactly --- crates/ra_hir_def/src/body.rs | 2 +- crates/ra_hir_def/src/body/lower.rs | 237 ++++++++++++++++++++++++------------ 2 files changed, 158 insertions(+), 81 deletions(-) (limited to 'crates/ra_hir_def/src') diff --git a/crates/ra_hir_def/src/body.rs b/crates/ra_hir_def/src/body.rs index 076d1a4fa..14a1f4773 100644 --- a/crates/ra_hir_def/src/body.rs +++ b/crates/ra_hir_def/src/body.rs @@ -184,7 +184,7 @@ pub struct Body { /// The `ExprId` of the actual body expression. pub body_expr: ExprId, pub item_scope: ItemScope, - pub parent_map: FxHashMap, + pub parent_map: ArenaMap, } pub type ExprPtr = AstPtr; diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index a1678adeb..114b38710 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -7,7 +7,7 @@ use hir_expand::{ name::{name, AsName, Name}, HirFileId, MacroDefId, MacroDefKind, }; -use ra_arena::Arena; +use ra_arena::{map::ArenaMap, Arena}; use ra_syntax::{ ast::{ self, ArgListOwner, ArrayExprKind, LiteralKind, LoopBodyOwner, ModuleItemOwner, NameOwner, @@ -15,7 +15,6 @@ use ra_syntax::{ }, AstNode, AstPtr, }; -use rustc_hash::FxHashMap; use test_utils::mark; use crate::{ @@ -75,7 +74,7 @@ pub(super) fn lower( params: Vec::new(), body_expr: dummy_expr_id(), item_scope: Default::default(), - parent_map: FxHashMap::default(), + parent_map: ArenaMap::default(), }, item_trees: { let mut map = FxHashMap::default(); @@ -97,6 +96,40 @@ struct ExprCollector<'a> { item_trees: FxHashMap>, } +macro_rules! track_parent { + (@build $collector:ident, $parent:expr $(,)?) => { + $parent + }; + (@build $collector:ident, $parent:expr, opt $expr:ident $($rest:tt)*) => { + { + if let Some(expr) = $expr { + $collector.body.parent_map.insert(expr, $parent); + } + track_parent!(@build $collector, $parent $($rest)*) + } + }; + (@build $collector:ident, $parent:expr, vec $expr:ident $($rest:tt)*) => { + { + for expr in $expr { + $collector.body.parent_map.insert(expr, $parent); + } + track_parent!(@build $collector, $parent $($rest)*) + } + }; + (@build $collector:ident, $parent:expr, $expr:ident $($rest:tt)*) => { + { + $collector.body.parent_map.insert($expr, $parent); + track_parent!(@build $collector, $parent $($rest)*) + } + }; + ($collector:ident, $parent:expr, $($rest:tt)*) => { + { + let parent = $parent; + track_parent!(@build $collector, parent, $($rest)*) + } + } +} + impl ExprCollector<'_> { fn collect( mut self, @@ -185,14 +218,42 @@ impl ExprCollector<'_> { } fn collect_expr(&mut self, expr: ast::Expr) -> ExprId { - let parent_and_children = self.collect_expr_inner(expr); - self.update_parent_map(parent_and_children) + let expr_id = self.collect_expr_inner(expr); + + debug_assert!({ + let mut found_count = 0; + let mut incr = || { + found_count += 1; + true + }; + let mut all_children_found = true; + self.body[expr_id].walk_child_exprs(|child| { + all_children_found = all_children_found + && self + .body + .parent_map + .get(child) + .map(|parent| *parent == expr_id) + .unwrap_or(false) + && incr() + }); + + if all_children_found { + let child_count_in_map = + self.body.parent_map.iter().filter(|&(_, parent)| *parent == expr_id).count(); + found_count == child_count_in_map + } else { + false + } + }); + + expr_id } - fn collect_expr_inner(&mut self, expr: ast::Expr) -> (ExprId, Vec) { + fn collect_expr_inner(&mut self, expr: ast::Expr) -> ExprId { let syntax_ptr = AstPtr::new(&expr); if !self.expander.is_cfg_enabled(&expr) { - return (self.missing_expr(), vec![]); + return self.missing_expr(); } match expr { @@ -224,48 +285,40 @@ impl ExprCollector<'_> { guard: None, }, ]; - let children_exprs = if let Some(else_branch) = else_branch { - vec![match_expr, then_branch, else_branch] - } else { - vec![match_expr, then_branch] - }; - return ( + let arm_exprs = arms.iter().map(|arm| arm.expr).collect::>(); + return track_parent!( + self, self.alloc_expr(Expr::Match { expr: match_expr, arms }, syntax_ptr), - children_exprs, + match_expr, vec arm_exprs ); } }, }; - let children_exprs = if let Some(else_branch) = else_branch { - vec![then_branch, else_branch, condition] - } else { - vec![then_branch, condition] - }; - - ( + track_parent!( + self, self.alloc_expr(Expr::If { condition, then_branch, else_branch }, syntax_ptr), - children_exprs, + then_branch, opt else_branch, condition ) } ast::Expr::EffectExpr(e) => match e.effect() { ast::Effect::Try(_) => { let body = self.collect_block_opt(e.block_expr()); - (self.alloc_expr(Expr::TryBlock { body }, syntax_ptr), vec![body]) + track_parent!(self, self.alloc_expr(Expr::TryBlock { body }, syntax_ptr), body) } ast::Effect::Unsafe(_) => { let body = self.collect_block_opt(e.block_expr()); - (self.alloc_expr(Expr::Unsafe { body }, syntax_ptr), vec![body]) + track_parent!(self, self.alloc_expr(Expr::Unsafe { body }, syntax_ptr), body) } // FIXME: we need to record these effects somewhere... ast::Effect::Async(_) | ast::Effect::Label(_) => { - (self.collect_block_opt(e.block_expr()), vec![]) + self.collect_block_opt(e.block_expr()) } }, - ast::Expr::BlockExpr(e) => (self.collect_block(e), vec![]), + ast::Expr::BlockExpr(e) => self.collect_block(e), ast::Expr::LoopExpr(e) => { let body = self.collect_block_opt(e.loop_body()); - (self.alloc_expr( + track_parent!(self, self.alloc_expr(Expr::Loop { body }, syntax_ptr), vec![body]) Expr::Loop { body, label: e @@ -274,7 +327,7 @@ impl ExprCollector<'_> { .map(|l| Name::new_lifetime(&l)), }, syntax_ptr, - ), vec![body]) + ), body) } ast::Expr::WhileExpr(e) => { let body = self.collect_block_opt(e.loop_body()); @@ -285,7 +338,6 @@ impl ExprCollector<'_> { None => self.collect_expr_opt(condition.expr()), // if let -- desugar to match Some(pat) => { - // FIXME(pfaria) track the break and arms parents here? mark::hit!(infer_resolve_while_let); let pat = self.collect_pat(pat); let match_expr = self.collect_expr_opt(condition.expr()); @@ -298,7 +350,7 @@ impl ExprCollector<'_> { ]; let match_expr = self.alloc_expr_desugared(Expr::Match { expr: match_expr, arms }); - return (self.alloc_expr( + return track_parent!(self, self.alloc_expr( Expr::Loop { body: match_expr, label: e @@ -307,12 +359,12 @@ impl ExprCollector<'_> { .map(|l| Name::new_lifetime(&l)), }, syntax_ptr, - ), vec![match_expr]); + ), match_expr); } }, }; - (self.alloc_expr( + track_parent!(self, self.alloc_expr( Expr::While { condition, body, @@ -322,13 +374,13 @@ impl ExprCollector<'_> { .map(|l| Name::new_lifetime(&l)), }, syntax_ptr, - ), vec![body, condition]) + ), body, condition) } ast::Expr::ForExpr(e) => { let iterable = self.collect_expr_opt(e.iterable()); let pat = self.collect_pat_opt(e.pat()); let body = self.collect_block_opt(e.loop_body()); - (self.alloc_expr( + track_parent!(self, self.alloc_expr( Expr::For { iterable, pat, @@ -339,7 +391,7 @@ impl ExprCollector<'_> { .map(|l| Name::new_lifetime(&l)), }, syntax_ptr, - ), vec![iterable, body]) + ), iterable, body) } ast::Expr::CallExpr(e) => { let callee = self.collect_expr_opt(e.expr()); @@ -348,9 +400,7 @@ impl ExprCollector<'_> { } else { Vec::new() }; - let mut children_exprs = args.clone(); - children_exprs.push(callee); - (self.alloc_expr(Expr::Call { callee, args }, syntax_ptr), children_exprs) + track_parent!(self, self.alloc_expr(Expr::Call { callee, args: args.clone() }, syntax_ptr), callee, vec args) } ast::Expr::MethodCallExpr(e) => { let receiver = self.collect_expr_opt(e.expr()); @@ -362,19 +412,24 @@ impl ExprCollector<'_> { let method_name = e.name_ref().map(|nr| nr.as_name()).unwrap_or_else(Name::missing); let generic_args = e.type_arg_list().and_then(|it| GenericArgs::from_ast(&self.ctx(), it)); - let mut children_exprs = args.clone(); - children_exprs.push(receiver); - ( + track_parent!( + self, self.alloc_expr( - Expr::MethodCall { receiver, method_name, args, generic_args }, + Expr::MethodCall { + receiver, + method_name, + args: args.clone(), + generic_args + }, syntax_ptr, ), - children_exprs, + receiver, + vec args ) } ast::Expr::MatchExpr(e) => { let expr = self.collect_expr_opt(e.expr()); - let (arms, mut children_exprs): (Vec<_>, Vec<_>) = + let (arms, children_exprs): (Vec<_>, Vec<_>) = if let Some(match_arm_list) = e.match_arm_list() { match_arm_list .arms() @@ -396,8 +451,7 @@ impl ExprCollector<'_> { } else { (vec![], vec![]) }; - children_exprs.push(expr); - (self.alloc_expr(Expr::Match { expr, arms }, syntax_ptr), children_exprs) + track_parent!(self, self.alloc_expr(Expr::Match { expr, arms: arms.clone() }, syntax_ptr), expr, vec children_exprs) } ast::Expr::PathExpr(e) => { let path = e @@ -405,7 +459,7 @@ impl ExprCollector<'_> { .and_then(|path| self.expander.parse_path(path)) .map(Expr::Path) .unwrap_or(Expr::Missing); - (self.alloc_expr(path, syntax_ptr), vec![]) + self.alloc_expr(path, syntax_ptr) } ast::Expr::ContinueExpr(e) => (self.alloc_expr( Expr::Continue { label: e.lifetime_token().map(|l| Name::new_lifetime(&l)) }, @@ -413,21 +467,21 @@ impl ExprCollector<'_> { ), vec![]), ast::Expr::BreakExpr(e) => { let expr = e.expr().map(|e| self.collect_expr(e)); - (self.alloc_expr( + track_parent!(self, self.alloc_expr( Expr::Break { expr, label: e.lifetime_token().map(|l| Name::new_lifetime(&l)) }, syntax_ptr, - ), expr.into_iter().collect()) + ), opt expr) } ast::Expr::ParenExpr(e) => { let inner = self.collect_expr_opt(e.expr()); // make the paren expr point to the inner expression as well let src = self.expander.to_source(syntax_ptr); self.source_map.expr_map.insert(src, inner); - (inner, vec![]) + inner } ast::Expr::ReturnExpr(e) => { let expr = e.expr().map(|e| self.collect_expr(e)); - (self.alloc_expr(Expr::Return { expr }, syntax_ptr), expr.into_iter().collect()) + track_parent!(self, self.alloc_expr(Expr::Return { expr }, syntax_ptr), opt expr) } ast::Expr::RecordLit(e) => { let path = e.path().and_then(|path| self.expander.parse_path(path)); @@ -463,7 +517,7 @@ impl ExprCollector<'_> { let src = self.expander.to_source(ptr); self.source_map.field_map.insert((res, i), src); } - (res, children) + track_parent!(self, res, vec children) } ast::Expr::FieldExpr(e) => { let expr = self.collect_expr_opt(e.expr()); @@ -471,20 +525,24 @@ impl ExprCollector<'_> { Some(kind) => kind.as_name(), _ => Name::missing(), }; - (self.alloc_expr(Expr::Field { expr, name }, syntax_ptr), vec![expr]) + track_parent!(self, self.alloc_expr(Expr::Field { expr, name }, syntax_ptr), expr) } ast::Expr::AwaitExpr(e) => { let expr = self.collect_expr_opt(e.expr()); - (self.alloc_expr(Expr::Await { expr }, syntax_ptr), vec![expr]) + track_parent!(self, self.alloc_expr(Expr::Await { expr }, syntax_ptr), expr) } ast::Expr::TryExpr(e) => { let expr = self.collect_expr_opt(e.expr()); - (self.alloc_expr(Expr::Try { expr }, syntax_ptr), vec![expr]) + track_parent!(self, self.alloc_expr(Expr::Try { expr }, syntax_ptr), expr) } ast::Expr::CastExpr(e) => { let expr = self.collect_expr_opt(e.expr()); let type_ref = TypeRef::from_ast_opt(&self.ctx(), e.type_ref()); - (self.alloc_expr(Expr::Cast { expr, type_ref }, syntax_ptr), vec![expr]) + track_parent!( + self, + self.alloc_expr(Expr::Cast { expr, type_ref }, syntax_ptr), + expr + ) } ast::Expr::RefExpr(e) => { let expr = self.collect_expr_opt(e.expr()); @@ -501,15 +559,22 @@ impl ExprCollector<'_> { Mutability::from_mutable(e.mut_token().is_some()) }; let rawness = Rawness::from_raw(raw_tok); - - self.alloc_expr(Expr::Ref { expr, rawness, mutability }, syntax_ptr) + track_parent!( + self, + self.alloc_expr(Expr::Ref { expr, rawness, mutability }, syntax_ptr), + expr + ) } ast::Expr::PrefixExpr(e) => { let expr = self.collect_expr_opt(e.expr()); if let Some(op) = e.op_kind() { - (self.alloc_expr(Expr::UnaryOp { expr, op }, syntax_ptr), vec![expr]) + track_parent!( + self, + self.alloc_expr(Expr::UnaryOp { expr, op }, syntax_ptr), + expr + ) } else { - (self.alloc_expr(Expr::Missing, syntax_ptr), vec![]) + self.alloc_expr(Expr::Missing, syntax_ptr) } } ast::Expr::LambdaExpr(e) => { @@ -529,24 +594,30 @@ impl ExprCollector<'_> { .and_then(|r| r.type_ref()) .map(|it| TypeRef::from_ast(&self.ctx(), it)); let body = self.collect_expr_opt(e.body()); - ( + track_parent!( + self, self.alloc_expr(Expr::Lambda { args, arg_types, ret_type, body }, syntax_ptr), - vec![body], + body, ) } ast::Expr::BinExpr(e) => { let lhs = self.collect_expr_opt(e.lhs()); let rhs = self.collect_expr_opt(e.rhs()); let op = e.op_kind().map(BinaryOp::from); - (self.alloc_expr(Expr::BinaryOp { lhs, rhs, op }, syntax_ptr), vec![lhs, rhs]) + track_parent!( + self, + self.alloc_expr(Expr::BinaryOp { lhs, rhs, op }, syntax_ptr), + lhs, + rhs + ) } ast::Expr::TupleExpr(e) => { let exprs = e.exprs().map(|expr| self.collect_expr(expr)).collect::>(); - (self.alloc_expr(Expr::Tuple { exprs: exprs.clone() }, syntax_ptr), exprs) + track_parent!(self, self.alloc_expr(Expr::Tuple { exprs: exprs.clone() }, syntax_ptr), vec exprs) } ast::Expr::BoxExpr(e) => { let expr = self.collect_expr_opt(e.expr()); - (self.alloc_expr(Expr::Box { expr }, syntax_ptr), vec![expr]) + track_parent!(self, self.alloc_expr(Expr::Box { expr }, syntax_ptr), expr) } ast::Expr::ArrayExpr(e) => { @@ -555,45 +626,51 @@ impl ExprCollector<'_> { match kind { ArrayExprKind::ElementList(e) => { let exprs = e.map(|expr| self.collect_expr(expr)).collect::>(); - ( + track_parent!(self, self.alloc_expr( Expr::Array(Array::ElementList(exprs.clone())), syntax_ptr, ), - exprs, + vec exprs, ) } ArrayExprKind::Repeat { initializer, repeat } => { let initializer = self.collect_expr_opt(initializer); let repeat = self.collect_expr_opt(repeat); - ( + track_parent!( + self, self.alloc_expr( Expr::Array(Array::Repeat { initializer, repeat }), syntax_ptr, ), - vec![initializer, repeat], + initializer, + repeat, ) } } } - ast::Expr::Literal(e) => { - (self.alloc_expr(Expr::Literal(e.kind().into()), syntax_ptr), vec![]) - } + ast::Expr::Literal(e) => self.alloc_expr(Expr::Literal(e.kind().into()), syntax_ptr), ast::Expr::IndexExpr(e) => { let base = self.collect_expr_opt(e.base()); let index = self.collect_expr_opt(e.index()); - (self.alloc_expr(Expr::Index { base, index }, syntax_ptr), vec![base, index]) + track_parent!( + self, + self.alloc_expr(Expr::Index { base, index }, syntax_ptr), + base, + index + ) } ast::Expr::RangeExpr(e) => { let lhs = e.start().map(|lhs| self.collect_expr(lhs)); let rhs = e.end().map(|rhs| self.collect_expr(rhs)); match e.op_kind() { - Some(range_type) => ( + Some(range_type) => track_parent!( + self, self.alloc_expr(Expr::Range { lhs, rhs, range_type }, syntax_ptr), - lhs.into_iter().chain(rhs.into_iter()).collect(), + opt lhs, opt rhs ), - None => (self.alloc_expr(Expr::Missing, syntax_ptr), vec![]), + None => self.alloc_expr(Expr::Missing, syntax_ptr), } } ast::Expr::MacroCall(e) => { @@ -607,7 +684,7 @@ impl ExprCollector<'_> { self.body.item_scope.define_legacy_macro(name, mac); // FIXME: do we still need to allocate this as missing ? - (self.alloc_expr(Expr::Missing, syntax_ptr), vec![]) + self.alloc_expr(Expr::Missing, syntax_ptr) } else { let macro_call = self.expander.to_source(AstPtr::new(&e)); match self.expander.enter_expand(self.db, Some(&self.body.item_scope), e) { @@ -620,15 +697,15 @@ impl ExprCollector<'_> { self.item_trees.insert(self.expander.current_file_id, item_tree); let id = self.collect_expr(expansion); self.expander.exit(self.db, mark); - (id, vec![]) + id } - None => (self.alloc_expr(Expr::Missing, syntax_ptr), vec![]), + None => self.alloc_expr(Expr::Missing, syntax_ptr), } } } // FIXME implement HIR for these: - ast::Expr::Label(_e) => (self.alloc_expr(Expr::Missing, syntax_ptr), vec![]), + ast::Expr::Label(_e) => self.alloc_expr(Expr::Missing, syntax_ptr), } } -- cgit v1.2.3 From f78df42f813504a376dca555b04ac427582f542c Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Tue, 2 Jun 2020 18:04:23 -0400 Subject: Fix issues caused during rebase --- crates/ra_hir_def/src/body/lower.rs | 108 +++++++++++++++++++++--------------- 1 file changed, 63 insertions(+), 45 deletions(-) (limited to 'crates/ra_hir_def/src') diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index 114b38710..4240c6ad8 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -318,16 +318,20 @@ impl ExprCollector<'_> { ast::Expr::BlockExpr(e) => self.collect_block(e), ast::Expr::LoopExpr(e) => { let body = self.collect_block_opt(e.loop_body()); - track_parent!(self, self.alloc_expr(Expr::Loop { body }, syntax_ptr), vec![body]) - Expr::Loop { - body, - label: e - .label() - .and_then(|l| l.lifetime_token()) - .map(|l| Name::new_lifetime(&l)), - }, - syntax_ptr, - ), body) + track_parent!( + self, + self.alloc_expr( + Expr::Loop { + body, + label: e + .label() + .and_then(|l| l.lifetime_token()) + .map(|l| Name::new_lifetime(&l)), + }, + syntax_ptr, + ), + body + ) } ast::Expr::WhileExpr(e) => { let body = self.collect_block_opt(e.loop_body()); @@ -350,48 +354,62 @@ impl ExprCollector<'_> { ]; let match_expr = self.alloc_expr_desugared(Expr::Match { expr: match_expr, arms }); - return track_parent!(self, self.alloc_expr( - Expr::Loop { - body: match_expr, - label: e - .label() - .and_then(|l| l.lifetime_token()) - .map(|l| Name::new_lifetime(&l)), - }, - syntax_ptr, - ), match_expr); + return track_parent!( + self, + self.alloc_expr( + Expr::Loop { + body: match_expr, + label: e + .label() + .and_then(|l| l.lifetime_token()) + .map(|l| Name::new_lifetime(&l)), + }, + syntax_ptr, + ), + match_expr + ); } }, }; - track_parent!(self, self.alloc_expr( - Expr::While { - condition, - body, - label: e - .label() - .and_then(|l| l.lifetime_token()) - .map(|l| Name::new_lifetime(&l)), - }, - syntax_ptr, - ), body, condition) + track_parent!( + self, + self.alloc_expr( + Expr::While { + condition, + body, + label: e + .label() + .and_then(|l| l.lifetime_token()) + .map(|l| Name::new_lifetime(&l)), + }, + syntax_ptr, + ), + body, + condition + ) } ast::Expr::ForExpr(e) => { let iterable = self.collect_expr_opt(e.iterable()); let pat = self.collect_pat_opt(e.pat()); let body = self.collect_block_opt(e.loop_body()); - track_parent!(self, self.alloc_expr( - Expr::For { - iterable, - pat, - body, - label: e - .label() - .and_then(|l| l.lifetime_token()) - .map(|l| Name::new_lifetime(&l)), - }, - syntax_ptr, - ), iterable, body) + track_parent!( + self, + self.alloc_expr( + Expr::For { + iterable, + pat, + body, + label: e + .label() + .and_then(|l| l.lifetime_token()) + .map(|l| Name::new_lifetime(&l)), + }, + syntax_ptr, + ), + iterable, + body + ) } ast::Expr::CallExpr(e) => { let callee = self.collect_expr_opt(e.expr()); @@ -461,10 +479,10 @@ impl ExprCollector<'_> { .unwrap_or(Expr::Missing); self.alloc_expr(path, syntax_ptr) } - ast::Expr::ContinueExpr(e) => (self.alloc_expr( + ast::Expr::ContinueExpr(e) => self.alloc_expr( Expr::Continue { label: e.lifetime_token().map(|l| Name::new_lifetime(&l)) }, syntax_ptr, - ), vec![]), + ), ast::Expr::BreakExpr(e) => { let expr = e.expr().map(|e| self.collect_expr(e)); track_parent!(self, self.alloc_expr( -- cgit v1.2.3 From 2fc92fa28cda858c25fea2e378c2eb73e7f65247 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Tue, 2 Jun 2020 18:44:04 -0400 Subject: Remove track_parent and parent_map, replace with simple walk in missign unsafe validator --- crates/ra_hir_def/src/body.rs | 1 - crates/ra_hir_def/src/body/lower.rs | 369 ++++++++++-------------------------- 2 files changed, 98 insertions(+), 272 deletions(-) (limited to 'crates/ra_hir_def/src') diff --git a/crates/ra_hir_def/src/body.rs b/crates/ra_hir_def/src/body.rs index 14a1f4773..4f2350915 100644 --- a/crates/ra_hir_def/src/body.rs +++ b/crates/ra_hir_def/src/body.rs @@ -184,7 +184,6 @@ pub struct Body { /// The `ExprId` of the actual body expression. pub body_expr: ExprId, pub item_scope: ItemScope, - pub parent_map: ArenaMap, } pub type ExprPtr = AstPtr; diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index 4240c6ad8..fdd2be843 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -7,7 +7,7 @@ use hir_expand::{ name::{name, AsName, Name}, HirFileId, MacroDefId, MacroDefKind, }; -use ra_arena::{map::ArenaMap, Arena}; +use ra_arena::Arena; use ra_syntax::{ ast::{ self, ArgListOwner, ArrayExprKind, LiteralKind, LoopBodyOwner, ModuleItemOwner, NameOwner, @@ -74,7 +74,6 @@ pub(super) fn lower( params: Vec::new(), body_expr: dummy_expr_id(), item_scope: Default::default(), - parent_map: ArenaMap::default(), }, item_trees: { let mut map = FxHashMap::default(); @@ -96,40 +95,6 @@ struct ExprCollector<'a> { item_trees: FxHashMap>, } -macro_rules! track_parent { - (@build $collector:ident, $parent:expr $(,)?) => { - $parent - }; - (@build $collector:ident, $parent:expr, opt $expr:ident $($rest:tt)*) => { - { - if let Some(expr) = $expr { - $collector.body.parent_map.insert(expr, $parent); - } - track_parent!(@build $collector, $parent $($rest)*) - } - }; - (@build $collector:ident, $parent:expr, vec $expr:ident $($rest:tt)*) => { - { - for expr in $expr { - $collector.body.parent_map.insert(expr, $parent); - } - track_parent!(@build $collector, $parent $($rest)*) - } - }; - (@build $collector:ident, $parent:expr, $expr:ident $($rest:tt)*) => { - { - $collector.body.parent_map.insert($expr, $parent); - track_parent!(@build $collector, $parent $($rest)*) - } - }; - ($collector:ident, $parent:expr, $($rest:tt)*) => { - { - let parent = $parent; - track_parent!(@build $collector, parent, $($rest)*) - } - } -} - impl ExprCollector<'_> { fn collect( mut self, @@ -206,51 +171,7 @@ impl ExprCollector<'_> { id } - fn update_parent_map( - &mut self, - (parent_expr, children_exprs): (ExprId, Vec), - ) -> ExprId { - for child_expr in children_exprs { - self.body.parent_map.insert(child_expr, parent_expr); - } - - parent_expr - } - fn collect_expr(&mut self, expr: ast::Expr) -> ExprId { - let expr_id = self.collect_expr_inner(expr); - - debug_assert!({ - let mut found_count = 0; - let mut incr = || { - found_count += 1; - true - }; - let mut all_children_found = true; - self.body[expr_id].walk_child_exprs(|child| { - all_children_found = all_children_found - && self - .body - .parent_map - .get(child) - .map(|parent| *parent == expr_id) - .unwrap_or(false) - && incr() - }); - - if all_children_found { - let child_count_in_map = - self.body.parent_map.iter().filter(|&(_, parent)| *parent == expr_id).count(); - found_count == child_count_in_map - } else { - false - } - }); - - expr_id - } - - fn collect_expr_inner(&mut self, expr: ast::Expr) -> ExprId { let syntax_ptr = AstPtr::new(&expr); if !self.expander.is_cfg_enabled(&expr) { return self.missing_expr(); @@ -285,30 +206,22 @@ impl ExprCollector<'_> { guard: None, }, ]; - let arm_exprs = arms.iter().map(|arm| arm.expr).collect::>(); - return track_parent!( - self, - self.alloc_expr(Expr::Match { expr: match_expr, arms }, syntax_ptr), - match_expr, vec arm_exprs - ); + return self + .alloc_expr(Expr::Match { expr: match_expr, arms }, syntax_ptr); } }, }; - track_parent!( - self, - self.alloc_expr(Expr::If { condition, then_branch, else_branch }, syntax_ptr), - then_branch, opt else_branch, condition - ) + self.alloc_expr(Expr::If { condition, then_branch, else_branch }, syntax_ptr) } ast::Expr::EffectExpr(e) => match e.effect() { ast::Effect::Try(_) => { let body = self.collect_block_opt(e.block_expr()); - track_parent!(self, self.alloc_expr(Expr::TryBlock { body }, syntax_ptr), body) + self.alloc_expr(Expr::TryBlock { body }, syntax_ptr) } ast::Effect::Unsafe(_) => { let body = self.collect_block_opt(e.block_expr()); - track_parent!(self, self.alloc_expr(Expr::Unsafe { body }, syntax_ptr), body) + self.alloc_expr(Expr::Unsafe { body }, syntax_ptr) } // FIXME: we need to record these effects somewhere... ast::Effect::Async(_) | ast::Effect::Label(_) => { @@ -318,19 +231,15 @@ impl ExprCollector<'_> { ast::Expr::BlockExpr(e) => self.collect_block(e), ast::Expr::LoopExpr(e) => { let body = self.collect_block_opt(e.loop_body()); - track_parent!( - self, - self.alloc_expr( - Expr::Loop { - body, - label: e - .label() - .and_then(|l| l.lifetime_token()) - .map(|l| Name::new_lifetime(&l)), - }, - syntax_ptr, - ), - body + self.alloc_expr( + Expr::Loop { + body, + label: e + .label() + .and_then(|l| l.lifetime_token()) + .map(|l| Name::new_lifetime(&l)), + }, + syntax_ptr, ) } ast::Expr::WhileExpr(e) => { @@ -354,61 +263,47 @@ impl ExprCollector<'_> { ]; let match_expr = self.alloc_expr_desugared(Expr::Match { expr: match_expr, arms }); - return track_parent!( - self, - self.alloc_expr( - Expr::Loop { - body: match_expr, - label: e - .label() - .and_then(|l| l.lifetime_token()) - .map(|l| Name::new_lifetime(&l)), - }, - syntax_ptr, - ), - match_expr + return self.alloc_expr( + Expr::Loop { + body: match_expr, + label: e + .label() + .and_then(|l| l.lifetime_token()) + .map(|l| Name::new_lifetime(&l)), + }, + syntax_ptr, ); } }, }; - track_parent!( - self, - self.alloc_expr( - Expr::While { - condition, - body, - label: e - .label() - .and_then(|l| l.lifetime_token()) - .map(|l| Name::new_lifetime(&l)), - }, - syntax_ptr, - ), - body, - condition + self.alloc_expr( + Expr::While { + condition, + body, + label: e + .label() + .and_then(|l| l.lifetime_token()) + .map(|l| Name::new_lifetime(&l)), + }, + syntax_ptr, ) } ast::Expr::ForExpr(e) => { let iterable = self.collect_expr_opt(e.iterable()); let pat = self.collect_pat_opt(e.pat()); let body = self.collect_block_opt(e.loop_body()); - track_parent!( - self, - self.alloc_expr( - Expr::For { - iterable, - pat, - body, - label: e - .label() - .and_then(|l| l.lifetime_token()) - .map(|l| Name::new_lifetime(&l)), - }, - syntax_ptr, - ), - iterable, - body + self.alloc_expr( + Expr::For { + iterable, + pat, + body, + label: e + .label() + .and_then(|l| l.lifetime_token()) + .map(|l| Name::new_lifetime(&l)), + }, + syntax_ptr, ) } ast::Expr::CallExpr(e) => { @@ -418,7 +313,7 @@ impl ExprCollector<'_> { } else { Vec::new() }; - track_parent!(self, self.alloc_expr(Expr::Call { callee, args: args.clone() }, syntax_ptr), callee, vec args) + self.alloc_expr(Expr::Call { callee, args: args.clone() }, syntax_ptr) } ast::Expr::MethodCallExpr(e) => { let receiver = self.collect_expr_opt(e.expr()); @@ -430,46 +325,29 @@ impl ExprCollector<'_> { let method_name = e.name_ref().map(|nr| nr.as_name()).unwrap_or_else(Name::missing); let generic_args = e.type_arg_list().and_then(|it| GenericArgs::from_ast(&self.ctx(), it)); - track_parent!( - self, - self.alloc_expr( - Expr::MethodCall { - receiver, - method_name, - args: args.clone(), - generic_args - }, - syntax_ptr, - ), - receiver, - vec args + self.alloc_expr( + Expr::MethodCall { receiver, method_name, args: args.clone(), generic_args }, + syntax_ptr, ) } ast::Expr::MatchExpr(e) => { let expr = self.collect_expr_opt(e.expr()); - let (arms, children_exprs): (Vec<_>, Vec<_>) = - if let Some(match_arm_list) = e.match_arm_list() { - match_arm_list - .arms() - .map(|arm| { - let expr = self.collect_expr_opt(arm.expr()); - ( - MatchArm { - pat: self.collect_pat_opt(arm.pat()), - expr, - guard: arm - .guard() - .and_then(|guard| guard.expr()) - .map(|e| self.collect_expr(e)), - }, - expr, - ) - }) - .unzip() - } else { - (vec![], vec![]) - }; - track_parent!(self, self.alloc_expr(Expr::Match { expr, arms: arms.clone() }, syntax_ptr), expr, vec children_exprs) + let arms = if let Some(match_arm_list) = e.match_arm_list() { + match_arm_list + .arms() + .map(|arm| MatchArm { + pat: self.collect_pat_opt(arm.pat()), + expr: self.collect_expr_opt(arm.expr()), + guard: arm + .guard() + .and_then(|guard| guard.expr()) + .map(|e| self.collect_expr(e)), + }) + .collect() + } else { + vec![] + }; + self.alloc_expr(Expr::Match { expr, arms }, syntax_ptr) } ast::Expr::PathExpr(e) => { let path = e @@ -485,10 +363,10 @@ impl ExprCollector<'_> { ), ast::Expr::BreakExpr(e) => { let expr = e.expr().map(|e| self.collect_expr(e)); - track_parent!(self, self.alloc_expr( + self.alloc_expr( Expr::Break { expr, label: e.lifetime_token().map(|l| Name::new_lifetime(&l)) }, syntax_ptr, - ), opt expr) + ) } ast::Expr::ParenExpr(e) => { let inner = self.collect_expr_opt(e.expr()); @@ -499,13 +377,13 @@ impl ExprCollector<'_> { } ast::Expr::ReturnExpr(e) => { let expr = e.expr().map(|e| self.collect_expr(e)); - track_parent!(self, self.alloc_expr(Expr::Return { expr }, syntax_ptr), opt expr) + self.alloc_expr(Expr::Return { expr }, syntax_ptr) } ast::Expr::RecordLit(e) => { let path = e.path().and_then(|path| self.expander.parse_path(path)); let mut field_ptrs = Vec::new(); - let (record_lit, children) = if let Some(nfl) = e.record_field_list() { - let (fields, children): (Vec<_>, Vec<_>) = nfl + let record_lit = if let Some(nfl) = e.record_field_list() { + let fields = nfl .fields() .inspect(|field| field_ptrs.push(AstPtr::new(field))) .filter_map(|field| { @@ -518,16 +396,13 @@ impl ExprCollector<'_> { Some(e) => self.collect_expr(e), None => self.missing_expr(), }; - Some((RecordLitField { name, expr }, expr)) + Some(RecordLitField { name, expr }) }) - .unzip(); + .collect(); let spread = nfl.spread().map(|s| self.collect_expr(s)); - ( - Expr::RecordLit { path, fields, spread: spread }, - children.into_iter().chain(spread.into_iter()).collect(), - ) + Expr::RecordLit { path, fields, spread: spread } } else { - (Expr::RecordLit { path, fields: Vec::new(), spread: None }, vec![]) + Expr::RecordLit { path, fields: Vec::new(), spread: None } }; let res = self.alloc_expr(record_lit, syntax_ptr); @@ -535,7 +410,7 @@ impl ExprCollector<'_> { let src = self.expander.to_source(ptr); self.source_map.field_map.insert((res, i), src); } - track_parent!(self, res, vec children) + res } ast::Expr::FieldExpr(e) => { let expr = self.collect_expr_opt(e.expr()); @@ -543,24 +418,20 @@ impl ExprCollector<'_> { Some(kind) => kind.as_name(), _ => Name::missing(), }; - track_parent!(self, self.alloc_expr(Expr::Field { expr, name }, syntax_ptr), expr) + self.alloc_expr(Expr::Field { expr, name }, syntax_ptr) } ast::Expr::AwaitExpr(e) => { let expr = self.collect_expr_opt(e.expr()); - track_parent!(self, self.alloc_expr(Expr::Await { expr }, syntax_ptr), expr) + self.alloc_expr(Expr::Await { expr }, syntax_ptr) } ast::Expr::TryExpr(e) => { let expr = self.collect_expr_opt(e.expr()); - track_parent!(self, self.alloc_expr(Expr::Try { expr }, syntax_ptr), expr) + self.alloc_expr(Expr::Try { expr }, syntax_ptr) } ast::Expr::CastExpr(e) => { let expr = self.collect_expr_opt(e.expr()); let type_ref = TypeRef::from_ast_opt(&self.ctx(), e.type_ref()); - track_parent!( - self, - self.alloc_expr(Expr::Cast { expr, type_ref }, syntax_ptr), - expr - ) + self.alloc_expr(Expr::Cast { expr, type_ref }, syntax_ptr) } ast::Expr::RefExpr(e) => { let expr = self.collect_expr_opt(e.expr()); @@ -577,20 +448,12 @@ impl ExprCollector<'_> { Mutability::from_mutable(e.mut_token().is_some()) }; let rawness = Rawness::from_raw(raw_tok); - track_parent!( - self, - self.alloc_expr(Expr::Ref { expr, rawness, mutability }, syntax_ptr), - expr - ) + self.alloc_expr(Expr::Ref { expr, rawness, mutability }, syntax_ptr) } ast::Expr::PrefixExpr(e) => { let expr = self.collect_expr_opt(e.expr()); if let Some(op) = e.op_kind() { - track_parent!( - self, - self.alloc_expr(Expr::UnaryOp { expr, op }, syntax_ptr), - expr - ) + self.alloc_expr(Expr::UnaryOp { expr, op }, syntax_ptr) } else { self.alloc_expr(Expr::Missing, syntax_ptr) } @@ -612,30 +475,21 @@ impl ExprCollector<'_> { .and_then(|r| r.type_ref()) .map(|it| TypeRef::from_ast(&self.ctx(), it)); let body = self.collect_expr_opt(e.body()); - track_parent!( - self, - self.alloc_expr(Expr::Lambda { args, arg_types, ret_type, body }, syntax_ptr), - body, - ) + self.alloc_expr(Expr::Lambda { args, arg_types, ret_type, body }, syntax_ptr) } ast::Expr::BinExpr(e) => { let lhs = self.collect_expr_opt(e.lhs()); let rhs = self.collect_expr_opt(e.rhs()); let op = e.op_kind().map(BinaryOp::from); - track_parent!( - self, - self.alloc_expr(Expr::BinaryOp { lhs, rhs, op }, syntax_ptr), - lhs, - rhs - ) + self.alloc_expr(Expr::BinaryOp { lhs, rhs, op }, syntax_ptr) } ast::Expr::TupleExpr(e) => { let exprs = e.exprs().map(|expr| self.collect_expr(expr)).collect::>(); - track_parent!(self, self.alloc_expr(Expr::Tuple { exprs: exprs.clone() }, syntax_ptr), vec exprs) + self.alloc_expr(Expr::Tuple { exprs: exprs.clone() }, syntax_ptr) } ast::Expr::BoxExpr(e) => { let expr = self.collect_expr_opt(e.expr()); - track_parent!(self, self.alloc_expr(Expr::Box { expr }, syntax_ptr), expr) + self.alloc_expr(Expr::Box { expr }, syntax_ptr) } ast::Expr::ArrayExpr(e) => { @@ -644,25 +498,14 @@ impl ExprCollector<'_> { match kind { ArrayExprKind::ElementList(e) => { let exprs = e.map(|expr| self.collect_expr(expr)).collect::>(); - track_parent!(self, - self.alloc_expr( - Expr::Array(Array::ElementList(exprs.clone())), - syntax_ptr, - ), - vec exprs, - ) + self.alloc_expr(Expr::Array(Array::ElementList(exprs.clone())), syntax_ptr) } ArrayExprKind::Repeat { initializer, repeat } => { let initializer = self.collect_expr_opt(initializer); let repeat = self.collect_expr_opt(repeat); - track_parent!( - self, - self.alloc_expr( - Expr::Array(Array::Repeat { initializer, repeat }), - syntax_ptr, - ), - initializer, - repeat, + self.alloc_expr( + Expr::Array(Array::Repeat { initializer, repeat }), + syntax_ptr, ) } } @@ -672,22 +515,15 @@ impl ExprCollector<'_> { ast::Expr::IndexExpr(e) => { let base = self.collect_expr_opt(e.base()); let index = self.collect_expr_opt(e.index()); - track_parent!( - self, - self.alloc_expr(Expr::Index { base, index }, syntax_ptr), - base, - index - ) + self.alloc_expr(Expr::Index { base, index }, syntax_ptr) } ast::Expr::RangeExpr(e) => { let lhs = e.start().map(|lhs| self.collect_expr(lhs)); let rhs = e.end().map(|rhs| self.collect_expr(rhs)); match e.op_kind() { - Some(range_type) => track_parent!( - self, - self.alloc_expr(Expr::Range { lhs, rhs, range_type }, syntax_ptr), - opt lhs, opt rhs - ), + Some(range_type) => { + self.alloc_expr(Expr::Range { lhs, rhs, range_type }, syntax_ptr) + } None => self.alloc_expr(Expr::Missing, syntax_ptr), } } @@ -762,14 +598,9 @@ impl ExprCollector<'_> { } fn collect_block(&mut self, block: ast::BlockExpr) -> ExprId { - let parent_and_children = self.collect_block_inner(block); - self.update_parent_map(parent_and_children) - } - - fn collect_block_inner(&mut self, block: ast::BlockExpr) -> (ExprId, Vec) { let syntax_node_ptr = AstPtr::new(&block.clone().into()); self.collect_block_items(&block); - let (statements, children_exprs): (Vec<_>, Vec<_>) = block + let statements = block .statements() .map(|s| match s { ast::Stmt::LetStmt(stmt) => { @@ -777,18 +608,14 @@ impl ExprCollector<'_> { let type_ref = stmt.ascribed_type().map(|it| TypeRef::from_ast(&self.ctx(), it)); let initializer = stmt.initializer().map(|e| self.collect_expr(e)); - (Statement::Let { pat, type_ref, initializer }, initializer) - } - ast::Stmt::ExprStmt(stmt) => { - let expr = self.collect_expr_opt(stmt.expr()); - (Statement::Expr(expr), Some(expr)) + Statement::Let { pat, type_ref, initializer } } + ast::Stmt::ExprStmt(stmt) => Statement::Expr(self.collect_expr_opt(stmt.expr())), }) - .unzip(); + .collect(); let tail = block.expr().map(|e| self.collect_expr(e)); let label = block.label().and_then(|l| l.lifetime_token()).map(|t| Name::new_lifetime(&t)); - let children_exprs = children_exprs.into_iter().flatten().chain(tail.into_iter()).collect(); - (self.alloc_expr(Expr::Block { statements, tail, label }, syntax_node_ptr), children_exprs) + self.alloc_expr(Expr::Block { statements, tail, label }, syntax_node_ptr) } fn collect_block_items(&mut self, block: &ast::BlockExpr) { -- cgit v1.2.3 From 28bb8ed9cb0aa9f1efad252748ea189716355157 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Tue, 2 Jun 2020 19:09:51 -0400 Subject: Cleanup changes leftover from previous tracking attempt --- crates/ra_hir_def/src/body/lower.rs | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) (limited to 'crates/ra_hir_def/src') diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index fdd2be843..c6bc85e2f 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -313,20 +313,20 @@ impl ExprCollector<'_> { } else { Vec::new() }; - self.alloc_expr(Expr::Call { callee, args: args.clone() }, syntax_ptr) + self.alloc_expr(Expr::Call { callee, args }, syntax_ptr) } ast::Expr::MethodCallExpr(e) => { let receiver = self.collect_expr_opt(e.expr()); let args = if let Some(arg_list) = e.arg_list() { arg_list.args().map(|e| self.collect_expr(e)).collect() } else { - vec![] + Vec::new() }; let method_name = e.name_ref().map(|nr| nr.as_name()).unwrap_or_else(Name::missing); let generic_args = e.type_arg_list().and_then(|it| GenericArgs::from_ast(&self.ctx(), it)); self.alloc_expr( - Expr::MethodCall { receiver, method_name, args: args.clone(), generic_args }, + Expr::MethodCall { receiver, method_name, args, generic_args }, syntax_ptr, ) } @@ -345,7 +345,7 @@ impl ExprCollector<'_> { }) .collect() } else { - vec![] + Vec::new() }; self.alloc_expr(Expr::Match { expr, arms }, syntax_ptr) } @@ -392,15 +392,17 @@ impl ExprCollector<'_> { } let name = field.field_name()?.as_name(); - let expr = match field.expr() { - Some(e) => self.collect_expr(e), - None => self.missing_expr(), - }; - Some(RecordLitField { name, expr }) + Some(RecordLitField { + name, + expr: match field.expr() { + Some(e) => self.collect_expr(e), + None => self.missing_expr(), + }, + }) }) .collect(); let spread = nfl.spread().map(|s| self.collect_expr(s)); - Expr::RecordLit { path, fields, spread: spread } + Expr::RecordLit { path, fields, spread } } else { Expr::RecordLit { path, fields: Vec::new(), spread: None } }; @@ -484,8 +486,8 @@ impl ExprCollector<'_> { self.alloc_expr(Expr::BinaryOp { lhs, rhs, op }, syntax_ptr) } ast::Expr::TupleExpr(e) => { - let exprs = e.exprs().map(|expr| self.collect_expr(expr)).collect::>(); - self.alloc_expr(Expr::Tuple { exprs: exprs.clone() }, syntax_ptr) + let exprs = e.exprs().map(|expr| self.collect_expr(expr)).collect(); + self.alloc_expr(Expr::Tuple { exprs }, syntax_ptr) } ast::Expr::BoxExpr(e) => { let expr = self.collect_expr_opt(e.expr()); @@ -497,8 +499,8 @@ impl ExprCollector<'_> { match kind { ArrayExprKind::ElementList(e) => { - let exprs = e.map(|expr| self.collect_expr(expr)).collect::>(); - self.alloc_expr(Expr::Array(Array::ElementList(exprs.clone())), syntax_ptr) + let exprs = e.map(|expr| self.collect_expr(expr)).collect(); + self.alloc_expr(Expr::Array(Array::ElementList(exprs)), syntax_ptr) } ArrayExprKind::Repeat { initializer, repeat } => { let initializer = self.collect_expr_opt(initializer); -- cgit v1.2.3