Track unsafe blocks, don't trigger missing unsafe diagnostic when unsafe exprs within unsafe block

This commit is contained in:
Paul Daniel Faria 2020-05-24 16:24:36 -04:00
parent 3df0f9ce7e
commit 278cbf12cd
7 changed files with 82 additions and 19 deletions

View file

@ -218,8 +218,12 @@ impl ExprCollector<'_> {
let body = self.collect_block_opt(e.block_expr()); 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)
} }
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... // 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()) self.collect_block_opt(e.block_expr())
} }
}, },

View file

@ -150,6 +150,9 @@ pub enum Expr {
Tuple { Tuple {
exprs: Vec<ExprId>, exprs: Vec<ExprId>,
}, },
UnsafeBlock {
body: ExprId,
},
Array(Array), Array(Array),
Literal(Literal), Literal(Literal),
} }
@ -247,7 +250,7 @@ impl Expr {
f(*expr); f(*expr);
} }
} }
Expr::TryBlock { body } => f(*body), Expr::TryBlock { body } | Expr::UnsafeBlock { body } => f(*body),
Expr::Loop { body, .. } => f(*body), Expr::Loop { body, .. } => f(*body),
Expr::While { condition, body, .. } => { Expr::While { condition, body, .. } => {
f(*condition); f(*condition);

View file

@ -318,46 +318,74 @@ pub fn record_pattern_missing_fields(
Some((variant_def, missed_fields, exhaustive)) 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( pub fn unsafe_expressions(
db: &dyn HirDatabase, db: &dyn HirDatabase,
infer: &InferenceResult, infer: &InferenceResult,
def: DefWithBodyId, def: DefWithBodyId,
) -> Vec<ExprId> { ) -> Vec<UnsafeExpr> {
let mut unsafe_expr_ids = vec![]; let mut unsafe_exprs = vec![];
let mut unsafe_block_scopes = vec![];
let body = db.body(def); let body = db.body(def);
let expr_scopes = db.expr_scopes(def);
for (id, expr) in body.exprs.iter() { for (id, expr) in body.exprs.iter() {
match expr { match expr {
Expr::UnsafeBlock { body } => {
if let Some(scope) = expr_scopes.scope_for(*body) {
unsafe_block_scopes.push(scope);
}
}
Expr::Call { callee, .. } => { Expr::Call { callee, .. } => {
let ty = &infer.type_of_expr[*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 { if db.function_data(func).is_unsafe {
unsafe_expr_ids.push(id); unsafe_exprs.push(UnsafeExpr::new(id));
} }
} }
} }
Expr::MethodCall { .. } => { Expr::MethodCall { .. } => {
if infer if infer
.method_resolution(id) .method_resolution(id)
.map(|func| { .map(|func| db.function_data(func).is_unsafe)
db.function_data(func).is_unsafe .unwrap_or_else(|| false)
})
.unwrap_or_else(|| {
false
})
{ {
unsafe_expr_ids.push(id); unsafe_exprs.push(UnsafeExpr::new(id));
} }
} }
Expr::UnaryOp { expr, op: UnaryOp::Deref } => { Expr::UnaryOp { expr, op: UnaryOp::Deref } => {
if let Ty::Apply(ApplicationTy { ctor: TypeCtor::RawPtr(..), .. }) = &infer[*expr] { 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> { pub struct UnsafeValidator<'a, 'b: 'a> {
@ -379,7 +407,13 @@ impl<'a, 'b> UnsafeValidator<'a, 'b> {
let def = self.func.into(); let def = self.func.into();
let unsafe_expressions = unsafe_expressions(db, self.infer.as_ref(), def); let unsafe_expressions = unsafe_expressions(db, self.infer.as_ref(), def);
let func_data = db.function_data(self.func); 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; return;
} }

View file

@ -142,6 +142,7 @@ impl<'a> InferenceContext<'a> {
// FIXME: Breakable block inference // FIXME: Breakable block inference
self.infer_block(statements, *tail, expected) self.infer_block(statements, *tail, expected)
} }
Expr::UnsafeBlock { body } => self.infer_expr(*body, expected),
Expr::TryBlock { body } => { Expr::TryBlock { body } => {
let _inner = self.infer_expr(*body, expected); let _inner = self.infer_expr(*body, expected);
// FIXME should be std::result::Result<{inner}, _> // FIXME should be std::result::Result<{inner}, _>

View file

@ -608,8 +608,8 @@ fn no_missing_unsafe_diagnostic_with_raw_ptr_in_unsafe_block() {
r" r"
//- /lib.rs //- /lib.rs
fn nothing_to_see_move_along() { fn nothing_to_see_move_along() {
unsafe {
let x = &5 as *const usize; let x = &5 as *const usize;
unsafe {
let y = *x; let y = *x;
} }
} }
@ -621,6 +621,26 @@ fn nothing_to_see_move_along() {
assert_snapshot!(diagnostics, @""); 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 y = *x;
}
let z = *x;
}
",
)
.diagnostics()
.0;
assert_snapshot!(diagnostics, @"");
}
#[test] #[test]
fn no_missing_unsafe_diagnostic_with_unsafe_call_in_unsafe_block() { fn no_missing_unsafe_diagnostic_with_unsafe_call_in_unsafe_block() {
let diagnostics = TestDB::with_files( let diagnostics = TestDB::with_files(

View file

@ -1880,6 +1880,7 @@ fn main() {
@r###" @r###"
10..130 '{ ...2 }; }': () 10..130 '{ ...2 }; }': ()
20..21 'x': i32 20..21 'x': i32
24..37 'unsafe { 92 }': i32
31..37 '{ 92 }': i32 31..37 '{ 92 }': i32
33..35 '92': i32 33..35 '92': i32
47..48 'y': {unknown} 47..48 'y': {unknown}

View file

@ -899,7 +899,7 @@ impl ast::LoopBodyOwner for LoopExpr {}
impl LoopExpr { impl LoopExpr {
pub fn loop_token(&self) -> Option<SyntaxToken> { support::token(&self.syntax, T![loop]) } pub fn loop_token(&self) -> Option<SyntaxToken> { 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...). /// unsafe keyword, async keyword...).
/// ///
/// ``` /// ```