Implement RFC 2585

Author: LeSeulArtichaut

src/librustc_feature/active.rs

@@ -571,6 +571,9 @@ declare_features! (
     /// Allows the use of `#[ffi_const]` on foreign functions.
     (active, ffi_const, "1.45.0", Some(58328), None),
 
+    /// No longer treat an unsafe function as an unsafe block.
+    (active, unsafe_block_in_unsafe_fn, "1.45.0", Some(71668), None),
+
     // -------------------------------------------------------------------------
     // feature-group-end: actual feature gates
     // -------------------------------------------------------------------------

src/librustc_middle/mir/mod.rs

@@ -408,7 +408,7 @@ impl<'tcx> Body<'tcx> {
     }
 }
 
-#[derive(Copy, Clone, Debug, RustcEncodable, RustcDecodable, HashStable)]
+#[derive(Copy, Clone, PartialEq, Eq, Debug, RustcEncodable, RustcDecodable, HashStable)]
 pub enum Safety {
     Safe,
     /// Unsafe because of a PushUnsafeBlock

src/librustc_middle/mir/query.rs

@@ -15,10 +15,17 @@ use super::{Field, SourceInfo};
 
 #[derive(Copy, Clone, PartialEq, RustcEncodable, RustcDecodable, HashStable)]
 pub enum UnsafetyViolationKind {
+    /// Only permitted in regular `fn`s, prohibitted in `const fn`s.
     General,
     /// Permitted both in `const fn`s and regular `fn`s.
     GeneralAndConstFn,
+    /// Borrow of packed field.
+    /// Has to be handled as a lint for backwards compatibility.
     BorrowPacked(hir::HirId),
+    /// Unsafe operation in an `unsafe fn` but outside an `unsafe` block.
+    /// Has to be handled as a lint for backwards compatibility.
+    /// Should stay gated under `#![feature(unsafe_block_in_unsafe_fn)]`.
+    UnsafeFn(hir::HirId),
 }
 
 #[derive(Copy, Clone, PartialEq, RustcEncodable, RustcDecodable, HashStable)]

src/librustc_mir/transform/check_unsafety.rs

@@ -9,7 +9,7 @@ use rustc_middle::mir::*;
 use rustc_middle::ty::cast::CastTy;
 use rustc_middle::ty::query::Providers;
 use rustc_middle::ty::{self, TyCtxt};
-use rustc_session::lint::builtin::{SAFE_PACKED_BORROWS, UNUSED_UNSAFE};
+use rustc_session::lint::builtin::{SAFE_PACKED_BORROWS, UNSAFE_OP_IN_UNSAFE_FN, UNUSED_UNSAFE};
 use rustc_span::symbol::{sym, Symbol};
 
 use std::ops::Bound;
@@ -351,14 +351,35 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
                                 violation.kind = UnsafetyViolationKind::General;
                             }
                         }
+                        UnsafetyViolationKind::UnsafeFn(_) => {
+                            bug!("`UnsafetyViolationKind::UnsafeFn` in an `Safe` context")
+                        }
+                    }
+                    if !self.violations.contains(&violation) {
+                        self.violations.push(violation)
                     }
+                }
+                false
+            }
+            // With the RFC 2585, no longer allow `unsafe` operations in `unsafe fn`s
+            Safety::FnUnsafe if self.tcx.features().unsafe_block_in_unsafe_fn => {
+                for violation in violations {
+                    let mut violation = *violation;
+                    let lint_root = self.body.source_scopes[self.source_info.scope]
+                        .local_data
+                        .as_ref()
+                        .assert_crate_local()
+                        .lint_root;
+
+                    // FIXME(LeSeulArtichaut): what to do with `UnsafetyViolationKind::BorrowPacked`?
+                    violation.kind = UnsafetyViolationKind::UnsafeFn(lint_root);
                     if !self.violations.contains(&violation) {
                         self.violations.push(violation)
                     }
                 }
                 false
             }
-            // `unsafe` function bodies allow unsafe without additional unsafe blocks
+            // `unsafe` function bodies allow unsafe without additional unsafe blocks (before RFC 2585)
             Safety::BuiltinUnsafe | Safety::FnUnsafe => true,
             Safety::ExplicitUnsafe(hir_id) => {
                 // mark unsafe block as used if there are any unsafe operations inside
@@ -383,6 +404,9 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
                                     self.violations.push(violation)
                                 }
                             }
+                            UnsafetyViolationKind::UnsafeFn(_) => bug!(
+                                "`UnsafetyViolationKind::UnsafeFn` in an `ExplicitUnsafe` context"
+                            ),
                         }
                     }
                 }
@@ -575,9 +599,12 @@ fn is_enclosed(
             kind: hir::ItemKind::Fn(ref sig, _, _), ..
         })) = tcx.hir().find(parent_id)
         {
-            match sig.header.unsafety {
-                hir::Unsafety::Unsafe => Some(("fn".to_string(), parent_id)),
-                hir::Unsafety::Normal => None,
+            if sig.header.unsafety == hir::Unsafety::Unsafe
+                && !tcx.features().unsafe_block_in_unsafe_fn
+            {
+                Some(("fn".to_string(), parent_id))
+            } else {
+                None
             }
         } else {
             is_enclosed(tcx, used_unsafe, parent_id)
@@ -630,16 +657,20 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: DefId) {
     let UnsafetyCheckResult { violations, unsafe_blocks } =
         tcx.unsafety_check_result(def_id.expect_local());
 
+    let or_block_msg = if tcx.features().unsafe_block_in_unsafe_fn { "" } else { " or block" };
+
     for &UnsafetyViolation { source_info, description, details, kind } in violations.iter() {
         // Report an error.
         match kind {
             UnsafetyViolationKind::GeneralAndConstFn | UnsafetyViolationKind::General => {
+                // once
                 struct_span_err!(
                     tcx.sess,
                     source_info.span,
                     E0133,
-                    "{} is unsafe and requires unsafe function or block",
-                    description
+                    "{} is unsafe and requires unsafe function{}",
+                    description,
+                    or_block_msg,
                 )
                 .span_label(source_info.span, &*description.as_str())
                 .note(&details.as_str())
@@ -655,15 +686,29 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: DefId) {
                         source_info.span,
                         |lint| {
                             lint.build(&format!(
-                                "{} is unsafe and requires unsafe function or block (error E0133)",
-                                description
+                                "{} is unsafe and requires unsafe function{} (error E0133)",
+                                description, or_block_msg,
                             ))
                             .note(&details.as_str())
                             .emit()
                         },
                     )
                 }
             }
+            UnsafetyViolationKind::UnsafeFn(lint_hir_id) => tcx.struct_span_lint_hir(
+                UNSAFE_OP_IN_UNSAFE_FN,
+                lint_hir_id,
+                source_info.span,
+                |lint| {
+                    lint.build(&format!(
+                        "{} is unsafe and requires unsafe block (error E0133)",
+                        description
+                    ))
+                    .span_label(source_info.span, &*description.as_str())
+                    .note(&details.as_str())
+                    .emit();
+                },
+            ),
         }
     }
 

src/librustc_mir_build/build/block.rs

@@ -217,6 +217,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 assert_eq!(self.push_unsafe_count, 0);
                 match self.unpushed_unsafe {
                     Safety::Safe => {}
+                    // no longer treat `unsafe fn`s as `unsafe` contexts (see RFC #2585)
+                    Safety::FnUnsafe if self.hir.tcx().features().unsafe_block_in_unsafe_fn => {}
                     _ => return,
                 }
                 self.unpushed_unsafe = Safety::ExplicitUnsafe(hir_id);

src/librustc_session/lint/builtin.rs

@@ -526,6 +526,12 @@ declare_lint! {
     "using only a subset of a register for inline asm inputs",
 }
 
+declare_lint! {
+    pub UNSAFE_OP_IN_UNSAFE_FN,
+    Allow,
+    "unsafe operations in unsafe functions without an explicit unsafe block are deprecated",
+}
+
 declare_lint_pass! {
     /// Does nothing as a lint pass, but registers some `Lint`s
     /// that are used by other parts of the compiler.
@@ -597,6 +603,7 @@ declare_lint_pass! {
         SOFT_UNSTABLE,
         INLINE_NO_SANITIZE,
         ASM_SUB_REGISTER,
+        UNSAFE_OP_IN_UNSAFE_FN,
     ]
 }
 

src/librustc_span/symbol.rs

@@ -806,6 +806,7 @@ symbols! {
         unmarked_api,
         unreachable_code,
         unrestricted_attribute_tokens,
+        unsafe_block_in_unsafe_fn,
         unsafe_no_drop_flag,
         unsized_locals,
         unsized_tuple_coercion,