Skip to content

Fix unused_unsafe label with unsafe_block_in_unsafe_fn #81110

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 19, 2021
Merged

Fix unused_unsafe label with unsafe_block_in_unsafe_fn #81110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 9 additions & 8 deletions compiler/rustc_mir/src/transform/check_unsafety.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,24 +580,23 @@ fn is_enclosed(
tcx: TyCtxt<'_>,
used_unsafe: &FxHashSet<hir::HirId>,
id: hir::HirId,
) -> Option<(String, hir::HirId)> {
unsafe_op_in_unsafe_fn_allowed: bool,
) -> Option<(&'static str, hir::HirId)> {
let parent_id = tcx.hir().get_parent_node(id);
if parent_id != id {
if used_unsafe.contains(&parent_id) {
Some(("block".to_string(), parent_id))
Some(("block", parent_id))
} else if let Some(Node::Item(&hir::Item {
kind: hir::ItemKind::Fn(ref sig, _, _), ..
})) = tcx.hir().find(parent_id)
{
if sig.header.unsafety == hir::Unsafety::Unsafe
&& !tcx.features().unsafe_block_in_unsafe_fn
{
Some(("fn".to_string(), parent_id))
if sig.header.unsafety == hir::Unsafety::Unsafe && unsafe_op_in_unsafe_fn_allowed {
Some(("fn", parent_id))
} else {
None
}
} else {
is_enclosed(tcx, used_unsafe, parent_id)
is_enclosed(tcx, used_unsafe, parent_id, unsafe_op_in_unsafe_fn_allowed)
}
} else {
None
Expand All @@ -610,7 +609,9 @@ fn report_unused_unsafe(tcx: TyCtxt<'_>, used_unsafe: &FxHashSet<hir::HirId>, id
let msg = "unnecessary `unsafe` block";
let mut db = lint.build(msg);
db.span_label(span, msg);
if let Some((kind, id)) = is_enclosed(tcx, used_unsafe, id) {
if let Some((kind, id)) =
is_enclosed(tcx, used_unsafe, id, unsafe_op_in_unsafe_fn_allowed(tcx, id))
{
db.span_label(
tcx.sess.source_map().guess_head_span(tcx.hir().span(id)),
format!("because it's nested under this `unsafe` {}", kind),
Expand Down
5 changes: 5 additions & 0 deletions src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ unsafe fn deny_level() {
//~^ ERROR dereference of raw pointer is unsafe and requires unsafe block
VOID = ();
//~^ ERROR use of mutable static is unsafe and requires unsafe block

unsafe {}
//~^ ERROR unnecessary `unsafe` block
}

// Check that `unsafe_op_in_unsafe_fn` works starting from the `warn` level.
Expand All @@ -25,6 +28,8 @@ unsafe fn warning_level() {
//~^ ERROR dereference of raw pointer is unsafe and requires unsafe block
VOID = ();
//~^ ERROR use of mutable static is unsafe and requires unsafe block
unsafe {}
//~^ ERROR unnecessary `unsafe` block
}

unsafe fn explicit_block() {
Expand Down
50 changes: 34 additions & 16 deletions src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,78 +27,96 @@ LL | VOID = ();
|
= note: mutable statics can be mutated by multiple threads: aliasing violations or data races will cause undefined behavior

error: unnecessary `unsafe` block
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:17:5
|
LL | unsafe {}
| ^^^^^^ unnecessary `unsafe` block
|
note: the lint level is defined here
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:3:9
|
LL | #![deny(unused_unsafe)]
| ^^^^^^^^^^^^^

error: call to unsafe function is unsafe and requires unsafe block (error E0133)
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:22:5
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:25:5
|
LL | unsf();
| ^^^^^^ call to unsafe function
|
note: the lint level is defined here
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:20:8
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:23:8
|
LL | #[deny(warnings)]
| ^^^^^^^^
= note: `#[deny(unsafe_op_in_unsafe_fn)]` implied by `#[deny(warnings)]`
= note: consult the function's documentation for information on how to avoid undefined behavior

error: dereference of raw pointer is unsafe and requires unsafe block (error E0133)
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:24:5
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:27:5
|
LL | *PTR;
| ^^^^ dereference of raw pointer
|
= note: raw pointers may be NULL, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior

error: use of mutable static is unsafe and requires unsafe block (error E0133)
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:26:5
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:29:5
|
LL | VOID = ();
| ^^^^^^^^^ use of mutable static
|
= note: mutable statics can be mutated by multiple threads: aliasing violations or data races will cause undefined behavior

error: unnecessary `unsafe` block
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:40:14
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:31:5
|
LL | unsafe {}
| ^^^^^^ unnecessary `unsafe` block

error: unnecessary `unsafe` block
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:45:14
|
LL | unsafe { unsafe { unsf() } }
| ------ ^^^^^^ unnecessary `unsafe` block
| |
| because it's nested under this `unsafe` block
|
note: the lint level is defined here
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:3:9
|
LL | #![deny(unused_unsafe)]
| ^^^^^^^^^^^^^

error: unnecessary `unsafe` block
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:51:5
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:56:5
|
LL | unsafe fn allow_level() {
| ----------------------- because it's nested under this `unsafe` fn
...
LL | unsafe { unsf() }
| ^^^^^^ unnecessary `unsafe` block
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, this error is kind of confusing because the "because" comes before the thing it is justifying. I guess there's also the error 'title' thing "unnecessary unsafe block", so it's probably okay, but just reading the spans things are in the wrong order... not sure how we usually handle situations where code order and "justification order" disagree.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how the warning is emitted today (without unsafe_block_in_unsafe_fn). I guess any improvement here should be made in another PR


error: unnecessary `unsafe` block
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:63:9
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:68:9
|
LL | unsafe fn nested_allow_level() {
| ------------------------------ because it's nested under this `unsafe` fn
...
LL | unsafe { unsf() }
| ^^^^^^ unnecessary `unsafe` block

error[E0133]: call to unsafe function is unsafe and requires unsafe block
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:69:5
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:74:5
|
LL | unsf();
| ^^^^^^ call to unsafe function
|
= note: consult the function's documentation for information on how to avoid undefined behavior

error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:73:9
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:78:9
|
LL | unsf();
| ^^^^^^ call to unsafe function
|
= note: consult the function's documentation for information on how to avoid undefined behavior

error: aborting due to 11 previous errors
error: aborting due to 13 previous errors

For more information about this error, try `rustc --explain E0133`.