1
Fork 0

Rollup merge of #86517 - camsteffen:unused-unsafe-async, r=LeSeulArtichaut

Fix `unused_unsafe` around `await`

Enables `unused_unsafe` lint for `unsafe { future.await }`.

The existing test for this is `unsafe { println!() }`, so I assume that `println!` used to contain compiler-generated unsafe but this is no longer true, and so the existing test is broken. I replaced the test with `unsafe { ...await }`. I believe `await` is currently the only instance of compiler-generated unsafe.

Reverts some parts of #85421, but the issue predates that PR.
This commit is contained in:
Yuki Okushi 2021-06-22 20:01:05 +09:00 committed by GitHub
commit 8ec4e7dfdd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 43 additions and 25 deletions

View file

@ -494,6 +494,8 @@ impl<'tcx> Body<'tcx> {
#[derive(Copy, Clone, PartialEq, Eq, Debug, TyEncodable, TyDecodable, HashStable)] #[derive(Copy, Clone, PartialEq, Eq, Debug, TyEncodable, TyDecodable, HashStable)]
pub enum Safety { pub enum Safety {
Safe, Safe,
/// Unsafe because of compiler-generated unsafe code, like `await` desugaring
BuiltinUnsafe,
/// Unsafe because of an unsafe fn /// Unsafe because of an unsafe fn
FnUnsafe, FnUnsafe,
/// Unsafe because of an `unsafe` block /// Unsafe because of an `unsafe` block

View file

@ -114,6 +114,7 @@ pub struct Adt<'tcx> {
#[derive(Copy, Clone, Debug, HashStable)] #[derive(Copy, Clone, Debug, HashStable)]
pub enum BlockSafety { pub enum BlockSafety {
Safe, Safe,
BuiltinUnsafe,
ExplicitUnsafe(hir::HirId), ExplicitUnsafe(hir::HirId),
} }

View file

@ -321,6 +321,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
} }
false false
} }
Safety::BuiltinUnsafe => true,
Safety::ExplicitUnsafe(hir_id) => { Safety::ExplicitUnsafe(hir_id) => {
// mark unsafe block as used if there are any unsafe operations inside // mark unsafe block as used if there are any unsafe operations inside
if !violations.is_empty() { if !violations.is_empty() {

View file

@ -214,6 +214,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
debug!("update_source_scope_for({:?}, {:?})", span, safety_mode); debug!("update_source_scope_for({:?}, {:?})", span, safety_mode);
let new_unsafety = match safety_mode { let new_unsafety = match safety_mode {
BlockSafety::Safe => None, BlockSafety::Safe => None,
BlockSafety::BuiltinUnsafe => Some(Safety::BuiltinUnsafe),
BlockSafety::ExplicitUnsafe(hir_id) => { BlockSafety::ExplicitUnsafe(hir_id) => {
match self.in_scope_unsafe { match self.in_scope_unsafe {
Safety::Safe => {} Safety::Safe => {}

View file

@ -29,11 +29,7 @@ struct UnsafetyVisitor<'a, 'tcx> {
} }
impl<'tcx> UnsafetyVisitor<'_, 'tcx> { impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
fn in_safety_context<R>( fn in_safety_context(&mut self, safety_context: SafetyContext, f: impl FnOnce(&mut Self)) {
&mut self,
safety_context: SafetyContext,
f: impl FnOnce(&mut Self) -> R,
) {
if let ( if let (
SafetyContext::UnsafeBlock { span: enclosing_span, .. }, SafetyContext::UnsafeBlock { span: enclosing_span, .. },
SafetyContext::UnsafeBlock { span: block_span, hir_id, .. }, SafetyContext::UnsafeBlock { span: block_span, hir_id, .. },
@ -63,7 +59,6 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
); );
} }
self.safety_context = prev_context; self.safety_context = prev_context;
return;
} }
} }
@ -71,6 +66,7 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
let (description, note) = kind.description_and_note(); let (description, note) = kind.description_and_note();
let unsafe_op_in_unsafe_fn_allowed = self.unsafe_op_in_unsafe_fn_allowed(); let unsafe_op_in_unsafe_fn_allowed = self.unsafe_op_in_unsafe_fn_allowed();
match self.safety_context { match self.safety_context {
SafetyContext::BuiltinUnsafeBlock => {}
SafetyContext::UnsafeBlock { ref mut used, .. } => { SafetyContext::UnsafeBlock { ref mut used, .. } => {
if !self.body_unsafety.is_unsafe() || !unsafe_op_in_unsafe_fn_allowed { if !self.body_unsafety.is_unsafe() || !unsafe_op_in_unsafe_fn_allowed {
// Mark this block as useful // Mark this block as useful
@ -142,13 +138,23 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
} }
fn visit_block(&mut self, block: &Block) { fn visit_block(&mut self, block: &Block) {
if let BlockSafety::ExplicitUnsafe(hir_id) = block.safety_mode { match block.safety_mode {
self.in_safety_context( // compiler-generated unsafe code should not count towards the usefulness of
SafetyContext::UnsafeBlock { span: block.span, hir_id, used: false }, // an outer unsafe block
|this| visit::walk_block(this, block), BlockSafety::BuiltinUnsafe => {
); self.in_safety_context(SafetyContext::BuiltinUnsafeBlock, |this| {
} else { visit::walk_block(this, block)
visit::walk_block(self, block); });
}
BlockSafety::ExplicitUnsafe(hir_id) => {
self.in_safety_context(
SafetyContext::UnsafeBlock { span: block.span, hir_id, used: false },
|this| visit::walk_block(this, block),
);
}
BlockSafety::Safe => {
visit::walk_block(self, block);
}
} }
} }
@ -250,6 +256,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
#[derive(Clone, Copy)] #[derive(Clone, Copy)]
enum SafetyContext { enum SafetyContext {
Safe, Safe,
BuiltinUnsafeBlock,
UnsafeFn, UnsafeFn,
UnsafeBlock { span: Span, hir_id: hir::HirId, used: bool }, UnsafeBlock { span: Span, hir_id: hir::HirId, used: bool },
} }

View file

@ -26,7 +26,12 @@ impl<'tcx> Cx<'tcx> {
expr: block.expr.map(|expr| self.mirror_expr(expr)), expr: block.expr.map(|expr| self.mirror_expr(expr)),
safety_mode: match block.rules { safety_mode: match block.rules {
hir::BlockCheckMode::DefaultBlock => BlockSafety::Safe, hir::BlockCheckMode::DefaultBlock => BlockSafety::Safe,
hir::BlockCheckMode::UnsafeBlock(..) => BlockSafety::ExplicitUnsafe(block.hir_id), hir::BlockCheckMode::UnsafeBlock(hir::UnsafeSource::CompilerGenerated) => {
BlockSafety::BuiltinUnsafe
}
hir::BlockCheckMode::UnsafeBlock(hir::UnsafeSource::UserProvided) => {
BlockSafety::ExplicitUnsafe(block.hir_id)
}
}, },
} }
} }

View file

@ -1,11 +1,11 @@
error: unnecessary `unsafe` block error: unnecessary `unsafe` block
--> $DIR/unsafe-around-compiler-generated-unsafe.rs:9:5 --> $DIR/unsafe-around-compiler-generated-unsafe.rs:9:9
| |
LL | unsafe { println!("foo"); } LL | unsafe { async {}.await; }
| ^^^^^^ unnecessary `unsafe` block | ^^^^^^ unnecessary `unsafe` block
| |
note: the lint level is defined here note: the lint level is defined here
--> $DIR/unsafe-around-compiler-generated-unsafe.rs:6:9 --> $DIR/unsafe-around-compiler-generated-unsafe.rs:5:9
| |
LL | #![deny(unused_unsafe)] LL | #![deny(unused_unsafe)]
| ^^^^^^^^^^^^^ | ^^^^^^^^^^^^^

View file

@ -1,10 +1,11 @@
// issue #12418 // edition:2018
// revisions: mir thir // revisions: mir thir
// [thir]compile-flags: -Z thir-unsafeck // [thir]compile-flags: -Z thir-unsafeck
#![deny(unused_unsafe)] #![deny(unused_unsafe)]
fn main() { fn main() {
unsafe { println!("foo"); } //~ ERROR unnecessary `unsafe` let _ = async {
unsafe { async {}.await; } //~ ERROR unnecessary `unsafe`
};
} }

View file

@ -1,11 +1,11 @@
error: unnecessary `unsafe` block error: unnecessary `unsafe` block
--> $DIR/unsafe-around-compiler-generated-unsafe.rs:9:5 --> $DIR/unsafe-around-compiler-generated-unsafe.rs:9:9
| |
LL | unsafe { println!("foo"); } LL | unsafe { async {}.await; }
| ^^^^^^ unnecessary `unsafe` block | ^^^^^^ unnecessary `unsafe` block
| |
note: the lint level is defined here note: the lint level is defined here
--> $DIR/unsafe-around-compiler-generated-unsafe.rs:6:9 --> $DIR/unsafe-around-compiler-generated-unsafe.rs:5:9
| |
LL | #![deny(unused_unsafe)] LL | #![deny(unused_unsafe)]
| ^^^^^^^^^^^^^ | ^^^^^^^^^^^^^