Auto merge of #115315 - RalfJung:field-capture-packed-alignment, r=oli-obk

Browse source

closure field capturing: don't depend on alignment of packed fields

This fixes the closure field capture part of https://github.com/rust-lang/rust/issues/115305: field capturing always stops at projections into packed structs, no matter the alignment of the field. This means changing a private field type from `u8` to `u64` can never change how closures capture fields, which is probably what we want.

Here's an example where, before this PR, changing the type of a private field in a repr(Rust) struct can change the output of a program:

```rust
#![allow(dead_code)]

mod m {
    // before patch
    #[derive(Default)]
    pub struct S1(u8);
    // after patch
    #[derive(Default)]
    pub struct S2(u64);
}

struct NoisyDrop;
impl Drop for NoisyDrop {
    fn drop(&mut self) {
        eprintln!("dropped!");
    }
}

#[repr(packed)]
struct MyType {
    field: m::S1, // output changes when this becomes S2
    other_field: NoisyDrop,
    third_field: Vec<()>,
}

fn test(r: MyType) {
    let c = || {
        let _val = std::ptr::addr_of!(r.field);
        let _val = r.third_field;
    };
    drop(c);
    eprintln!("before dropping");
}

fn main() {
    test(MyType {
        field: Default::default(),
        other_field: NoisyDrop,
        third_field: Vec::new(),
    });
}
```

Of course this is a breaking change for the same reason that doing field capturing in the first place was a breaking change. Packed fields are relatively rare and depending on drop order is relatively rare, so I don't expect this to have much impact, but it's hard to be sure and even a crater run will only tell us so much.

Also see the [nomination comment](https://github.com/rust-lang/rust/pull/115315#issuecomment-1702807825).

Cc `@rust-lang/wg-rfc-2229` `@ehuss`

...

This commit is contained in:

bors 2023-09-16 05:29:23 +00:00

parent 9d2372ef0f 9abdcad044

commit 1c6d7aae69

Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL

Show stats Download patch file Download diff file Expand all files Collapse all files

Diff content is not available