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 ++- crates/ra_hir_ty/src/expr.rs | 64 ++++++++++++++++++++++------- crates/ra_hir_ty/src/infer/expr.rs | 1 + crates/ra_hir_ty/src/tests.rs | 22 +++++++++- crates/ra_hir_ty/src/tests/simple.rs | 1 + crates/ra_syntax/src/ast/generated/nodes.rs | 2 +- 7 files changed, 82 insertions(+), 19 deletions(-) (limited to 'crates') 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); diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs index 1a0710e0b..d600aca21 100644 --- a/crates/ra_hir_ty/src/expr.rs +++ b/crates/ra_hir_ty/src/expr.rs @@ -318,46 +318,74 @@ pub fn record_pattern_missing_fields( Some((variant_def, missed_fields, exhaustive)) } +pub struct UnsafeExpr { + expr: ExprId, + inside_unsafe_block: bool, +} + +impl UnsafeExpr { + fn new(expr: ExprId) -> Self { + Self { expr, inside_unsafe_block: false } + } +} + pub fn unsafe_expressions( db: &dyn HirDatabase, infer: &InferenceResult, def: DefWithBodyId, -) -> Vec { - let mut unsafe_expr_ids = vec![]; +) -> Vec { + let mut unsafe_exprs = vec![]; + let mut unsafe_block_scopes = vec![]; let body = db.body(def); + let expr_scopes = db.expr_scopes(def); for (id, expr) in body.exprs.iter() { match expr { + Expr::UnsafeBlock { body } => { + if let Some(scope) = expr_scopes.scope_for(*body) { + unsafe_block_scopes.push(scope); + } + } Expr::Call { callee, .. } => { let ty = &infer.type_of_expr[*callee]; - if let &Ty::Apply(ApplicationTy {ctor: TypeCtor::FnDef(CallableDef::FunctionId(func)), .. }) = ty { + if let &Ty::Apply(ApplicationTy { + ctor: TypeCtor::FnDef(CallableDef::FunctionId(func)), + .. + }) = ty + { if db.function_data(func).is_unsafe { - unsafe_expr_ids.push(id); + unsafe_exprs.push(UnsafeExpr::new(id)); } - } + } } Expr::MethodCall { .. } => { if infer .method_resolution(id) - .map(|func| { - db.function_data(func).is_unsafe - }) - .unwrap_or_else(|| { - false - }) + .map(|func| db.function_data(func).is_unsafe) + .unwrap_or_else(|| false) { - unsafe_expr_ids.push(id); + unsafe_exprs.push(UnsafeExpr::new(id)); } } Expr::UnaryOp { expr, op: UnaryOp::Deref } => { if let Ty::Apply(ApplicationTy { ctor: TypeCtor::RawPtr(..), .. }) = &infer[*expr] { - unsafe_expr_ids.push(id); + unsafe_exprs.push(UnsafeExpr::new(id)); } } _ => {} } } - unsafe_expr_ids + 'unsafe_exprs: for unsafe_expr in &mut unsafe_exprs { + let scope = expr_scopes.scope_for(unsafe_expr.expr); + for scope in expr_scopes.scope_chain(scope) { + if unsafe_block_scopes.contains(&scope) { + unsafe_expr.inside_unsafe_block = true; + continue 'unsafe_exprs; + } + } + } + + unsafe_exprs } pub struct UnsafeValidator<'a, 'b: 'a> { @@ -379,7 +407,13 @@ impl<'a, 'b> UnsafeValidator<'a, 'b> { let def = self.func.into(); let unsafe_expressions = unsafe_expressions(db, self.infer.as_ref(), def); let func_data = db.function_data(self.func); - if func_data.is_unsafe || unsafe_expressions.len() == 0 { + if func_data.is_unsafe + || unsafe_expressions + .into_iter() + .filter(|unsafe_expr| !unsafe_expr.inside_unsafe_block) + .count() + == 0 + { return; } diff --git a/crates/ra_hir_ty/src/infer/expr.rs b/crates/ra_hir_ty/src/infer/expr.rs index a9565a58d..df7bcb5fa 100644 --- a/crates/ra_hir_ty/src/infer/expr.rs +++ b/crates/ra_hir_ty/src/infer/expr.rs @@ -142,6 +142,7 @@ impl<'a> InferenceContext<'a> { // FIXME: Breakable block inference self.infer_block(statements, *tail, expected) } + Expr::UnsafeBlock { body } => self.infer_expr(*body, expected), Expr::TryBlock { body } => { let _inner = self.infer_expr(*body, expected); // FIXME should be std::result::Result<{inner}, _> diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs index 8889fa3ba..4bc2e8b27 100644 --- a/crates/ra_hir_ty/src/tests.rs +++ b/crates/ra_hir_ty/src/tests.rs @@ -608,10 +608,30 @@ fn no_missing_unsafe_diagnostic_with_raw_ptr_in_unsafe_block() { r" //- /lib.rs fn nothing_to_see_move_along() { + let x = &5 as *const usize; + unsafe { + let y = *x; + } +} +", + ) + .diagnostics() + .0; + + assert_snapshot!(diagnostics, @""); +} + +#[test] +fn missing_unsafe_diagnostic_with_raw_ptr_outside_unsafe_block() { + let diagnostics = TestDB::with_files( + r" +//- /lib.rs +fn nothing_to_see_move_along() { + let x = &5 as *const usize; unsafe { - let x = &5 as *const usize; let y = *x; } + let z = *x; } ", ) diff --git a/crates/ra_hir_ty/src/tests/simple.rs b/crates/ra_hir_ty/src/tests/simple.rs index 7d8197f8b..cd919466f 100644 --- a/crates/ra_hir_ty/src/tests/simple.rs +++ b/crates/ra_hir_ty/src/tests/simple.rs @@ -1880,6 +1880,7 @@ fn main() { @r###" 10..130 '{ ...2 }; }': () 20..21 'x': i32 + 24..37 'unsafe { 92 }': i32 31..37 '{ 92 }': i32 33..35 '92': i32 47..48 'y': {unknown} diff --git a/crates/ra_syntax/src/ast/generated/nodes.rs b/crates/ra_syntax/src/ast/generated/nodes.rs index a2366f997..58141da11 100644 --- a/crates/ra_syntax/src/ast/generated/nodes.rs +++ b/crates/ra_syntax/src/ast/generated/nodes.rs @@ -899,7 +899,7 @@ impl ast::LoopBodyOwner for LoopExpr {} impl LoopExpr { pub fn loop_token(&self) -> Option { support::token(&self.syntax, T![loop]) } } -/// Block expression with an optional prefix (label, try keyword, +/// Block expression with an optional prefix (label, try ketword, /// unsafe keyword, async keyword...). /// /// ``` -- cgit v1.2.3