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

Take 2: Implement object-safety and dynamic dispatch for arbitrary_self_types #54383

Merged
merged 19 commits into from
Nov 3, 2018

Commits on Nov 1, 2018

  1. Configuration menu
    Copy the full SHA
    d14af13 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    a920036 View commit details
    Browse the repository at this point in the history
  3. Add CoerceSized trait and lang item

    This trait is more-or-less the reverse of CoerceUnsized, and will be
    used for object-safety checks. Receiver types like `Rc` will have to
    implement `CoerceSized` so that methods that use `Rc<Self>` as the
    receiver will be considered object-safe.
    mikeyhew committed Nov 1, 2018
    Configuration menu
    Copy the full SHA
    be80a79 View commit details
    Browse the repository at this point in the history
  4. Implement the object-safety checks for arbitrary_self_types: part 1

    For a trait method to be considered object-safe, the receiver type must
    satisfy certain properties: first, we need to be able to get the vtable
    to so we can look up the method, and second, we need to convert the
    receiver from the version where `Self=dyn Trait`, to the version where
    `Self=T`, `T` being some unknown, `Sized` type that implements `Trait`.
    
    To check that the receiver satisfies those properties, we use the
    following query:
    
    forall (U) {
    if (Self: Unsize<U>) {
    Receiver[Self => U]: CoerceSized<Receiver>
    }
    }
    
    where `Receiver` is the receiver type of the method (e.g. `Rc<Self>`),
    and `Receiver[Self => U]` is the receiver type where `Self = U`, e.g.
    `Rc<U>`.
    
    forall queries like this aren’t implemented in the trait system yet, so
    for now we are using a bit of a hack — see the code for explanation.
    mikeyhew committed Nov 1, 2018
    Configuration menu
    Copy the full SHA
    d5c2c4a View commit details
    Browse the repository at this point in the history
  5. Implement object-safety for arbitrary_self_types: part 2

    For now, all of the receivers that we care about are just a newtyped
    pointer — i.e. `Box<Self>`, `Rc<Self>`, `Pin<Box<Self>>`, `Pin<&mut
    Self>`. This is much simpler to implement in codeine than the more
    general case, because the ABI is the same as a pointer. So we add some
    checks in typeck/coherence/builtin.rs to make sure that implementors of
    CoerceSized are just newtyped pointers. In this commit, we also
    implement the codegen bits.
    mikeyhew committed Nov 1, 2018
    Configuration menu
    Copy the full SHA
    9f59da2 View commit details
    Browse the repository at this point in the history
  6. Add CoerceSized impls throughout libstd

    This will make receiver types like `Rc<Self>` and `Pin<&mut Self>`
    object-safe.
    mikeyhew committed Nov 1, 2018
    Configuration menu
    Copy the full SHA
    192900e View commit details
    Browse the repository at this point in the history
  7. update tests that have changed output

     I’m not sure why these tests have different output now, but they do.
    In all cases, the error message that is missing looks like this: “the
    trait bound `dyn Trait: Trait` is not satisfied”
    
    My guess is that the error message is going away because object-safety
    now involves trait solving, and these extra error messages are no
    longer leaking out.
    mikeyhew committed Nov 1, 2018
    Configuration menu
    Copy the full SHA
    a0f23f8 View commit details
    Browse the repository at this point in the history
  8. Configuration menu
    Copy the full SHA
    82f1f9a View commit details
    Browse the repository at this point in the history
  9. fix docs on trait

    tests were failing because I didn't wrap code snippets like  in backticks. fixed that now, so hopefully tests will pass on travis
    mikeyhew committed Nov 1, 2018
    Configuration menu
    Copy the full SHA
    c29641e View commit details
    Browse the repository at this point in the history
  10. Replace CoerceSized trait with DispatchFromDyn

    Rename `CoerceSized` to `DispatchFromDyn`, and reverse the direction so that, for example, you write
    
    ```
    impl<T: Unsize<U>, U> DispatchFromDyn<*const U> for *const T {}
    ```
    
    instead of
    
    ```
    impl<T: Unsize<U>, U> DispatchFromDyn<*const T> for *const U {}
    ```
    
    this way the trait is really just a subset of `CoerceUnsized`.
    
    The checks in object_safety.rs are updated for the new trait, and some documentation and method names in there are updated for the new trait name — e.g. `receiver_is_coercible` is now called `receiver_is_dispatchable`. Since the trait now works in the opposite direction, some code had to updated here for that too.
    
    I did not update the error messages for invalid `CoerceSized` (now `DispatchFromDyn`) implementations, except to find/replace `CoerceSized` with `DispatchFromDyn`. Will ask for suggestions in the PR thread.
    mikeyhew committed Nov 1, 2018
    Configuration menu
    Copy the full SHA
    f12c250 View commit details
    Browse the repository at this point in the history
  11. Configuration menu
    Copy the full SHA
    48d7f8d View commit details
    Browse the repository at this point in the history
  12. fix error-index test

    mikeyhew committed Nov 1, 2018
    Configuration menu
    Copy the full SHA
    3c56d8d View commit details
    Browse the repository at this point in the history
  13. Configuration menu
    Copy the full SHA
    74ef46c View commit details
    Browse the repository at this point in the history
  14. Add layout sanity checks in object safety

    If object-safety checks succeed for a receiver type, make sure the
    receiver’s abi is
    a) a Scalar, when Self = ()
    b) a ScalarPair, when Self = dyn Trait
    mikeyhew committed Nov 1, 2018
    Configuration menu
    Copy the full SHA
    6f2a161 View commit details
    Browse the repository at this point in the history
  15. Put backticks around field names, types and paths in error messages

    Added to `DispatchFromDyn` and `CoerceUnsized` error messages
    mikeyhew committed Nov 1, 2018
    Configuration menu
    Copy the full SHA
    b5b25f8 View commit details
    Browse the repository at this point in the history
  16. add U: Trait to the param env during DispatchFromDyn check

    also updated the doc on `receiver_is_dispatchable` to reflect current state of the implementation
    mikeyhew committed Nov 1, 2018
    Configuration menu
    Copy the full SHA
    eb997d7 View commit details
    Browse the repository at this point in the history
  17. Remove this check for object-safety during selection of trait object …

    …candidates
    
    I don't really understand what it's for, but see the comment here:
    rust-lang#50173 (comment)
    
    where arielb1 said
    
    > Does this check do anything these days? I think `$0: Trait` is always considered ambiguous
    
    and nikomatsakis agreed we may be able to get rid of it
    mikeyhew committed Nov 1, 2018
    Configuration menu
    Copy the full SHA
    3db2203 View commit details
    Browse the repository at this point in the history
  18. Add a check for reprs that could change the ABI

    disallow `#[repr(C)] and `#[repr(packed)]` on structs implementing DispatchFromDyn because they will change the ABI from Scalar/ScalarPair to Aggregrate, resulting in an ICE during object-safety checks or codegen
    mikeyhew committed Nov 1, 2018
    Configuration menu
    Copy the full SHA
    a468da9 View commit details
    Browse the repository at this point in the history
  19. Configuration menu
    Copy the full SHA
    192e7c4 View commit details
    Browse the repository at this point in the history