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

Properly handle Pin<&mut dyn* Trait> receiver in codegen #104594

Merged
merged 3 commits into from
Nov 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// that is understood elsewhere in the compiler as a method on
// `dyn Trait`.
// To get a `*mut RcBox<Self>`, we just keep unwrapping newtypes until
// we get a value of a built-in pointer type
// we get a value of a built-in pointer type.
//
// This is also relevant for `Pin<&mut Self>`, where we need to peel the `Pin`.
'descend_newtypes: while !op.layout.ty.is_unsafe_ptr()
&& !op.layout.ty.is_region_ptr()
{
Expand Down Expand Up @@ -980,13 +982,29 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
continue;
}
Immediate(_) => {
let ty::Ref(_, ty, _) = op.layout.ty.kind() else {
span_bug!(span, "can't codegen a virtual call on {:#?}", op);
};
if !ty.is_dyn_star() {
// See comment above explaining why we peel these newtypes
'descend_newtypes: while !op.layout.ty.is_unsafe_ptr()
&& !op.layout.ty.is_region_ptr()
{
for i in 0..op.layout.fields.count() {
let field = op.extract_field(bx, i);
if !field.layout.is_zst() {
// we found the one non-zero-sized field that is allowed
// now find *its* non-zero-sized field, or stop if it's a
// pointer
op = field;
continue 'descend_newtypes;
}
}

span_bug!(span, "receiver has no non-zero-sized fields {:?}", op);
}

// Make sure that we've actually unwrapped the rcvr down
// to a pointer or ref to `dyn* Trait`.
if !op.layout.ty.builtin_deref(true).unwrap().ty.is_dyn_star() {
span_bug!(span, "can't codegen a virtual call on {:#?}", op);
}
// FIXME(dyn-star): Make sure this is done on a &dyn* receiver
let place = op.deref(bx.cx());
let data_ptr = place.project_field(bx, 0);
let meta_ptr = place.project_field(bx, 1);
Expand Down
38 changes: 12 additions & 26 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -755,20 +755,9 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {

if let ty::Dynamic(a_data, _, _) = a.kind()
&& let ty::Dynamic(b_data, _, _) = b.kind()
&& a_data.principal_def_id() == b_data.principal_def_id()
{
if a_data.principal_def_id() == b_data.principal_def_id() {
return self.unify_and(a, b, |_| vec![]);
} else if !self.tcx().features().trait_upcasting {
let mut err = feature_err(
&self.tcx.sess.parse_sess,
sym::trait_upcasting,
self.cause.span,
&format!(
"cannot cast `{a}` to `{b}`, trait upcasting coercion is experimental"
),
);
err.emit();
}
return self.unify_and(a, b, |_| vec![]);
}

// Check the obligations of the cast -- for example, when casting
Expand Down Expand Up @@ -796,19 +785,16 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
])
.collect();

// Enforce that the type is `usize`/pointer-sized. For now, only those
// can be coerced to `dyn*`, except for `dyn* -> dyn*` upcasts.
if !a.is_dyn_star() {
obligations.push(Obligation::new(
self.tcx,
self.cause.clone(),
self.param_env,
ty::Binder::dummy(
self.tcx.at(self.cause.span).mk_trait_ref(hir::LangItem::PointerSized, [a]),
)
.to_poly_trait_predicate(),
));
}
// Enforce that the type is `usize`/pointer-sized.
obligations.push(Obligation::new(
self.tcx,
self.cause.clone(),
self.param_env,
ty::Binder::dummy(
self.tcx.at(self.cause.span).mk_trait_ref(hir::LangItem::PointerSized, [a]),
)
.to_poly_trait_predicate(),
));

Ok(InferOk {
value: (vec![Adjustment { kind: Adjust::DynStar, target: b }], b),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -776,9 +776,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {

match (source.kind(), target.kind()) {
// Trait+Kx+'a -> Trait+Ky+'b (upcasts).
(&ty::Dynamic(ref data_a, _, dyn_a), &ty::Dynamic(ref data_b, _, dyn_b))
if dyn_a == dyn_b =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surprised by this removal

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the more specific pattern here and in the next file, but doesn't this code have to run for non dyn*?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you be more specific? Did you see the PR description that mentions "Also, disable dyn* trait upcasting"?

Because all this disables is CoerceUnsized implementations from dyn* Trait to dyn* SuperTrait, which cannot be implemented with CoerceUnsized and need to be reworked.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. r=me after rebasing

{
(&ty::Dynamic(ref data_a, _, ty::Dyn), &ty::Dynamic(ref data_b, _, ty::Dyn)) => {
// Upcast coercions permit several things:
//
// 1. Dropping auto traits, e.g., `Foo + Send` to `Foo`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -803,9 +803,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
let upcast_trait_ref;
match (source.kind(), target.kind()) {
// TraitA+Kx+'a -> TraitB+Ky+'b (trait upcasting coercion).
(&ty::Dynamic(ref data_a, r_a, repr_a), &ty::Dynamic(ref data_b, r_b, repr_b))
if repr_a == repr_b =>
{
(
&ty::Dynamic(ref data_a, r_a, repr_a @ ty::Dyn),
&ty::Dynamic(ref data_b, r_b, ty::Dyn),
) => {
// See `assemble_candidates_for_unsizing` for more info.
// We already checked the compatibility of auto traits within `assemble_candidates_for_unsizing`.
let principal_a = data_a.principal().unwrap();
Expand All @@ -831,7 +832,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
.map(ty::Binder::dummy),
);
let existential_predicates = tcx.mk_poly_existential_predicates(iter);
let source_trait = tcx.mk_dynamic(existential_predicates, r_b, repr_b);
let source_trait = tcx.mk_dynamic(existential_predicates, r_b, repr_a);

// Require that the traits involved in this upcast are **equal**;
// only the **lifetime bound** is changed.
Expand Down
52 changes: 52 additions & 0 deletions src/test/ui/dyn-star/dispatch-on-pin-mut.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// run-pass
// edition:2021
// check-run-results

#![feature(dyn_star)]
//~^ WARN the feature `dyn_star` is incomplete and may not be safe to use and/or cause compiler crashes

use std::future::Future;

async fn foo(f: dyn* Future<Output = i32>) {
println!("value: {}", f.await);
}

async fn async_main() {
foo(Box::pin(async { 1 })).await
}

// ------------------------------------------------------------------------- //
// Implementation Details Below...

use std::pin::Pin;
use std::task::*;

pub fn noop_waker() -> Waker {
let raw = RawWaker::new(std::ptr::null(), &NOOP_WAKER_VTABLE);

// SAFETY: the contracts for RawWaker and RawWakerVTable are upheld
unsafe { Waker::from_raw(raw) }
}

const NOOP_WAKER_VTABLE: RawWakerVTable = RawWakerVTable::new(noop_clone, noop, noop, noop);

unsafe fn noop_clone(_p: *const ()) -> RawWaker {
RawWaker::new(std::ptr::null(), &NOOP_WAKER_VTABLE)
}

unsafe fn noop(_p: *const ()) {}

fn main() {
let mut fut = async_main();

// Poll loop, just to test the future...
let waker = noop_waker();
let ctx = &mut Context::from_waker(&waker);

loop {
match unsafe { Pin::new_unchecked(&mut fut).poll(ctx) } {
Poll::Pending => {}
Poll::Ready(()) => break,
}
}
}
1 change: 1 addition & 0 deletions src/test/ui/dyn-star/dispatch-on-pin-mut.run.stdout
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
value: 1
11 changes: 11 additions & 0 deletions src/test/ui/dyn-star/dispatch-on-pin-mut.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
warning: the feature `dyn_star` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/dispatch-on-pin-mut.rs:5:12
|
LL | #![feature(dyn_star)]
| ^^^^^^^^
|
= note: see issue #91611 <https://github.com/rust-lang/rust/issues/91611> for more information
= note: `#[warn(incomplete_features)]` on by default

warning: 1 warning emitted

5 changes: 3 additions & 2 deletions src/test/ui/dyn-star/dont-unsize-coerce-dyn-star.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// check-pass
// run-pass
// check-run-results

#![feature(dyn_star)]
#![allow(incomplete_features)]
//~^ WARN the feature `dyn_star` is incomplete and may not be safe to use and/or cause compiler crashes

trait AddOne {
fn add1(&mut self) -> usize;
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/dyn-star/dont-unsize-coerce-dyn-star.run.stdout
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
43
44
11 changes: 11 additions & 0 deletions src/test/ui/dyn-star/dont-unsize-coerce-dyn-star.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
warning: the feature `dyn_star` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/dont-unsize-coerce-dyn-star.rs:4:12
|
LL | #![feature(dyn_star)]
| ^^^^^^^^
|
= note: see issue #91611 <https://github.com/rust-lang/rust/issues/91611> for more information
= note: `#[warn(incomplete_features)]` on by default

warning: 1 warning emitted

13 changes: 13 additions & 0 deletions src/test/ui/dyn-star/no-unsize-coerce-dyn-trait.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#![feature(dyn_star, trait_upcasting)]
//~^ WARN the feature `dyn_star` is incomplete and may not be safe to use and/or cause compiler crashes

trait A: B {}
trait B {}
impl A for usize {}
impl B for usize {}

fn main() {
let x: Box<dyn* A> = Box::new(1usize as dyn* A);
let y: Box<dyn* B> = x;
//~^ ERROR mismatched types
}
23 changes: 23 additions & 0 deletions src/test/ui/dyn-star/no-unsize-coerce-dyn-trait.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
warning: the feature `dyn_star` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/no-unsize-coerce-dyn-trait.rs:1:12
|
LL | #![feature(dyn_star, trait_upcasting)]
| ^^^^^^^^
|
= note: see issue #91611 <https://github.com/rust-lang/rust/issues/91611> for more information
= note: `#[warn(incomplete_features)]` on by default

error[E0308]: mismatched types
--> $DIR/no-unsize-coerce-dyn-trait.rs:11:26
|
LL | let y: Box<dyn* B> = x;
| ----------- ^ expected trait `B`, found trait `A`
| |
| expected due to this
|
= note: expected struct `Box<dyn* B>`
found struct `Box<dyn* A>`

error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0308`.
3 changes: 1 addition & 2 deletions src/test/ui/dyn-star/upcast.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// run-pass
// known-bug: #104800

#![feature(dyn_star, trait_upcasting)]
#![allow(incomplete_features)]

trait Foo: Bar {
fn hello(&self);
Expand Down
20 changes: 20 additions & 0 deletions src/test/ui/dyn-star/upcast.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
warning: the feature `dyn_star` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/upcast.rs:3:12
|
LL | #![feature(dyn_star, trait_upcasting)]
| ^^^^^^^^
|
= note: see issue #91611 <https://github.com/rust-lang/rust/issues/91611> for more information
= note: `#[warn(incomplete_features)]` on by default

error[E0277]: `dyn* Foo` needs to be a pointer-sized type
--> $DIR/upcast.rs:30:23
|
LL | let w: dyn* Bar = w;
| ^ `dyn* Foo` needs to be a pointer-sized type
|
= help: the trait `PointerSized` is not implemented for `dyn* Foo`

error: aborting due to previous error; 1 warning emitted

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