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

Make closures inherit their parent's "safety context" #85607

Merged
merged 2 commits into from
May 28, 2021
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
32 changes: 30 additions & 2 deletions compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,30 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
self.requires_unsafe(expr.span, CastOfPointerToInt);
}
}
ExprKind::Closure {
closure_id,
substs: _,
upvars: _,
movability: _,
fake_reads: _,
} => {
let closure_id = closure_id.expect_local();
let closure_def = if let Some((did, const_param_id)) =
ty::WithOptConstParam::try_lookup(closure_id, self.tcx)
{
ty::WithOptConstParam { did, const_param_did: Some(const_param_id) }
} else {
ty::WithOptConstParam::unknown(closure_id)
};
let (closure_thir, expr) = self.tcx.thir_body(closure_def);
let closure_thir = &closure_thir.borrow();
let hir_context = self.tcx.hir().local_def_id_to_hir_id(closure_id);
let mut closure_visitor =
UnsafetyVisitor { thir: closure_thir, hir_context, ..*self };
closure_visitor.visit_expr(&closure_thir[expr]);
// Unsafe blocks can be used in closures, make sure to take it into account
self.safety_context = closure_visitor.safety_context;
}
_ => {}
}

Expand Down Expand Up @@ -335,14 +359,18 @@ impl UnsafeOpKind {
}
}

// FIXME: checking unsafety for closures should be handled by their parent body,
// as they inherit their "safety context" from their declaration site.
pub fn check_unsafety<'tcx>(tcx: TyCtxt<'tcx>, def: ty::WithOptConstParam<LocalDefId>) {
// THIR unsafeck is gated under `-Z thir-unsafeck`
if !tcx.sess.opts.debugging_opts.thir_unsafeck {
return;
}

// Closures are handled by their parent function
if tcx.is_closure(def.did.to_def_id()) {
tcx.ensure().thir_check_unsafety(tcx.hir().local_def_id_to_hir_id(def.did).owner);
return;
}

let (thir, expr) = tcx.thir_body(def);
let thir = &thir.borrow();
// If `thir` is empty, a type error occured, skip this body.
Expand Down
3 changes: 2 additions & 1 deletion src/test/ui/async-await/async-await.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// run-pass

// revisions: default nomiropt
// revisions: default nomiropt thirunsafeck
//[nomiropt]compile-flags: -Z mir-opt-level=0
//[thirunsafeck]compile-flags: -Zthir-unsafeck

#![allow(unused)]

Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/command/command-pre-exec.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// run-pass
// revisions: mir thir
// [thir]compile-flags: -Zthir-unsafeck

#![allow(stable_features)]
// ignore-windows - this is a unix-specific test
Expand Down
3 changes: 3 additions & 0 deletions src/test/ui/generator/static-mut-reference-across-yield.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
// build-pass
// revisions: mir thir
// [thir]compile-flags: -Zthir-unsafeck

#![feature(generators)]

static mut A: [i32; 5] = [1, 2, 3, 4, 5];
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/intrinsics/panic-uninitialized-zeroed.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// run-pass
// ignore-wasm32-bare compiled with panic=abort by default
// revisions: mir thir
// [thir]compile-flags: -Zthir-unsafeck

// This test checks panic emitted from `mem::{uninitialized,zeroed}`.

Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/issues/issue-11740.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// check-pass
// revisions: mir thir
// [thir]compile-flags: -Zthir-unsafeck

struct Attr {
name: String,
Expand Down
3 changes: 3 additions & 0 deletions src/test/ui/issues/issue-39367.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
// run-pass
// revisions: mir thir
// [thir]compile-flags: -Zthir-unsafeck

use std::ops::Deref;

struct ArenaSet<U: Deref, V=<U as Deref>::Target>(U, &'static V)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: unnecessary `unsafe` block
--> $DIR/issue-45107-unnecessary-unsafe-in-closure.rs:7:13
--> $DIR/issue-45107-unnecessary-unsafe-in-closure.rs:10:13
|
LL | unsafe {
| ------ because it's nested under this `unsafe` block
Expand All @@ -8,13 +8,13 @@ LL | unsafe {
| ^^^^^^ unnecessary `unsafe` block
|
note: the lint level is defined here
--> $DIR/issue-45107-unnecessary-unsafe-in-closure.rs:1:8
--> $DIR/issue-45107-unnecessary-unsafe-in-closure.rs:4:8
|
LL | #[deny(unused_unsafe)]
| ^^^^^^^^^^^^^

error: unnecessary `unsafe` block
--> $DIR/issue-45107-unnecessary-unsafe-in-closure.rs:9:38
--> $DIR/issue-45107-unnecessary-unsafe-in-closure.rs:12:38
|
LL | unsafe {
| ------ because it's nested under this `unsafe` block
Expand All @@ -23,7 +23,7 @@ LL | |w: &mut Vec<u32>| { unsafe {
| ^^^^^^ unnecessary `unsafe` block

error: unnecessary `unsafe` block
--> $DIR/issue-45107-unnecessary-unsafe-in-closure.rs:13:34
--> $DIR/issue-45107-unnecessary-unsafe-in-closure.rs:16:34
|
LL | unsafe {
| ------ because it's nested under this `unsafe` block
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// revisions: mir thir
// [thir]compile-flags: -Zthir-unsafeck

#[deny(unused_unsafe)]
fn main() {
let mut v = Vec::<i32>::with_capacity(24);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
error: unnecessary `unsafe` block
--> $DIR/issue-45107-unnecessary-unsafe-in-closure.rs:10:13
|
LL | unsafe {
| ------ because it's nested under this `unsafe` block
LL | let f = |v: &mut Vec<_>| {
LL | unsafe {
| ^^^^^^ unnecessary `unsafe` block
|
note: the lint level is defined here
--> $DIR/issue-45107-unnecessary-unsafe-in-closure.rs:4:8
|
LL | #[deny(unused_unsafe)]
| ^^^^^^^^^^^^^

error: unnecessary `unsafe` block
--> $DIR/issue-45107-unnecessary-unsafe-in-closure.rs:12:38
|
LL | unsafe {
| ------ because it's nested under this `unsafe` block
...
LL | |w: &mut Vec<u32>| { unsafe {
| ^^^^^^ unnecessary `unsafe` block

error: unnecessary `unsafe` block
--> $DIR/issue-45107-unnecessary-unsafe-in-closure.rs:16:34
|
LL | unsafe {
| ------ because it's nested under this `unsafe` block
...
LL | |x: &mut Vec<u32>| { unsafe {
| ^^^^^^ unnecessary `unsafe` block

error: aborting due to 3 previous errors

2 changes: 2 additions & 0 deletions src/test/ui/lto-still-runs-thread-dtors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// compile-flags: -C lto
// no-prefer-dynamic
// ignore-emscripten no threads support
// revisions: mir thir
// [thir]compile-flags: -Zthir-unsafeck

use std::thread;

Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/no-stdio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// ignore-android
// ignore-emscripten no processes
// ignore-sgx no processes
// revisions: mir thir
// [thir]compile-flags: -Zthir-unsafeck

#![feature(rustc_private)]

Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/running-with-no-runtime.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// run-pass
// ignore-emscripten spawning processes is not supported
// ignore-sgx no processes
// revisions: mir thir
// [thir]compile-flags: -Zthir-unsafeck

#![feature(start)]

Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/simd/simd-intrinsic-generic-comparison.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// run-pass
// ignore-emscripten FIXME(#45351) hits an LLVM assert
// revisions: mir thir
// [thir]compile-flags: -Zthir-unsafeck

#![feature(repr_simd, platform_intrinsics, concat_idents)]
#![allow(non_camel_case_types)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,61 +1,61 @@
error: unnecessary `unsafe` block
--> $DIR/lint-unused-unsafe.rs:16:13
--> $DIR/lint-unused-unsafe.rs:19:13
|
LL | fn bad1() { unsafe {} }
| ^^^^^^ unnecessary `unsafe` block
|
note: the lint level is defined here
--> $DIR/lint-unused-unsafe.rs:4:9
--> $DIR/lint-unused-unsafe.rs:7:9
|
LL | #![deny(unused_unsafe)]
| ^^^^^^^^^^^^^

error: unnecessary `unsafe` block
--> $DIR/lint-unused-unsafe.rs:17:13
--> $DIR/lint-unused-unsafe.rs:20:13
|
LL | fn bad2() { unsafe { bad1() } }
| ^^^^^^ unnecessary `unsafe` block

error: unnecessary `unsafe` block
--> $DIR/lint-unused-unsafe.rs:18:20
--> $DIR/lint-unused-unsafe.rs:21:20
|
LL | unsafe fn bad3() { unsafe {} }
| ---------------- ^^^^^^ unnecessary `unsafe` block
| |
| because it's nested under this `unsafe` fn

error: unnecessary `unsafe` block
--> $DIR/lint-unused-unsafe.rs:19:13
--> $DIR/lint-unused-unsafe.rs:22:13
|
LL | fn bad4() { unsafe { callback(||{}) } }
| ^^^^^^ unnecessary `unsafe` block

error: unnecessary `unsafe` block
--> $DIR/lint-unused-unsafe.rs:20:20
--> $DIR/lint-unused-unsafe.rs:23:20
|
LL | unsafe fn bad5() { unsafe { unsf() } }
| ---------------- ^^^^^^ unnecessary `unsafe` block
| |
| because it's nested under this `unsafe` fn

error: unnecessary `unsafe` block
--> $DIR/lint-unused-unsafe.rs:23:9
--> $DIR/lint-unused-unsafe.rs:26:9
|
LL | unsafe { // don't put the warning here
| ------ because it's nested under this `unsafe` block
LL | unsafe {
| ^^^^^^ unnecessary `unsafe` block

error: unnecessary `unsafe` block
--> $DIR/lint-unused-unsafe.rs:29:5
--> $DIR/lint-unused-unsafe.rs:32:5
|
LL | unsafe fn bad7() {
| ---------------- because it's nested under this `unsafe` fn
LL | unsafe {
| ^^^^^^ unnecessary `unsafe` block

error: unnecessary `unsafe` block
--> $DIR/lint-unused-unsafe.rs:30:9
--> $DIR/lint-unused-unsafe.rs:33:9
|
LL | unsafe fn bad7() {
| ---------------- because it's nested under this `unsafe` fn
Expand Down
3 changes: 3 additions & 0 deletions src/test/ui/span/lint-unused-unsafe.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// Exercise the unused_unsafe attribute in some positive and negative cases

// revisions: mir thir
// [thir]compile-flags: -Zthir-unsafeck

#![allow(dead_code)]
#![deny(unused_unsafe)]

Expand Down
66 changes: 66 additions & 0 deletions src/test/ui/span/lint-unused-unsafe.thir.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
error: unnecessary `unsafe` block
--> $DIR/lint-unused-unsafe.rs:19:13
|
LL | fn bad1() { unsafe {} }
| ^^^^^^ unnecessary `unsafe` block
|
note: the lint level is defined here
--> $DIR/lint-unused-unsafe.rs:7:9
|
LL | #![deny(unused_unsafe)]
| ^^^^^^^^^^^^^

error: unnecessary `unsafe` block
--> $DIR/lint-unused-unsafe.rs:20:13
|
LL | fn bad2() { unsafe { bad1() } }
| ^^^^^^ unnecessary `unsafe` block

error: unnecessary `unsafe` block
--> $DIR/lint-unused-unsafe.rs:21:20
|
LL | unsafe fn bad3() { unsafe {} }
| ---------------- ^^^^^^ unnecessary `unsafe` block
| |
| because it's nested under this `unsafe` fn

error: unnecessary `unsafe` block
--> $DIR/lint-unused-unsafe.rs:22:13
|
LL | fn bad4() { unsafe { callback(||{}) } }
| ^^^^^^ unnecessary `unsafe` block

error: unnecessary `unsafe` block
--> $DIR/lint-unused-unsafe.rs:23:20
|
LL | unsafe fn bad5() { unsafe { unsf() } }
| ---------------- ^^^^^^ unnecessary `unsafe` block
| |
| because it's nested under this `unsafe` fn

error: unnecessary `unsafe` block
--> $DIR/lint-unused-unsafe.rs:26:9
|
LL | unsafe { // don't put the warning here
| ------ because it's nested under this `unsafe` block
LL | unsafe {
| ^^^^^^ unnecessary `unsafe` block

error: unnecessary `unsafe` block
--> $DIR/lint-unused-unsafe.rs:33:9
|
LL | unsafe {
| ------ because it's nested under this `unsafe` block
LL | unsafe {
| ^^^^^^ unnecessary `unsafe` block

error: unnecessary `unsafe` block
--> $DIR/lint-unused-unsafe.rs:32:5
|
LL | unsafe fn bad7() {
| ---------------- because it's nested under this `unsafe` fn
LL | unsafe {
| ^^^^^^ unnecessary `unsafe` block

error: aborting due to 8 previous errors

2 changes: 1 addition & 1 deletion src/tools/tidy/src/ui_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::path::Path;
const ENTRY_LIMIT: usize = 1000;
// FIXME: The following limits should be reduced eventually.
const ROOT_ENTRY_LIMIT: usize = 1371;
const ISSUES_ENTRY_LIMIT: usize = 2558;
const ISSUES_ENTRY_LIMIT: usize = 2559;

fn check_entries(path: &Path, bad: &mut bool) {
let dirs = walkdir::WalkDir::new(&path.join("test/ui"))
Expand Down