Add trait_upcasting related languages changes #1259
Conversation
ehuss
added
the
S-waiting-on-stabilization
Co-authored-by: Michael Goulet <[email protected]>
|
What is the status here? The feature is being stabilized so maybe we should merge the reference changes :) |
| @@ -34,6 +34,7 @@ code. | |||
| * Invoking undefined behavior via compiler intrinsics. | |||
| * Executing code compiled with platform features that the current platform | |||
| does not support (see [`target_feature`]), *except* if the platform explicitly documents this to be safe. | |||
| * Performing non-nop coercion on a dangling or unaligned raw pointer. | |||
There was a problem hiding this comment.
I'm not exactly clear why this is being added in this PR, since I can't find any reference to it related to RFC 3324. Can you say more about why this is being added? Is there some context here?
cc @RalfJung since you often know about UB stuff.
There was a problem hiding this comment.
I believe it was decided in a T-lang FCP in rust-lang/rust#101336, the stabilization of trait_upcasting just becomes the first time that this behavior can be exercised, as far as I recall.
There was a problem hiding this comment.
My expectation was that it is completely okay for the pointer itself to be dangling or unaligned; only the vtable must be valid. And we already require the vtable to be valid; we say it is UB when one creates the following:
Invalid metadata in a wide reference, `Box<T>`, or raw pointer
So I don't think any change is needed to the UB list. At least I highly doubt we want to require the data part of the wide raw pointer to be valid in any way for such a coercion.
Cc @rust-lang/opsem
The reference changes have to wait for the stabilization PR to merge. Sorry I didn't get to reviewing this earlier, I did not know it was being stabilized, and I tend to not prioritize pre-stabilization PRs when it seems like they are a long way off from being stabilized. |
| @@ -34,6 +34,7 @@ code. | |||
| * Invoking undefined behavior via compiler intrinsics. | |||
| * Executing code compiled with platform features that the current platform | |||
| does not support (see [`target_feature`]), *except* if the platform explicitly documents this to be safe. | |||
| * Performing non-nop coercion on a dangling or unaligned raw pointer. | |||
There was a problem hiding this comment.
Though now that I read this, I think this should be clarified though to integrate language which more precisely reflects the decision in rust-lang/rust#101336.
Specifically, should we mention that the vtable pointer needs to point to a valid vtable?
| @@ -172,6 +172,8 @@ an implementation of `Unsize<U>` for `T` will be provided: | |||
|
|
|||
| * `T` to `dyn U`, when `T` implements `U + Sized`, and `U` is [object safe]. | |||
|
|
|||
| * `dyn T` to `dyn U`, when `U` is one of `T`'s supertraits. | |||
There was a problem hiding this comment.
This should also mention that upcasting coercions allow dropping auto traits. i.e. dyn Trait + Send -> dyn Trait.
| @@ -159,7 +159,7 @@ Coercion is allowed between the following types: | |||
| ### Unsized Coercions | |||
|
|
|||
| The following coercions are called `unsized coercions`, since they | |||
| relate to converting sized types to unsized types, and are permitted in a few | |||
| relate to converting types to unsized types, and are permitted in a few | |||
| cases where other coercions are not, as described above. They can still happen | |||
| anywhere else a coercion can occur. | |||
There was a problem hiding this comment.
IMO it is worth stating explicitly that "unsizing" is a misnomer now and this also covers coercions from unsized types to unsized types.
|
Ping @crlf0710 Do you think you'd be able to address the comments here? |
|
This feature is being reverted so we should not land this PR. That said, it'd still be good to have the PR ready for when this gets stabilized for real. :) |
|
Yea, I wanted to get this ready for when it lands again. The stabilization should not have been done without this being ready, but I let it fall through the cracks. |
Frankly, i don't feel i have enough skill of writing to address the comments here(especially as i'm not a English-speaker). I can deal with "add XXX after YYY"-style of comments well, but i can't deal with the "This should mention" and "worth stating explicitly" style of comments. |
|
Help wanted: It would be nice if someone could post a new PR addressing comments above and superseding this PR. |
|
We discussed this in the lang-docs call today. It seems this stabilization still needs to happen, though the way for it has been mostly cleared with this PR being merged: ..and the documentation here likely needs to be updated accordingly. |
|
Superseded by #1622 :) |
cc rust-lang/rust#65991