From ac2bc7c570eec8a024cec779908c8ae718924e77 Mon Sep 17 00:00:00 2001 From: Esteban Küber Date: Thu, 19 Jan 2017 23:17:48 -0800 Subject: Point to enclosing block/fn on nested unsafe When declaring nested unsafe blocks (`unsafe {unsafe {}}`) that trigger the "unnecessary `unsafe` block" error, point out the enclosing `unsafe block` or `unsafe fn` that makes it unnecessary. --- src/librustc_lint/unused.rs | 29 ++++++- src/test/compile-fail/lint-unused-unsafe.rs | 66 ---------------- src/test/ui/span/lint-unused-unsafe.rs | 66 ++++++++++++++++ src/test/ui/span/lint-unused-unsafe.stderr | 116 ++++++++++++++++++++++++++++ 4 files changed, 210 insertions(+), 67 deletions(-) delete mode 100644 src/test/compile-fail/lint-unused-unsafe.rs create mode 100644 src/test/ui/span/lint-unused-unsafe.rs create mode 100644 src/test/ui/span/lint-unused-unsafe.stderr (limited to 'src') diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 28ce9126019..f9b7c685876 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -189,11 +189,38 @@ impl LintPass for UnusedUnsafe { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedUnsafe { fn check_expr(&mut self, cx: &LateContext, e: &hir::Expr) { + /// Return the NodeId for an enclosing scope that is also `unsafe` + fn is_enclosed(cx: &LateContext, id: ast::NodeId) -> Option<(String, ast::NodeId)> { + let parent_id = cx.tcx.hir.get_parent_node(id); + if parent_id != id { + if cx.tcx.used_unsafe.borrow().contains(&parent_id) { + Some(("block".to_string(), parent_id)) + } else if let Some(hir::map::NodeItem(&hir::Item { + node: hir::ItemFn(_, hir::Unsafety::Unsafe, _, _, _, _), + .. + })) = cx.tcx.hir.find(parent_id) { + Some(("fn".to_string(), parent_id)) + } else { + is_enclosed(cx, parent_id) + } + } else { + None + } + } if let hir::ExprBlock(ref blk) = e.node { // Don't warn about generated blocks, that'll just pollute the output. if blk.rules == hir::UnsafeBlock(hir::UserProvided) && !cx.tcx.used_unsafe.borrow().contains(&blk.id) { - cx.span_lint(UNUSED_UNSAFE, blk.span, "unnecessary `unsafe` block"); + + let mut db = cx.struct_span_lint(UNUSED_UNSAFE, blk.span, + "unnecessary `unsafe` block"); + + db.span_label(blk.span, &"unnecessary `unsafe` block"); + if let Some((kind, id)) = is_enclosed(cx, blk.id) { + db.span_note(cx.tcx.hir.span(id), + &format!("because it's nested under this `unsafe` {}", kind)); + } + db.emit(); } } } diff --git a/src/test/compile-fail/lint-unused-unsafe.rs b/src/test/compile-fail/lint-unused-unsafe.rs deleted file mode 100644 index 5c8e73e6747..00000000000 --- a/src/test/compile-fail/lint-unused-unsafe.rs +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright 2013 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -// Exercise the unused_unsafe attribute in some positive and negative cases - -#![allow(dead_code)] -#![deny(unused_unsafe)] - - -mod foo { - extern { - pub fn bar(); - } -} - -fn callback(_f: F) -> T where F: FnOnce() -> T { panic!() } -unsafe fn unsf() {} - -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 -fn bad6() { - unsafe { // don't put the warning here - unsafe { //~ ERROR: unnecessary `unsafe` block - unsf() - } - } -} -unsafe fn bad7() { - unsafe { //~ ERROR: unnecessary `unsafe` block - unsafe { //~ ERROR: unnecessary `unsafe` block - unsf() - } - } -} - -unsafe fn good0() { unsf() } -fn good1() { unsafe { unsf() } } -fn good2() { - /* bug uncovered when implementing warning about unused unsafe blocks. Be - sure that when purity is inherited that the source of the unsafe-ness - is tracked correctly */ - unsafe { - unsafe fn what() -> Vec { panic!() } - - callback(|| { - what(); - }); - } -} - -unsafe fn good3() { foo::bar() } -fn good4() { unsafe { foo::bar() } } - -#[allow(unused_unsafe)] fn allowed() { unsafe {} } - -fn main() {} diff --git a/src/test/ui/span/lint-unused-unsafe.rs b/src/test/ui/span/lint-unused-unsafe.rs new file mode 100644 index 00000000000..5c8e73e6747 --- /dev/null +++ b/src/test/ui/span/lint-unused-unsafe.rs @@ -0,0 +1,66 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Exercise the unused_unsafe attribute in some positive and negative cases + +#![allow(dead_code)] +#![deny(unused_unsafe)] + + +mod foo { + extern { + pub fn bar(); + } +} + +fn callback(_f: F) -> T where F: FnOnce() -> T { panic!() } +unsafe fn unsf() {} + +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 +fn bad6() { + unsafe { // don't put the warning here + unsafe { //~ ERROR: unnecessary `unsafe` block + unsf() + } + } +} +unsafe fn bad7() { + unsafe { //~ ERROR: unnecessary `unsafe` block + unsafe { //~ ERROR: unnecessary `unsafe` block + unsf() + } + } +} + +unsafe fn good0() { unsf() } +fn good1() { unsafe { unsf() } } +fn good2() { + /* bug uncovered when implementing warning about unused unsafe blocks. Be + sure that when purity is inherited that the source of the unsafe-ness + is tracked correctly */ + unsafe { + unsafe fn what() -> Vec { panic!() } + + callback(|| { + what(); + }); + } +} + +unsafe fn good3() { foo::bar() } +fn good4() { unsafe { foo::bar() } } + +#[allow(unused_unsafe)] fn allowed() { unsafe {} } + +fn main() {} diff --git a/src/test/ui/span/lint-unused-unsafe.stderr b/src/test/ui/span/lint-unused-unsafe.stderr new file mode 100644 index 00000000000..0df3fa43022 --- /dev/null +++ b/src/test/ui/span/lint-unused-unsafe.stderr @@ -0,0 +1,116 @@ +error: unnecessary `unsafe` block + --> $DIR/lint-unused-unsafe.rs:26:13 + | +26 | fn bad1() { unsafe {} } //~ ERROR: unnecessary `unsafe` block + | ^^^^^^^^^ unnecessary `unsafe` block + | +note: lint level defined here + --> $DIR/lint-unused-unsafe.rs:14:9 + | +14 | #![deny(unused_unsafe)] + | ^^^^^^^^^^^^^ + +error: unnecessary `unsafe` block + --> $DIR/lint-unused-unsafe.rs:27:13 + | +27 | fn bad2() { unsafe { bad1() } } //~ ERROR: unnecessary `unsafe` block + | ^^^^^^^^^^^^^^^^^ unnecessary `unsafe` block + +error: unnecessary `unsafe` block + --> $DIR/lint-unused-unsafe.rs:28:20 + | +28 | unsafe fn bad3() { unsafe {} } //~ ERROR: unnecessary `unsafe` block + | ^^^^^^^^^ unnecessary `unsafe` block + | +note: because it's nested under this `unsafe` fn + --> $DIR/lint-unused-unsafe.rs:28:1 + | +28 | unsafe fn bad3() { unsafe {} } //~ ERROR: unnecessary `unsafe` block + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: unnecessary `unsafe` block + --> $DIR/lint-unused-unsafe.rs:29:13 + | +29 | fn bad4() { unsafe { callback(||{}) } } //~ ERROR: unnecessary `unsafe` block + | ^^^^^^^^^^^^^^^^^^^^^^^^^ unnecessary `unsafe` block + +error: unnecessary `unsafe` block + --> $DIR/lint-unused-unsafe.rs:30:20 + | +30 | unsafe fn bad5() { unsafe { unsf() } } //~ ERROR: unnecessary `unsafe` block + | ^^^^^^^^^^^^^^^^^ unnecessary `unsafe` block + | +note: because it's nested under this `unsafe` fn + --> $DIR/lint-unused-unsafe.rs:30:1 + | +30 | unsafe fn bad5() { unsafe { unsf() } } //~ ERROR: unnecessary `unsafe` block + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: unnecessary `unsafe` block + --> $DIR/lint-unused-unsafe.rs:33:9 + | +33 | unsafe { //~ ERROR: unnecessary `unsafe` block + | _________^ starting here... +34 | | unsf() +35 | | } + | |_________^ ...ending here: unnecessary `unsafe` block + | +note: because it's nested under this `unsafe` block + --> $DIR/lint-unused-unsafe.rs:32:5 + | +32 | unsafe { // don't put the warning here + | _____^ starting here... +33 | | unsafe { //~ ERROR: unnecessary `unsafe` block +34 | | unsf() +35 | | } +36 | | } + | |_____^ ...ending here + +error: unnecessary `unsafe` block + --> $DIR/lint-unused-unsafe.rs:39:5 + | +39 | unsafe { //~ ERROR: unnecessary `unsafe` block + | _____^ starting here... +40 | | unsafe { //~ ERROR: unnecessary `unsafe` block +41 | | unsf() +42 | | } +43 | | } + | |_____^ ...ending here: unnecessary `unsafe` block + | +note: because it's nested under this `unsafe` fn + --> $DIR/lint-unused-unsafe.rs:38:1 + | +38 | unsafe fn bad7() { + | _^ starting here... +39 | | unsafe { //~ ERROR: unnecessary `unsafe` block +40 | | unsafe { //~ ERROR: unnecessary `unsafe` block +41 | | unsf() +42 | | } +43 | | } +44 | | } + | |_^ ...ending here + +error: unnecessary `unsafe` block + --> $DIR/lint-unused-unsafe.rs:40:9 + | +40 | unsafe { //~ ERROR: unnecessary `unsafe` block + | _________^ starting here... +41 | | unsf() +42 | | } + | |_________^ ...ending here: unnecessary `unsafe` block + | +note: because it's nested under this `unsafe` fn + --> $DIR/lint-unused-unsafe.rs:38:1 + | +38 | unsafe fn bad7() { + | _^ starting here... +39 | | unsafe { //~ ERROR: unnecessary `unsafe` block +40 | | unsafe { //~ ERROR: unnecessary `unsafe` block +41 | | unsf() +42 | | } +43 | | } +44 | | } + | |_^ ...ending here + +error: aborting due to 8 previous errors + -- cgit 1.4.1-3-g733a5