Rollup merge of rust-lang#86517 - camsteffen:unused-unsafe-async, r=L…

…eSeulArtichaut 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 rust-lang#85421, but the issue predates that PR.
JohnTitor · Jun 22, 2021 · 8ec4e7d · 8ec4e7d
2 parents 8af9339 + b07bb6d
commit 8ec4e7d
Show file tree

Hide file tree

Showing 9 changed files with 43 additions and 25 deletions.
diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs
@@ -494,6 +494,8 @@ impl<'tcx> Body<'tcx> {
 #[derive(Copy, Clone, PartialEq, Eq, Debug, TyEncodable, TyDecodable, HashStable)]
 pub enum Safety {
     Safe,
+    /// Unsafe because of compiler-generated unsafe code, like `await` desugaring
+    BuiltinUnsafe,
     /// Unsafe because of an unsafe fn
     FnUnsafe,
     /// Unsafe because of an `unsafe` block

diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs
@@ -114,6 +114,7 @@ pub struct Adt<'tcx> {
 #[derive(Copy, Clone, Debug, HashStable)]
 pub enum BlockSafety {
     Safe,
+    BuiltinUnsafe,
     ExplicitUnsafe(hir::HirId),
 }
 

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

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

diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs
@@ -29,11 +29,7 @@ struct UnsafetyVisitor<'a, 'tcx> {
 }
 
 impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
-    fn in_safety_context<R>(
-        &mut self,
-        safety_context: SafetyContext,
-        f: impl FnOnce(&mut Self) -> R,
-    ) {
+    fn in_safety_context(&mut self, safety_context: SafetyContext, f: impl FnOnce(&mut Self)) {
         if let (
             SafetyContext::UnsafeBlock { span: enclosing_span, .. },
             SafetyContext::UnsafeBlock { span: block_span, hir_id, .. },
@@ -63,14 +59,14 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
                 );
             }
             self.safety_context = prev_context;
-            return;
         }
     }
 
     fn requires_unsafe(&mut self, span: Span, kind: UnsafeOpKind) {
         let (description, note) = kind.description_and_note();
         let unsafe_op_in_unsafe_fn_allowed = self.unsafe_op_in_unsafe_fn_allowed();
         match self.safety_context {
+            SafetyContext::BuiltinUnsafeBlock => {}
             SafetyContext::UnsafeBlock { ref mut used, .. } => {
                 if !self.body_unsafety.is_unsafe() || !unsafe_op_in_unsafe_fn_allowed {
                     // 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) {
-        if let BlockSafety::ExplicitUnsafe(hir_id) = block.safety_mode {
-            self.in_safety_context(
-                SafetyContext::UnsafeBlock { span: block.span, hir_id, used: false },
-                |this| visit::walk_block(this, block),
-            );
-        } else {
-            visit::walk_block(self, block);
+        match block.safety_mode {
+            // compiler-generated unsafe code should not count towards the usefulness of
+            // an outer unsafe block
+            BlockSafety::BuiltinUnsafe => {
+                self.in_safety_context(SafetyContext::BuiltinUnsafeBlock, |this| {
+                    visit::walk_block(this, 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)]
 enum SafetyContext {
     Safe,
+    BuiltinUnsafeBlock,
     UnsafeFn,
     UnsafeBlock { span: Span, hir_id: hir::HirId, used: bool },
 }

diff --git a/compiler/rustc_mir_build/src/thir/cx/block.rs b/compiler/rustc_mir_build/src/thir/cx/block.rs
@@ -26,7 +26,12 @@ impl<'tcx> Cx<'tcx> {
             expr: block.expr.map(|expr| self.mirror_expr(expr)),
             safety_mode: match block.rules {
                 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)
+                }
             },
         }
     }

diff --git a/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.mir.stderr b/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.mir.stderr
@@ -1,11 +1,11 @@
 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"); }
-   |     ^^^^^^ unnecessary `unsafe` block
+LL |         unsafe { async {}.await; }
+   |         ^^^^^^ unnecessary `unsafe` block
    |
 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)]
    |         ^^^^^^^^^^^^^

diff --git a/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.rs b/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.rs
@@ -1,10 +1,11 @@
-// issue #12418
-
+// edition:2018
 // revisions: mir thir
 // [thir]compile-flags: -Z thir-unsafeck
 
 #![deny(unused_unsafe)]
 
 fn main() {
-    unsafe { println!("foo"); } //~ ERROR unnecessary `unsafe`
+    let _ = async {
+        unsafe { async {}.await; } //~ ERROR unnecessary `unsafe`
+    };
 }
diff --git a/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.thir.stderr b/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.thir.stderr
@@ -1,11 +1,11 @@
 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"); }
-   |     ^^^^^^ unnecessary `unsafe` block
+LL |         unsafe { async {}.await; }
+   |         ^^^^^^ unnecessary `unsafe` block
    |
 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)]
    |         ^^^^^^^^^^^^^