From f678e0d837e472dc2f1421f89f794d33f3ade55c Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Thu, 28 May 2020 09:30:19 -0400 Subject: Add HighlightTag::Operator, use it for unsafe deref. Move unsafe validation to its own file --- crates/ra_hir_ty/src/unsafe_validation.rs | 63 +++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 crates/ra_hir_ty/src/unsafe_validation.rs (limited to 'crates/ra_hir_ty/src/unsafe_validation.rs') diff --git a/crates/ra_hir_ty/src/unsafe_validation.rs b/crates/ra_hir_ty/src/unsafe_validation.rs new file mode 100644 index 000000000..55dbe23fa --- /dev/null +++ b/crates/ra_hir_ty/src/unsafe_validation.rs @@ -0,0 +1,63 @@ +//! Provides validations for unsafe code. Currently checks if unsafe functions are missing +//! unsafe blocks. + +use std::sync::Arc; + +use hir_def::FunctionId; +use hir_expand::diagnostics::DiagnosticSink; + +use crate::{ + db::HirDatabase, diagnostics::MissingUnsafe, expr::unsafe_expressions, InferenceResult, +}; + +pub use hir_def::{ + body::{ + scope::{ExprScopes, ScopeEntry, ScopeId}, + Body, BodySourceMap, ExprPtr, ExprSource, PatPtr, PatSource, + }, + expr::{ + ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Literal, LogicOp, + MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement, UnaryOp, + }, + LocalFieldId, VariantId, +}; + +pub struct UnsafeValidator<'a, 'b: 'a> { + func: FunctionId, + infer: Arc, + sink: &'a mut DiagnosticSink<'b>, +} + +impl<'a, 'b> UnsafeValidator<'a, 'b> { + pub fn new( + func: FunctionId, + infer: Arc, + sink: &'a mut DiagnosticSink<'b>, + ) -> UnsafeValidator<'a, 'b> { + UnsafeValidator { func, infer, sink } + } + + pub fn validate_body(&mut self, db: &dyn HirDatabase) { + 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 + .iter() + .filter(|unsafe_expr| !unsafe_expr.inside_unsafe_block) + .count() + == 0 + { + return; + } + + let (_, body_source) = db.body_with_source_map(def); + for unsafe_expr in unsafe_expressions { + if !unsafe_expr.inside_unsafe_block { + if let Ok(in_file) = body_source.as_ref().expr_syntax(unsafe_expr.expr) { + self.sink.push(MissingUnsafe { file: in_file.file_id, expr: in_file.value }) + } + } + } + } +} -- 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_ty/src/unsafe_validation.rs | 75 ++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 2 deletions(-) (limited to 'crates/ra_hir_ty/src/unsafe_validation.rs') diff --git a/crates/ra_hir_ty/src/unsafe_validation.rs b/crates/ra_hir_ty/src/unsafe_validation.rs index 55dbe23fa..430182803 100644 --- a/crates/ra_hir_ty/src/unsafe_validation.rs +++ b/crates/ra_hir_ty/src/unsafe_validation.rs @@ -3,13 +3,16 @@ use std::sync::Arc; -use hir_def::FunctionId; +use hir_def::{DefWithBodyId, FunctionId}; use hir_expand::diagnostics::DiagnosticSink; use crate::{ - db::HirDatabase, diagnostics::MissingUnsafe, expr::unsafe_expressions, InferenceResult, + db::HirDatabase, diagnostics::MissingUnsafe, lower::CallableDef, ApplicationTy, + InferenceResult, Ty, TypeCtor, }; +use rustc_hash::FxHashSet; + pub use hir_def::{ body::{ scope::{ExprScopes, ScopeEntry, ScopeId}, @@ -61,3 +64,71 @@ impl<'a, 'b> UnsafeValidator<'a, 'b> { } } } + +pub struct UnsafeExpr { + pub expr: ExprId, + pub 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_exprs = vec![]; + let mut unsafe_block_exprs = FxHashSet::default(); + let body = db.body(def); + for (id, expr) in body.exprs.iter() { + match expr { + Expr::Unsafe { .. } => { + unsafe_block_exprs.insert(id); + } + Expr::Call { callee, .. } => { + let ty = &infer[*callee]; + if let &Ty::Apply(ApplicationTy { + ctor: TypeCtor::FnDef(CallableDef::FunctionId(func)), + .. + }) = ty + { + if db.function_data(func).is_unsafe { + unsafe_exprs.push(UnsafeExpr::new(id)); + } + } + } + Expr::MethodCall { .. } => { + if infer + .method_resolution(id) + .map(|func| db.function_data(func).is_unsafe) + .unwrap_or(false) + { + unsafe_exprs.push(UnsafeExpr::new(id)); + } + } + Expr::UnaryOp { expr, op: UnaryOp::Deref } => { + if let Ty::Apply(ApplicationTy { ctor: TypeCtor::RawPtr(..), .. }) = &infer[*expr] { + unsafe_exprs.push(UnsafeExpr::new(id)); + } + } + _ => {} + } + } + + 'unsafe_exprs: for unsafe_expr in &mut unsafe_exprs { + let mut child = unsafe_expr.expr; + while let Some(parent) = body.parent_map.get(child) { + if unsafe_block_exprs.contains(parent) { + unsafe_expr.inside_unsafe_block = true; + continue 'unsafe_exprs; + } + child = *parent; + } + } + + unsafe_exprs +} -- 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_ty/src/unsafe_validation.rs | 42 +++++++++++++++++-------------- 1 file changed, 23 insertions(+), 19 deletions(-) (limited to 'crates/ra_hir_ty/src/unsafe_validation.rs') diff --git a/crates/ra_hir_ty/src/unsafe_validation.rs b/crates/ra_hir_ty/src/unsafe_validation.rs index 430182803..f3ce7112a 100644 --- a/crates/ra_hir_ty/src/unsafe_validation.rs +++ b/crates/ra_hir_ty/src/unsafe_validation.rs @@ -13,16 +13,9 @@ use crate::{ use rustc_hash::FxHashSet; -pub use hir_def::{ - body::{ - scope::{ExprScopes, ScopeEntry, ScopeId}, - Body, BodySourceMap, ExprPtr, ExprSource, PatPtr, PatSource, - }, - expr::{ - ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Literal, LogicOp, - MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement, UnaryOp, - }, - LocalFieldId, VariantId, +use hir_def::{ + body::Body, + expr::{Expr, ExprId, UnaryOp}, }; pub struct UnsafeValidator<'a, 'b: 'a> { @@ -119,16 +112,27 @@ pub fn unsafe_expressions( } } - 'unsafe_exprs: for unsafe_expr in &mut unsafe_exprs { - let mut child = unsafe_expr.expr; - while let Some(parent) = body.parent_map.get(child) { - if unsafe_block_exprs.contains(parent) { - unsafe_expr.inside_unsafe_block = true; - continue 'unsafe_exprs; - } - child = *parent; - } + for unsafe_expr in &mut unsafe_exprs { + unsafe_expr.inside_unsafe_block = + is_in_unsafe(&body, body.body_expr, unsafe_expr.expr, false); } unsafe_exprs } + +fn is_in_unsafe(body: &Body, current: ExprId, needle: ExprId, within_unsafe: bool) -> bool { + if current == needle { + return within_unsafe; + } + + let expr = &body.exprs[current]; + if let &Expr::Unsafe { body: child } = expr { + return is_in_unsafe(body, child, needle, true); + } + + let mut found = false; + expr.walk_child_exprs(|child| { + found = found || is_in_unsafe(body, child, needle, within_unsafe); + }); + found +} -- cgit v1.2.3 From b1992b469cae689f7de01ea9031962735a409198 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Sat, 27 Jun 2020 11:20:02 -0400 Subject: Remove unneeded code, filename from tests, fix rebasing issues --- crates/ra_hir_ty/src/unsafe_validation.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) (limited to 'crates/ra_hir_ty/src/unsafe_validation.rs') diff --git a/crates/ra_hir_ty/src/unsafe_validation.rs b/crates/ra_hir_ty/src/unsafe_validation.rs index f3ce7112a..e2353404b 100644 --- a/crates/ra_hir_ty/src/unsafe_validation.rs +++ b/crates/ra_hir_ty/src/unsafe_validation.rs @@ -3,7 +3,11 @@ use std::sync::Arc; -use hir_def::{DefWithBodyId, FunctionId}; +use hir_def::{ + body::Body, + expr::{Expr, ExprId, UnaryOp}, + DefWithBodyId, FunctionId, +}; use hir_expand::diagnostics::DiagnosticSink; use crate::{ @@ -11,13 +15,6 @@ use crate::{ InferenceResult, Ty, TypeCtor, }; -use rustc_hash::FxHashSet; - -use hir_def::{ - body::Body, - expr::{Expr, ExprId, UnaryOp}, -}; - pub struct UnsafeValidator<'a, 'b: 'a> { func: FunctionId, infer: Arc, @@ -75,13 +72,9 @@ pub fn unsafe_expressions( def: DefWithBodyId, ) -> Vec { let mut unsafe_exprs = vec![]; - let mut unsafe_block_exprs = FxHashSet::default(); let body = db.body(def); for (id, expr) in body.exprs.iter() { match expr { - Expr::Unsafe { .. } => { - unsafe_block_exprs.insert(id); - } Expr::Call { callee, .. } => { let ty = &infer[*callee]; if let &Ty::Apply(ApplicationTy { -- cgit v1.2.3 From b7e25ba854a5ca0f1dee7082c113170876358632 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Sat, 27 Jun 2020 11:55:54 -0400 Subject: Improve perf of finding unsafe exprs --- crates/ra_hir_ty/src/unsafe_validation.rs | 88 ++++++++++++++----------------- 1 file changed, 39 insertions(+), 49 deletions(-) (limited to 'crates/ra_hir_ty/src/unsafe_validation.rs') diff --git a/crates/ra_hir_ty/src/unsafe_validation.rs b/crates/ra_hir_ty/src/unsafe_validation.rs index e2353404b..aad13d99c 100644 --- a/crates/ra_hir_ty/src/unsafe_validation.rs +++ b/crates/ra_hir_ty/src/unsafe_validation.rs @@ -60,12 +60,6 @@ pub struct UnsafeExpr { pub 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, @@ -73,59 +67,55 @@ pub fn unsafe_expressions( ) -> Vec { let mut unsafe_exprs = vec![]; let body = db.body(def); - for (id, expr) in body.exprs.iter() { - match expr { - Expr::Call { callee, .. } => { - let ty = &infer[*callee]; - if let &Ty::Apply(ApplicationTy { - ctor: TypeCtor::FnDef(CallableDef::FunctionId(func)), - .. - }) = ty - { - if db.function_data(func).is_unsafe { - unsafe_exprs.push(UnsafeExpr::new(id)); - } + walk_unsafe(&mut unsafe_exprs, db, infer, &body, body.body_expr, false); + + unsafe_exprs +} + +fn walk_unsafe( + unsafe_exprs: &mut Vec, + db: &dyn HirDatabase, + infer: &InferenceResult, + body: &Body, + current: ExprId, + inside_unsafe_block: bool, +) { + let expr = &body.exprs[current]; + match expr { + Expr::Call { callee, .. } => { + let ty = &infer[*callee]; + if let &Ty::Apply(ApplicationTy { + ctor: TypeCtor::FnDef(CallableDef::FunctionId(func)), + .. + }) = ty + { + if db.function_data(func).is_unsafe { + unsafe_exprs.push(UnsafeExpr { expr: current, inside_unsafe_block }); } } - Expr::MethodCall { .. } => { - if infer - .method_resolution(id) - .map(|func| db.function_data(func).is_unsafe) - .unwrap_or(false) - { - unsafe_exprs.push(UnsafeExpr::new(id)); - } + } + Expr::MethodCall { .. } => { + if infer + .method_resolution(current) + .map(|func| db.function_data(func).is_unsafe) + .unwrap_or(false) + { + unsafe_exprs.push(UnsafeExpr { expr: current, inside_unsafe_block }); } - Expr::UnaryOp { expr, op: UnaryOp::Deref } => { - if let Ty::Apply(ApplicationTy { ctor: TypeCtor::RawPtr(..), .. }) = &infer[*expr] { - unsafe_exprs.push(UnsafeExpr::new(id)); - } + } + Expr::UnaryOp { expr, op: UnaryOp::Deref } => { + if let Ty::Apply(ApplicationTy { ctor: TypeCtor::RawPtr(..), .. }) = &infer[*expr] { + unsafe_exprs.push(UnsafeExpr { expr: current, inside_unsafe_block }); } - _ => {} } + _ => {} } - for unsafe_expr in &mut unsafe_exprs { - unsafe_expr.inside_unsafe_block = - is_in_unsafe(&body, body.body_expr, unsafe_expr.expr, false); - } - - unsafe_exprs -} - -fn is_in_unsafe(body: &Body, current: ExprId, needle: ExprId, within_unsafe: bool) -> bool { - if current == needle { - return within_unsafe; - } - - let expr = &body.exprs[current]; if let &Expr::Unsafe { body: child } = expr { - return is_in_unsafe(body, child, needle, true); + return walk_unsafe(unsafe_exprs, db, infer, body, child, true); } - let mut found = false; expr.walk_child_exprs(|child| { - found = found || is_in_unsafe(body, child, needle, within_unsafe); + walk_unsafe(unsafe_exprs, db, infer, body, child, inside_unsafe_block); }); - found } -- cgit v1.2.3 From 68a649d547c844cb44ee619b7f45d1193dad2b02 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Sat, 27 Jun 2020 12:00:46 -0400 Subject: Simplify unsafe expr collection match --- crates/ra_hir_ty/src/unsafe_validation.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'crates/ra_hir_ty/src/unsafe_validation.rs') diff --git a/crates/ra_hir_ty/src/unsafe_validation.rs b/crates/ra_hir_ty/src/unsafe_validation.rs index aad13d99c..c512c4f8e 100644 --- a/crates/ra_hir_ty/src/unsafe_validation.rs +++ b/crates/ra_hir_ty/src/unsafe_validation.rs @@ -108,13 +108,12 @@ fn walk_unsafe( unsafe_exprs.push(UnsafeExpr { expr: current, inside_unsafe_block }); } } + Expr::Unsafe { body: child } => { + return walk_unsafe(unsafe_exprs, db, infer, body, *child, true); + } _ => {} } - if let &Expr::Unsafe { body: child } = expr { - return walk_unsafe(unsafe_exprs, db, infer, body, child, true); - } - expr.walk_child_exprs(|child| { walk_unsafe(unsafe_exprs, db, infer, body, child, inside_unsafe_block); }); -- cgit v1.2.3