Skip to content

Commit

Permalink
Rollup merge of rust-lang#39202 - estebank:nested-unsafe, r=jonathand…
Browse files Browse the repository at this point in the history
…turner

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.

<img width="621" alt="" src="https://cloud.githubusercontent.com/assets/1606434/22139922/26ad468a-de9e-11e6-8884-2945be882ea8.png">

Fixes rust-lang#39144.
  • Loading branch information
Ariel Ben-Yehuda authored Mar 10, 2017
2 parents f573db4 + ac2bc7c commit 6de7215
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 1 deletion.
29 changes: 28 additions & 1 deletion src/librustc_lint/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
}
Expand Down
File renamed without changes.
116 changes: 116 additions & 0 deletions src/test/ui/span/lint-unused-unsafe.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 6de7215

Please sign in to comment.