Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Point to enclosing block/fn on nested unsafe #39202

Merged
merged 1 commit into from
Mar 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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