From c187074e6d40e2a9f8a4af4aee2f23792959a266 Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Mon, 17 Feb 2020 20:27:35 +0100 Subject: [PATCH 1/3] Remove "unnecessary `unsafe` block in `unsafe` fn" lint --- src/librustc_mir/transform/check_unsafety.rs | 8 ------- src/librustc_mir_build/build/block.rs | 2 +- src/test/ui/span/lint-unused-unsafe.rs | 4 ++-- src/test/ui/span/lint-unused-unsafe.stderr | 25 +++----------------- 4 files changed, 6 insertions(+), 33 deletions(-) diff --git a/src/librustc_mir/transform/check_unsafety.rs b/src/librustc_mir/transform/check_unsafety.rs index 437a154a9b80f..070ac4170595f 100644 --- a/src/librustc_mir/transform/check_unsafety.rs +++ b/src/librustc_mir/transform/check_unsafety.rs @@ -548,14 +548,6 @@ fn is_enclosed( if parent_id != id { if used_unsafe.contains(&parent_id) { Some(("block".to_string(), parent_id)) - } else if let Some(Node::Item(&hir::Item { - 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, - } } else { is_enclosed(tcx, used_unsafe, parent_id) } diff --git a/src/librustc_mir_build/build/block.rs b/src/librustc_mir_build/build/block.rs index df5526ad76281..f9272a5abc622 100644 --- a/src/librustc_mir_build/build/block.rs +++ b/src/librustc_mir_build/build/block.rs @@ -210,7 +210,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { BlockSafety::ExplicitUnsafe(hir_id) => { assert_eq!(self.push_unsafe_count, 0); match self.unpushed_unsafe { - Safety::Safe => {} + Safety::Safe | Safety::FnUnsafe => {} _ => return, } self.unpushed_unsafe = Safety::ExplicitUnsafe(hir_id); diff --git a/src/test/ui/span/lint-unused-unsafe.rs b/src/test/ui/span/lint-unused-unsafe.rs index 338fbb994c5c5..cec78cfbea9b9 100644 --- a/src/test/ui/span/lint-unused-unsafe.rs +++ b/src/test/ui/span/lint-unused-unsafe.rs @@ -17,7 +17,7 @@ fn bad1() { unsafe {} } //~ ERROR: unnecessary `unsafe` block fn bad2() { unsafe { bad1() } } //~ ERROR: unnecessary `unsafe` block unsafe fn bad3() { unsafe {} } //~ ERROR: unnecessary `unsafe` block fn bad4() { unsafe { callback(||{}) } } //~ ERROR: unnecessary `unsafe` block -unsafe fn bad5() { unsafe { unsf() } } //~ ERROR: unnecessary `unsafe` block +unsafe fn bad5() { unsafe { unsf() } } fn bad6() { unsafe { // don't put the warning here unsafe { //~ ERROR: unnecessary `unsafe` block @@ -26,7 +26,7 @@ fn bad6() { } } unsafe fn bad7() { - unsafe { //~ ERROR: unnecessary `unsafe` block + unsafe { unsafe { //~ ERROR: unnecessary `unsafe` block unsf() } diff --git a/src/test/ui/span/lint-unused-unsafe.stderr b/src/test/ui/span/lint-unused-unsafe.stderr index c35a3349121b7..a5eb896903005 100644 --- a/src/test/ui/span/lint-unused-unsafe.stderr +++ b/src/test/ui/span/lint-unused-unsafe.stderr @@ -20,9 +20,7 @@ error: unnecessary `unsafe` block --> $DIR/lint-unused-unsafe.rs:18:20 | LL | unsafe fn bad3() { unsafe {} } - | ---------------- ^^^^^^ unnecessary `unsafe` block - | | - | because it's nested under this `unsafe` fn + | ^^^^^^ unnecessary `unsafe` block error: unnecessary `unsafe` block --> $DIR/lint-unused-unsafe.rs:19:13 @@ -30,14 +28,6 @@ error: unnecessary `unsafe` block LL | fn bad4() { unsafe { callback(||{}) } } | ^^^^^^ unnecessary `unsafe` block -error: unnecessary `unsafe` block - --> $DIR/lint-unused-unsafe.rs:20:20 - | -LL | unsafe fn bad5() { unsafe { unsf() } } - | ---------------- ^^^^^^ unnecessary `unsafe` block - | | - | because it's nested under this `unsafe` fn - error: unnecessary `unsafe` block --> $DIR/lint-unused-unsafe.rs:23:9 | @@ -46,22 +36,13 @@ LL | unsafe { // don't put the warning here LL | unsafe { | ^^^^^^ unnecessary `unsafe` block -error: unnecessary `unsafe` block - --> $DIR/lint-unused-unsafe.rs:29:5 - | -LL | unsafe fn bad7() { - | ---------------- because it's nested under this `unsafe` fn -LL | unsafe { - | ^^^^^^ unnecessary `unsafe` block - error: unnecessary `unsafe` block --> $DIR/lint-unused-unsafe.rs:30:9 | -LL | unsafe fn bad7() { - | ---------------- because it's nested under this `unsafe` fn LL | unsafe { + | ------ because it's nested under this `unsafe` block LL | unsafe { | ^^^^^^ unnecessary `unsafe` block -error: aborting due to 8 previous errors +error: aborting due to 6 previous errors From 27d1fb8b49e3df8e3af5b501215b9c4ddff882df Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Mon, 17 Feb 2020 21:33:04 +0100 Subject: [PATCH 2/3] Add regression test --- .../ui/lint/issue-69173-unsafe-block-in-unsafe-fn.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 src/test/ui/lint/issue-69173-unsafe-block-in-unsafe-fn.rs diff --git a/src/test/ui/lint/issue-69173-unsafe-block-in-unsafe-fn.rs b/src/test/ui/lint/issue-69173-unsafe-block-in-unsafe-fn.rs new file mode 100644 index 0000000000000..6e86f162b43ba --- /dev/null +++ b/src/test/ui/lint/issue-69173-unsafe-block-in-unsafe-fn.rs @@ -0,0 +1,11 @@ +// Regression test for #69173: do not warn for `unsafe` blocks in `unsafe` functions + +// check-pass + +#![allow(dead_code)] +#![deny(unused_unsafe)] + +unsafe fn foo() {} +unsafe fn bar() { unsafe { foo(); } } + +fn main() {} From ffb2e03bf1dac8f2a3177e5a104da694b3b5fb93 Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Thu, 19 Mar 2020 16:15:47 +0100 Subject: [PATCH 3/3] Fix bad conflict resolvement --- src/librustc_mir/transform/check_unsafety.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc_mir/transform/check_unsafety.rs b/src/librustc_mir/transform/check_unsafety.rs index 070ac4170595f..c1213d430e70d 100644 --- a/src/librustc_mir/transform/check_unsafety.rs +++ b/src/librustc_mir/transform/check_unsafety.rs @@ -8,7 +8,6 @@ use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_hir::intravisit; -use rustc_hir::Node; use rustc_session::lint::builtin::{SAFE_PACKED_BORROWS, UNUSED_UNSAFE}; use rustc_span::symbol::{sym, Symbol};