Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More deriving on packed structs #104429

Merged
merged 1 commit into from
Jan 30, 2023

Commits on Jan 30, 2023

  1. Allow more deriving on packed structs.

    Currently, deriving on packed structs has some non-trivial limitations,
    related to the fact that taking references on unaligned fields is UB.
    
    The current approach to field accesses in derived code:
    - Normal case: `&self.0`
    - In a packed struct that derives `Copy`: `&{self.0}`
    - In a packed struct that doesn't derive `Copy`: `&self.0`
    
    Plus, we disallow deriving any builtin traits other than `Default` for any
    packed generic type, because it's possible that there might be
    misaligned fields. This is a fairly broad restriction.
    
    Plus, we disallow deriving any builtin traits other than `Default` for most
    packed types that don't derive `Copy`. (The exceptions are those where the
    alignments inherently satisfy the packing, e.g. in a type with
    `repr(packed(N))` where all the fields have alignments of `N` or less
    anyway. Such types are pretty strange, because the `packed` attribute is
    not having any effect.)
    
    This commit introduces a new, simpler approach to field accesses:
    - Normal case: `&self.0`
    - In a packed struct: `&{self.0}`
    
    In the latter case, this requires that all fields impl `Copy`, which is
    a new restriction. This means that the following example compiles under
    the old approach and doesn't compile under the new approach.
    ```
     #[derive(Debug)]
     struct NonCopy(u8);
    
     #[derive(Debug)
     #[repr(packed)]
     struct MyType(NonCopy);
    ```
    (Note that the old approach's support for cases like this was brittle.
    Changing the `u8` to a `u16` would be enough to stop it working. So not
    much capability is lost here.)
    
    However, the other constraints from the old rules are removed. We can now
    derive builtin traits for packed generic structs like this:
    ```
     trait Trait { type A; }
    
     #[derive(Hash)]
     #[repr(packed)]
     pub struct Foo<T: Trait>(T, T::A);
    ```
    To allow this, we add a `T: Copy` bound in the derived impl and a `T::A:
    Copy` bound in where clauses. So `T` and `T::A` must impl `Copy`.
    
    We can now also derive builtin traits for packed structs that don't derive
    `Copy`, so long as the fields impl `Copy`:
    ```
     #[derive(Hash)]
     #[repr(packed)]
     pub struct Foo(u32);
    ```
    This includes types that hand-impl `Copy` rather than deriving it, such as the
    following, that show up in winapi-0.2:
    ```
     #[derive(Clone)]
     #[repr(packed)]
     struct MyType(i32);
    
     impl Copy for MyType {}
    ```
    The new approach is simpler to understand and implement, and it avoids
    the need for the `unsafe_derive_on_repr_packed` check.
    
    One exception is required for backwards-compatibility: we allow `[u8]`
    fields for now. There is a new lint for this,
    `byte_slice_in_packed_struct_with_derive`.
    nnethercote committed Jan 30, 2023
    Configuration menu
    Copy the full SHA
    2e93f2c View commit details
    Browse the repository at this point in the history