From 75dc9638a84281262fb316b7581c1015bc7d1eed Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 21 May 2021 14:55:09 -0400 Subject: [PATCH 1/3] Revert portion of PR #83521 that injected issue #85435 (and thus exposed underlying issue #85561). --- .../src/build/expr/as_rvalue.rs | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs index 822fbd91c947e..689b27150e9ed 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs @@ -179,20 +179,26 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // match x { _ => () } // fake read of `x` // }; // ``` - for (thir_place, cause, hir_id) in fake_reads.into_iter() { - let place_builder = unpack!(block = this.as_place_builder(block, thir_place)); - - if let Ok(place_builder_resolved) = - place_builder.try_upvars_resolved(this.tcx, this.typeck_results) - { - let mir_place = - place_builder_resolved.into_place(this.tcx, this.typeck_results); - this.cfg.push_fake_read( - block, - this.source_info(this.tcx.hir().span(*hir_id)), - *cause, - mir_place, - ); + // + // FIXME(RFC2229, rust#85435): Remove feature gate once diagnostics are + // improved and unsafe checking works properly in closure bodies again. + if this.tcx.features().capture_disjoint_fields { + for (thir_place, cause, hir_id) in fake_reads.into_iter() { + let place_builder = + unpack!(block = this.as_place_builder(block, thir_place)); + + if let Ok(place_builder_resolved) = + place_builder.try_upvars_resolved(this.tcx, this.typeck_results) + { + let mir_place = + place_builder_resolved.into_place(this.tcx, this.typeck_results); + this.cfg.push_fake_read( + block, + this.source_info(this.tcx.hir().span(*hir_id)), + *cause, + mir_place, + ); + } } } From 355dc5cae201735da46e1bb7153e2e476255d88c Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 21 May 2021 15:05:53 -0400 Subject: [PATCH 2/3] Regression test for issue 85435. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply suggestions from code review: 1. (removing confusing comment from my test, since the comment reflects the bad undesirable behavior that is being fixed here.) 2. test THIR unsafeck too. Co-authored-by: Léo Lanteri Thauvin --- ...fe-op-in-let-under-unsafe-under-closure.rs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 src/test/ui/unsafe/issue-85435-unsafe-op-in-let-under-unsafe-under-closure.rs diff --git a/src/test/ui/unsafe/issue-85435-unsafe-op-in-let-under-unsafe-under-closure.rs b/src/test/ui/unsafe/issue-85435-unsafe-op-in-let-under-unsafe-under-closure.rs new file mode 100644 index 0000000000000..72f7b67477712 --- /dev/null +++ b/src/test/ui/unsafe/issue-85435-unsafe-op-in-let-under-unsafe-under-closure.rs @@ -0,0 +1,27 @@ +// check-pass +// revisions: mir thir +// [thir]compile-flags: -Z thir-unsafeck + +// This is issue #85435. But the real story is reflected in issue #85561, where +// a bug in the implementation of feature(capture_disjoint_fields) () was +// exposed to non-feature-gated code by a diagnostic changing PR that removed +// the gating in one case. + +// This test is double-checking that the case of interest continues to work as +// expected in the *absence* of that feature gate. At the time of this writing, +// enabling the feature gate will cause this test to fail. We obviously cannot +// stabilize that feature until it can correctly handle this test. + +fn main() { + let val: u8 = 5; + let u8_ptr: *const u8 = &val; + let _closure = || { + unsafe { + let tmp = *u8_ptr; + tmp + + // Just dereferencing and returning directly compiles fine: + // *u8_ptr + } + }; +} From f9f5fc89267ab9ccc67a35d56d69e370d0852328 Mon Sep 17 00:00:00 2001 From: Felix S Klock II Date: Tue, 1 Jun 2021 14:43:16 -0400 Subject: [PATCH 3/3] no thir-unsafeck in beta remove testing via `-Z thir-unsafeck`, since it is not appropriate for beta branch. --- .../issue-85435-unsafe-op-in-let-under-unsafe-under-closure.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/ui/unsafe/issue-85435-unsafe-op-in-let-under-unsafe-under-closure.rs b/src/test/ui/unsafe/issue-85435-unsafe-op-in-let-under-unsafe-under-closure.rs index 72f7b67477712..b0d738855d731 100644 --- a/src/test/ui/unsafe/issue-85435-unsafe-op-in-let-under-unsafe-under-closure.rs +++ b/src/test/ui/unsafe/issue-85435-unsafe-op-in-let-under-unsafe-under-closure.rs @@ -1,6 +1,4 @@ // check-pass -// revisions: mir thir -// [thir]compile-flags: -Z thir-unsafeck // This is issue #85435. But the real story is reflected in issue #85561, where // a bug in the implementation of feature(capture_disjoint_fields) () was