From 6c4f40dee18c2166dc23d9077182ee6a27496da5 Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Sun, 23 May 2021 18:10:32 +0200 Subject: [PATCH 1/2] Make closures inherit their parent's "safety context" --- .../rustc_mir_build/src/check_unsafety.rs | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index 971b6dd9e1c89..d1aaabe92edae 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -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; + } _ => {} } @@ -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) { // 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. From d1f0e9f65ce537f8026fdc207a596909c1fe4ed8 Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Thu, 27 May 2021 20:37:49 +0200 Subject: [PATCH 2/2] Test THIR unsafeck for unsafe ops in closures --- src/test/ui/async-await/async-await.rs | 3 +- src/test/ui/command/command-pre-exec.rs | 2 + .../static-mut-reference-across-yield.rs | 3 + .../intrinsics/panic-uninitialized-zeroed.rs | 2 + src/test/ui/issues/issue-11740.rs | 2 + src/test/ui/issues/issue-39367.rs | 3 + ...-unnecessary-unsafe-in-closure.mir.stderr} | 8 +-- ...sue-45107-unnecessary-unsafe-in-closure.rs | 3 + ...-unnecessary-unsafe-in-closure.thir.stderr | 35 ++++++++++ src/test/ui/lto-still-runs-thread-dtors.rs | 2 + src/test/ui/no-stdio.rs | 2 + src/test/ui/running-with-no-runtime.rs | 2 + .../simd/simd-intrinsic-generic-comparison.rs | 2 + ...e.stderr => lint-unused-unsafe.mir.stderr} | 18 ++--- src/test/ui/span/lint-unused-unsafe.rs | 3 + .../ui/span/lint-unused-unsafe.thir.stderr | 66 +++++++++++++++++++ src/tools/tidy/src/ui_tests.rs | 2 +- 17 files changed, 143 insertions(+), 15 deletions(-) rename src/test/ui/issues/{issue-45107-unnecessary-unsafe-in-closure.stderr => issue-45107-unnecessary-unsafe-in-closure.mir.stderr} (78%) create mode 100644 src/test/ui/issues/issue-45107-unnecessary-unsafe-in-closure.thir.stderr rename src/test/ui/span/{lint-unused-unsafe.stderr => lint-unused-unsafe.mir.stderr} (83%) create mode 100644 src/test/ui/span/lint-unused-unsafe.thir.stderr diff --git a/src/test/ui/async-await/async-await.rs b/src/test/ui/async-await/async-await.rs index 0207752afe098..3d22025bf286a 100644 --- a/src/test/ui/async-await/async-await.rs +++ b/src/test/ui/async-await/async-await.rs @@ -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)] diff --git a/src/test/ui/command/command-pre-exec.rs b/src/test/ui/command/command-pre-exec.rs index 819ed0b2ddef7..61914e2293070 100644 --- a/src/test/ui/command/command-pre-exec.rs +++ b/src/test/ui/command/command-pre-exec.rs @@ -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 diff --git a/src/test/ui/generator/static-mut-reference-across-yield.rs b/src/test/ui/generator/static-mut-reference-across-yield.rs index 2926bba997803..0fa6d9cdc77b6 100644 --- a/src/test/ui/generator/static-mut-reference-across-yield.rs +++ b/src/test/ui/generator/static-mut-reference-across-yield.rs @@ -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]; diff --git a/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs b/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs index 4a91198ab9f6f..72c0d7913e525 100644 --- a/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs +++ b/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs @@ -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}`. diff --git a/src/test/ui/issues/issue-11740.rs b/src/test/ui/issues/issue-11740.rs index dc10a205f2418..9faeb7770a75a 100644 --- a/src/test/ui/issues/issue-11740.rs +++ b/src/test/ui/issues/issue-11740.rs @@ -1,4 +1,6 @@ // check-pass +// revisions: mir thir +// [thir]compile-flags: -Zthir-unsafeck struct Attr { name: String, diff --git a/src/test/ui/issues/issue-39367.rs b/src/test/ui/issues/issue-39367.rs index 8314be3d14cc5..e7beb8a0392e8 100644 --- a/src/test/ui/issues/issue-39367.rs +++ b/src/test/ui/issues/issue-39367.rs @@ -1,4 +1,7 @@ // run-pass +// revisions: mir thir +// [thir]compile-flags: -Zthir-unsafeck + use std::ops::Deref; struct ArenaSet::Target>(U, &'static V) diff --git a/src/test/ui/issues/issue-45107-unnecessary-unsafe-in-closure.stderr b/src/test/ui/issues/issue-45107-unnecessary-unsafe-in-closure.mir.stderr similarity index 78% rename from src/test/ui/issues/issue-45107-unnecessary-unsafe-in-closure.stderr rename to src/test/ui/issues/issue-45107-unnecessary-unsafe-in-closure.mir.stderr index 321698e763696..9e9cbcf33ae17 100644 --- a/src/test/ui/issues/issue-45107-unnecessary-unsafe-in-closure.stderr +++ b/src/test/ui/issues/issue-45107-unnecessary-unsafe-in-closure.mir.stderr @@ -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 @@ -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 @@ -23,7 +23,7 @@ LL | |w: &mut Vec| { 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 diff --git a/src/test/ui/issues/issue-45107-unnecessary-unsafe-in-closure.rs b/src/test/ui/issues/issue-45107-unnecessary-unsafe-in-closure.rs index de275ff701a61..ac1cfd62a0568 100644 --- a/src/test/ui/issues/issue-45107-unnecessary-unsafe-in-closure.rs +++ b/src/test/ui/issues/issue-45107-unnecessary-unsafe-in-closure.rs @@ -1,3 +1,6 @@ +// revisions: mir thir +// [thir]compile-flags: -Zthir-unsafeck + #[deny(unused_unsafe)] fn main() { let mut v = Vec::::with_capacity(24); diff --git a/src/test/ui/issues/issue-45107-unnecessary-unsafe-in-closure.thir.stderr b/src/test/ui/issues/issue-45107-unnecessary-unsafe-in-closure.thir.stderr new file mode 100644 index 0000000000000..9e9cbcf33ae17 --- /dev/null +++ b/src/test/ui/issues/issue-45107-unnecessary-unsafe-in-closure.thir.stderr @@ -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| { 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| { unsafe { + | ^^^^^^ unnecessary `unsafe` block + +error: aborting due to 3 previous errors + diff --git a/src/test/ui/lto-still-runs-thread-dtors.rs b/src/test/ui/lto-still-runs-thread-dtors.rs index 635ad783b3155..1c7368b36e135 100644 --- a/src/test/ui/lto-still-runs-thread-dtors.rs +++ b/src/test/ui/lto-still-runs-thread-dtors.rs @@ -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; diff --git a/src/test/ui/no-stdio.rs b/src/test/ui/no-stdio.rs index 68e6fa838b4e2..24985386a97a1 100644 --- a/src/test/ui/no-stdio.rs +++ b/src/test/ui/no-stdio.rs @@ -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)] diff --git a/src/test/ui/running-with-no-runtime.rs b/src/test/ui/running-with-no-runtime.rs index c321e86dc1821..c575a6bec8e92 100644 --- a/src/test/ui/running-with-no-runtime.rs +++ b/src/test/ui/running-with-no-runtime.rs @@ -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)] diff --git a/src/test/ui/simd/simd-intrinsic-generic-comparison.rs b/src/test/ui/simd/simd-intrinsic-generic-comparison.rs index 103132c18ae2f..da5c42a1a9888 100644 --- a/src/test/ui/simd/simd-intrinsic-generic-comparison.rs +++ b/src/test/ui/simd/simd-intrinsic-generic-comparison.rs @@ -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)] diff --git a/src/test/ui/span/lint-unused-unsafe.stderr b/src/test/ui/span/lint-unused-unsafe.mir.stderr similarity index 83% rename from src/test/ui/span/lint-unused-unsafe.stderr rename to src/test/ui/span/lint-unused-unsafe.mir.stderr index c35a3349121b7..c2adb7be7a220 100644 --- a/src/test/ui/span/lint-unused-unsafe.stderr +++ b/src/test/ui/span/lint-unused-unsafe.mir.stderr @@ -1,23 +1,23 @@ 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 @@ -25,13 +25,13 @@ LL | unsafe fn bad3() { unsafe {} } | 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 @@ -39,7 +39,7 @@ LL | unsafe fn bad5() { unsafe { unsf() } } | 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 @@ -47,7 +47,7 @@ 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 @@ -55,7 +55,7 @@ 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 diff --git a/src/test/ui/span/lint-unused-unsafe.rs b/src/test/ui/span/lint-unused-unsafe.rs index b6c4894d91807..b889cc981cad1 100644 --- a/src/test/ui/span/lint-unused-unsafe.rs +++ b/src/test/ui/span/lint-unused-unsafe.rs @@ -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)] diff --git a/src/test/ui/span/lint-unused-unsafe.thir.stderr b/src/test/ui/span/lint-unused-unsafe.thir.stderr new file mode 100644 index 0000000000000..dda45c3679ab6 --- /dev/null +++ b/src/test/ui/span/lint-unused-unsafe.thir.stderr @@ -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 + diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index 3f98388446003..f61295c88308c 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -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"))