Skip to content

Commit

Permalink
Rollup merge of rust-lang#57229 - mikeyhew:fix-56806, r=varkor
Browse files Browse the repository at this point in the history
Fix rust-lang#56806 by using `delay_span_bug` in object safety layout sanity checks

It's possible that `is_object_safe` is called on a trait method that with an invalid receiver type. This caused an ICE in rust-lang#56806, because `receiver_is_dispatchable` returns `true` for `self: Box<dyn Trait>`, which causes one of the layout sanity checks in object_safety.rs to fail. Replacing `bug!` with `delay_span_bug` solves this.

The fact that `receiver_is_dispatchable` returns `true` here could be considered a bug. It passes the check that the method implements, though: `Box<dyn Trait>` implements `DispatchFromDyn<Box<dyn Trait>>` because `dyn Trait` implements `Unsize<dyn Trait>`. It would be good to hear what @eddyb and @nikomatsakis think.

Note that I only added a test for the case encountered in rust-lang#56806. I could not come up with a case that triggered an ICE from the other check, `bug!("receiver when Self = dyn Trait should be ScalarPair, found Scalar")`. There is no way, to my knowledge, that you can make `receiver_is_dispatchable` return true but still have a `Scalar` ABI when `Self = dyn Trait`.

One other case I encountered while debugging rust-lang#56806 was that if you have a type parameter `T` that implements `Deref<Target=Self>` and `DispatchFromDyn<T>`, and use it as a method receiver, it will cause an ICE during `is_object_safe` because `T` has no layout ([playground](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=d9b7497b3be0ca8382fa7d9497263214)):

```rust
trait Trait<T: Deref<Target=Self> + DispatchFromDyn<T>> {
    fn foo(self: T) -> dyn Trait<T>;
}
```

I don't intend to remove the ICE there because it is a pathological case, especially since there is no way to implement `DispatchFromDyn<T>` for `T` — the checks in typeck/coherence/builtin.rs do not allow that.

fixes rust-lang#56806
r? @varkor
  • Loading branch information
kennytm authored Jan 5, 2019
2 parents aa83481 + 2433526 commit 6c58b84
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 5 deletions.
23 changes: 18 additions & 5 deletions src/librustc/traits/object_safety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,15 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> {

match abi_of_ty(unit_receiver_ty) {
&Abi::Scalar(..) => (),
abi => bug!("Receiver when Self = () should have a Scalar ABI, found {:?}", abi)
abi => {
self.sess.delay_span_bug(
self.def_span(method.def_id),
&format!(
"Receiver when Self = () should have a Scalar ABI, found {:?}",
abi
),
);
}
}

let trait_object_ty = self.object_ty_for_trait(
Expand All @@ -373,10 +381,15 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> {

match abi_of_ty(trait_object_receiver) {
&Abi::ScalarPair(..) => (),
abi => bug!(
"Receiver when Self = {} should have a ScalarPair ABI, found {:?}",
trait_object_ty, abi
)
abi => {
self.sess.delay_span_bug(
self.def_span(method.def_id),
&format!(
"Receiver when Self = {} should have a ScalarPair ABI, found {:?}",
trait_object_ty, abi
),
);
}
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions src/test/ui/issues/issue-56806.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
pub trait Trait {
fn dyn_instead_of_self(self: Box<dyn Trait>);
//~^ ERROR invalid method receiver type: std::boxed::Box<(dyn Trait + 'static)>
}

pub fn main() {
}
12 changes: 12 additions & 0 deletions src/test/ui/issues/issue-56806.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0307]: invalid method receiver type: std::boxed::Box<(dyn Trait + 'static)>
--> $DIR/issue-56806.rs:2:34
|
LL | fn dyn_instead_of_self(self: Box<dyn Trait>);
| ^^^^^^^^^^^^^^
|
= note: type must be `Self` or a type that dereferences to it
= help: consider changing to `self`, `&self`, `&mut self`, or `self: Box<Self>`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0307`.

0 comments on commit 6c58b84

Please sign in to comment.