From ac2bc7c570eec8a024cec779908c8ae718924e77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 19 Jan 2017 23:17:48 -0800 Subject: [PATCH] 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 ++++- .../span}/lint-unused-unsafe.rs | 0 src/test/ui/span/lint-unused-unsafe.stderr | 116 ++++++++++++++++++ 3 files changed, 144 insertions(+), 1 deletion(-) rename src/test/{compile-fail => ui/span}/lint-unused-unsafe.rs (100%) create mode 100644 src/test/ui/span/lint-unused-unsafe.stderr 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/ui/span/lint-unused-unsafe.rs similarity index 100% rename from src/test/compile-fail/lint-unused-unsafe.rs rename to src/test/ui/span/lint-unused-unsafe.rs 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 +