Skip to content
This repository has been archived by the owner on Apr 5, 2024. It is now read-only.

Handle #[repr(packed)] with feature gate #33

Closed
arora-aman opened this issue Dec 4, 2020 · 3 comments · Fixed by sexxi-goose/rust#49 or rust-lang/rust#82878
Closed

Handle #[repr(packed)] with feature gate #33

arora-aman opened this issue Dec 4, 2020 · 3 comments · Fixed by sexxi-goose/rust#49 or rust-lang/rust#82878
Assignees
Labels
bug Something isn't working
Projects

Comments

@arora-aman
Copy link
Member

#[repr(packed)]
struct Foo { x: u8, y: u8 }

fn main(foo: Foo) {
    let c = || {
        let z: &u8 = unsafe { &foo.x };
    
    };
    // cannot capture `foo.x` at the point of closure *creation*, must just capture `foo`
    // In your code:
    // You'll see a place like `[foo, x]`
    // if you have a field projection where the field is defined in a `#[repr(packed)]` struct, you need to capture the struct itself
}
@arora-aman arora-aman added the bug Something isn't working label Dec 4, 2020
@arora-aman arora-aman added this to To-do in RFC-2229 via automation Dec 4, 2020
@arora-aman
Copy link
Member Author

  • If foo.x is being moved, that's safe to do
  • If we do an Imm/Mut Borrow to the foo.x then it's going to be an unsafe operation and we stop at the struct and prevent capturing the field.

@arora-aman
Copy link
Member Author

We only want to truncate if the alignment is not 1. eg: when using u8s

@nikomatsakis
Copy link
Contributor

Note: There was an interesting case that we discussed on 2021-03-03. If you have a closure that accesses a packed field:

#[repr(packed)] struct Foo { x: String }

let f: Foo;
let c = || {
    let x = &f.x;
    drop(f.x);
}

This could conceivably be safe because the closure desugaring will move out of f.x and hence the packed field is never actually borrowed. However, we choose to still truncate (and hence make this unsafe) for consistency and lack of surprise. If the closure were inlined into its caller, for example, you would get errors because of the lack of an unsafe block.

The workaround for users is to add let tmp = f.x before the closure, just as you would in non-closure code.

@arora-aman arora-aman self-assigned this Mar 5, 2021
@arora-aman arora-aman moved this from To-do to In progress in RFC-2229 Mar 6, 2021
@arora-aman arora-aman moved this from In progress to In review in RFC-2229 Mar 7, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 12, 2021
…akis

2229: Handle capturing a reference into a repr packed struct

RFC 1240 states that it is unsafe to capture references into a
packed-struct. This PR ensures that when a closure captures a precise
path, we aren't violating this safety constraint.

To acheive so we restrict the capture precision to the struct itself.

An interesting edge case where we decided to restrict precision:
```rust
struct Foo(String);

let foo: Foo;
let c = || {
    println!("{}", foo.0);
    let x = foo.0;
}
```

Given how closures get desugared today, foo.0 will be moved into the
closure, making the `println!`, safe. However this can be very subtle
and also will be unsafe if the closure gets inline.

Closes: rust-lang/project-rfc-2229#33

r? `@nikomatsakis`
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 13, 2021
2229: Handle capturing a reference into a repr packed struct

RFC 1240 states that it is unsafe to capture references into a
packed-struct. This PR ensures that when a closure captures a precise
path, we aren't violating this safety constraint.

To acheive so we restrict the capture precision to the struct itself.

An interesting edge case where we decided to restrict precision:
```rust
struct Foo(String);

let foo: Foo;
let c = || {
    println!("{}", foo.0);
    let x = foo.0;
}
```

Given how closures get desugared today, foo.0 will be moved into the
closure, making the `println!`, safe. However this can be very subtle
and also will be unsafe if the closure gets inline.

Closes: rust-lang/project-rfc-2229#33

r? `@nikomatsakis`
RFC-2229 automation moved this from In review to Done Mar 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
RFC-2229
  
Done
2 participants