From a671127941921bc11be4d99106fea5cba398b383 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Mon, 28 Aug 2023 15:42:23 +0200
Subject: [PATCH] closure field capturing: don't depend on alignment of packed
fields
---
compiler/rustc_hir_typeck/src/upvar.rs | 46 ++++---------------
.../2229_closure_analysis/repr_packed.rs | 10 ++--
.../2229_closure_analysis/repr_packed.stderr | 19 +++-----
3 files changed, 22 insertions(+), 53 deletions(-)
diff --git a/compiler/rustc_hir_typeck/src/upvar.rs b/compiler/rustc_hir_typeck/src/upvar.rs
index 1a41786d251..ba469bd029d 100644
--- a/compiler/rustc_hir_typeck/src/upvar.rs
+++ b/compiler/rustc_hir_typeck/src/upvar.rs
@@ -195,7 +195,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
assert_eq!(self.tcx.hir().body_owner_def_id(body.id()), closure_def_id);
let mut delegate = InferBorrowKind {
- fcx: self,
closure_def_id,
capture_information: Default::default(),
fake_reads: Default::default(),
@@ -1607,34 +1606,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// Truncate the capture so that the place being borrowed is in accordance with RFC 1240,
/// which states that it's unsafe to take a reference into a struct marked `repr(packed)`.
fn restrict_repr_packed_field_ref_capture<'tcx>(
- tcx: TyCtxt<'tcx>,
- param_env: ty::ParamEnv<'tcx>,
mut place: Place<'tcx>,
mut curr_borrow_kind: ty::UpvarCapture,
) -> (Place<'tcx>, ty::UpvarCapture) {
let pos = place.projections.iter().enumerate().position(|(i, p)| {
let ty = place.ty_before_projection(i);
- // Return true for fields of packed structs, unless those fields have alignment 1.
+ // Return true for fields of packed structs.
match p.kind {
ProjectionKind::Field(..) => match ty.kind() {
ty::Adt(def, _) if def.repr().packed() => {
- // We erase regions here because they cannot be hashed
- match tcx.layout_of(param_env.and(tcx.erase_regions(p.ty))) {
- Ok(layout) if layout.align.abi.bytes() == 1 => {
- // if the alignment is 1, the type can't be further
- // disaligned.
- debug!(
- "restrict_repr_packed_field_ref_capture: ({:?}) - align = 1",
- place
- );
- false
- }
- _ => {
- debug!("restrict_repr_packed_field_ref_capture: ({:?}) - true", place);
- true
- }
- }
+ // We stop here regardless of field alignment. Field alignment can change as
+ // types change, including the types of private fields in other crates, and that
+ // shouldn't affect how we compute our captures.
+ true
}
_ => false,
@@ -1689,9 +1674,7 @@ fn drop_location_span(tcx: TyCtxt<'_>, hir_id: hir::HirId) -> Span {
tcx.sess.source_map().end_point(owner_span)
}
-struct InferBorrowKind<'a, 'tcx> {
- fcx: &'a FnCtxt<'a, 'tcx>,
-
+struct InferBorrowKind<'tcx> {
// The def-id of the closure whose kind and upvar accesses are being inferred.
closure_def_id: LocalDefId,
@@ -1725,7 +1708,7 @@ struct InferBorrowKind<'a, 'tcx> {
fake_reads: Vec<(Place<'tcx>, FakeReadCause, hir::HirId)>,
}
-impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
+impl<'tcx> euv::Delegate<'tcx> for InferBorrowKind<'tcx> {
fn fake_read(
&mut self,
place: &PlaceWithHirId<'tcx>,
@@ -1740,12 +1723,7 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
let (place, _) = restrict_capture_precision(place.place.clone(), dummy_capture_kind);
- let (place, _) = restrict_repr_packed_field_ref_capture(
- self.fcx.tcx,
- self.fcx.param_env,
- place,
- dummy_capture_kind,
- );
+ let (place, _) = restrict_repr_packed_field_ref_capture(place, dummy_capture_kind);
self.fake_reads.push((place, cause, diag_expr_id));
}
@@ -1780,12 +1758,8 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
// We only want repr packed restriction to be applied to reading references into a packed
// struct, and not when the data is being moved. Therefore we call this method here instead
// of in `restrict_capture_precision`.
- let (place, mut capture_kind) = restrict_repr_packed_field_ref_capture(
- self.fcx.tcx,
- self.fcx.param_env,
- place_with_id.place.clone(),
- capture_kind,
- );
+ let (place, mut capture_kind) =
+ restrict_repr_packed_field_ref_capture(place_with_id.place.clone(), capture_kind);
// Raw pointers don't inherit mutability
if place_with_id.place.deref_tys().any(Ty::is_unsafe_ptr) {
diff --git a/tests/ui/closures/2229_closure_analysis/repr_packed.rs b/tests/ui/closures/2229_closure_analysis/repr_packed.rs
index f23670f63ac..8c23454fae9 100644
--- a/tests/ui/closures/2229_closure_analysis/repr_packed.rs
+++ b/tests/ui/closures/2229_closure_analysis/repr_packed.rs
@@ -3,7 +3,8 @@
#![feature(rustc_attrs)]
// `u8` aligned at a byte and are unaffected by repr(packed).
-// Therefore we can precisely (and safely) capture references to both the fields.
+// Therefore we *could* precisely (and safely) capture references to both the fields,
+// but we don't, since we don't want capturing to change when field types change alignment.
fn test_alignment_not_affected() {
#[repr(packed)]
struct Foo { x: u8, y: u8 }
@@ -17,11 +18,10 @@ fn test_alignment_not_affected() {
//~^ ERROR: First Pass analysis includes:
//~| ERROR: Min Capture analysis includes:
let z1: &u8 = &foo.x;
- //~^ NOTE: Capturing foo[(0, 0)] -> ImmBorrow
- //~| NOTE: Min Capture foo[(0, 0)] -> ImmBorrow
+ //~^ NOTE: Capturing foo[] -> ImmBorrow
let z2: &mut u8 = &mut foo.y;
- //~^ NOTE: Capturing foo[(1, 0)] -> MutBorrow
- //~| NOTE: Min Capture foo[(1, 0)] -> MutBorrow
+ //~^ NOTE: Capturing foo[] -> MutBorrow
+ //~| NOTE: Min Capture foo[] -> MutBorrow
*z2 = 42;
diff --git a/tests/ui/closures/2229_closure_analysis/repr_packed.stderr b/tests/ui/closures/2229_closure_analysis/repr_packed.stderr
index 580061ebc6e..32b3d844c6e 100644
--- a/tests/ui/closures/2229_closure_analysis/repr_packed.stderr
+++ b/tests/ui/closures/2229_closure_analysis/repr_packed.stderr
@@ -1,5 +1,5 @@
error[E0658]: attributes on expressions are experimental
- --> $DIR/repr_packed.rs:13:17
+ --> $DIR/repr_packed.rs:14:17
|
LL | let mut c = #[rustc_capture_analysis]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -26,7 +26,7 @@ LL | let c = #[rustc_capture_analysis]
= help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable
error: First Pass analysis includes:
- --> $DIR/repr_packed.rs:16:5
+ --> $DIR/repr_packed.rs:17:5
|
LL | / || {
LL | |
@@ -37,19 +37,19 @@ LL | | println!("({}, {})", z1, z2);
LL | | };
| |_____^
|
-note: Capturing foo[(0, 0)] -> ImmBorrow
- --> $DIR/repr_packed.rs:19:24
+note: Capturing foo[] -> ImmBorrow
+ --> $DIR/repr_packed.rs:20:24
|
LL | let z1: &u8 = &foo.x;
| ^^^^^
-note: Capturing foo[(1, 0)] -> MutBorrow
+note: Capturing foo[] -> MutBorrow
--> $DIR/repr_packed.rs:22:32
|
LL | let z2: &mut u8 = &mut foo.y;
| ^^^^^
error: Min Capture analysis includes:
- --> $DIR/repr_packed.rs:16:5
+ --> $DIR/repr_packed.rs:17:5
|
LL | / || {
LL | |
@@ -60,12 +60,7 @@ LL | | println!("({}, {})", z1, z2);
LL | | };
| |_____^
|
-note: Min Capture foo[(0, 0)] -> ImmBorrow
- --> $DIR/repr_packed.rs:19:24
- |
-LL | let z1: &u8 = &foo.x;
- | ^^^^^
-note: Min Capture foo[(1, 0)] -> MutBorrow
+note: Min Capture foo[] -> MutBorrow
--> $DIR/repr_packed.rs:22:32
|
LL | let z2: &mut u8 = &mut foo.y;