Support composing two deref coercion adjustments by adwinwhite · Pull Request #148320 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation27 Commits1 Checks11 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Fixes #148283
Given a deref chain like &C -> &B -> &A, we do can coerce &C -> &A directly with adjustments:
rustc_hir_typeck::fn_ctxt::_impl::apply_adjustments adj=[
Deref(None) -> C,
Deref(Some(OverloadedDeref { mutbl: Not, ..)) -> B,
Deref(Some(OverloadedDeref { mutbl: Not, ..)) -> A,
Borrow(Ref(Not)) -> &A
]
But if we coerce in two steps: &C -> &B and &B -> &A, it errs with
can't compose [
Deref(None) -> C,
Deref(Some(OverloadedDeref { mutbl: Not, ..)) -> B,
Borrow(Ref(Not)) -> &B
] and [
Deref(None) -> B,
Deref(Some(OverloadedDeref { mutbl: Not, ..)) -> A,
Borrow(Ref(Not)) -> &A
]
This PR bridges the gap.
About the FIXME, I'm not familiar with unsafe fn pointer so that's not covered.
Edited: My change allows more than deref composition. I think restricting it to exactly deref composition is suitable, to avoid surprising behavior where coercion order affects method selection.
Other unsupported composition can be reported by proper diagnostics.
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
r? @BoxyUwU
rustbot has assigned @BoxyUwU.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
With this PR, the following code compiles, and prints Two method: 1. Is this intended?
(The PR makes the compiler do the coercion of &Three -> &Two -> &dyn OneTrait, even though a direct coercion &Three -> &dyn OneTrait is available and has different behavior.)
(This code ICEs on nightly.)
use core::ops::Deref;
trait OneTrait { fn method(&self); } struct Two(i32); struct Three(Two);
struct OneThing; impl OneTrait for OneThing { fn method(&self) { println!("OneThing method"); } }
impl Deref for Three { type Target = Two; fn deref(&self) -> &Self::Target { &self.0 } }
// Commenting the following impl causes the code to output Three method: 1
impl OneTrait for Two {
fn method(&self) {
println!("Two method: {}", self.0);
}
}
// Commenting the following impl does NOT change the output impl OneTrait for Three { fn method(&self) { println!("Three method: {}", self.0.0); } }
fn main() {
let x = match 0 {
0 => &Three(Two(1)),
// Commenting the below line causes the code to output Three method: 1
1 => &Two(2),
_ => &OneThing as &dyn OneTrait,
};
x.method();
}
This seems a bit fishy, why would we special-case two-step coercions? Does this handle three-steps &D -> &C -> &B -> &A? Or is this supposed to do an n-ary &Tn -> ... -> &T1 deref-coercion-chain where we do a "big-step" from &Tn -> &T1? This seems very surprising and not-obvious from a user POV?
EDIT: oh, reading the original issue, is this supposed to be an intended language behavior? That feels a bit surprising to me.
It does handle arbitrary steps of composition. But I may need to further restrict what composition is allowed, given the example.
The following code ICEs both on nightly and with this PR:
use std::ops::Deref;
trait One {} trait Two {}
struct Thing; impl One for Thing {} struct Thing2; impl Two for Thing2 {}
impl<'a> Deref for dyn One + 'a { type Target = dyn Two + 'a; fn deref(&self) -> &(dyn Two + 'a) { &Thing2 } }
fn main() { let _ = match 0 { 0 => &Thing as &dyn One, 1 => &Thing, _ => &Thing2 as &dyn Two, }; }
This code also ICEs both on nightly and with this PR:
trait Super {} trait Sub: Super {}
struct Thing; impl Super for Thing {} impl Sub for Thing {}
fn main() { let _ = match 0 { 0 => &Thing as &dyn Sub, 1 => &Thing, _ => &Thing as &dyn Super, }; }
can't compose [
Deref(None) -> Thing,
Borrow(Ref(Not)) -> &Thing,
Pointer(Unsize) -> &dyn One
] and [
Deref(None) -> dyn One,
Deref(Some(OverloadedDeref { mutbl: Not, ..)) -> dyn Two,
Borrow(Ref(Not)) -> &dyn Two
]
This can be supported as well. Not so sure about whether we should.
I need to think about your first example first before allowing more code.
Of note: This code currently compiles on stable, and seems to rely on a special case in the compiler to treat this as a double coercion and allow it, I think.
fn never() -> ! { panic!() }
fn main() { let _ = match 1 { 0 => never(), 1 => &Box::new(2), _ => &3, }; }
Lang-nominating, since a decision needs to be made on what exactly should LUB coercion do with double coercions, especially in cases where an X -> Y -> Z coercion has different run time behavior from an X -> Z coercion.
These two code snippets seem to involve a double-coercion, and compiles on stable:
use std::ops::Deref; use std:📑:PhantomData;
struct Thing; struct Wrap(PhantomData);
// Sub is a subtype of Super type Sub = Wrap<for<'a> fn(&'a ()) -> &'a ()>; type Super = Wrap<fn(&'static ()) -> &'static ()>;
impl Deref for Thing { type Target = Sub; fn deref(&self) -> &Sub { &Wrap(PhantomData) } }
fn main() { let _ = match 0 { 0 => &Thing, 1 => &Wrap(PhantomData) as &Super, _ => &Wrap(PhantomData) as &Sub, }; }
use std::ops::Deref; use std:📑:PhantomData;
struct Thing; struct Wrap(PhantomData);
// Sub is a subtype of Super type Sub = Wrap<for<'a> fn(&'a ()) -> &'a ()>; type Super = Wrap<fn(&'static ()) -> &'static ()>;
impl Deref for Super { type Target = Thing; fn deref(&self) -> &Thing { &Thing } }
fn main() { let _ = match 0 { 0 => &Wrap(PhantomData) as &Super, 1 => &Wrap(PhantomData) as &Sub, _ => &Thing, }; }
Oddly enough, swapping the order of Super and Sub makes it not compile (Edit: this is #73154 and #147565):
use std:📑:PhantomData;
struct Wrap(PhantomData);
// Sub is a subtype of Super type Sub = Wrap<for<'a> fn(&'a ()) -> &'a ()>; type Super = Wrap<fn(&'static ()) -> &'static ()>;
fn main() { let _ = match 0 { 1 => &Wrap(PhantomData) as &Sub, _ => &Wrap(PhantomData) as &Super, }; }
error[E0308]: mismatched types
--> src/main.rs:12:36
|
12 | _ => &Wrap(PhantomData) as &Super,
| ^^^^^^ one type is more general than the other
|
= note: expected reference `&Wrap<for<'a> fn(&'a ()) -> &'a ()>`
found reference `&Wrap<fn(&()) -> &()>`
For more information about this error, try `rustc --explain E0308`.
My change does allow more than deref composition. I think restricting it to exactly deref composition is suitable.
Other unsupported composition can be reported by proper diagnostics.
The question here isn't whether these double coercions can be supported. The question is whether they should be supported.
Even for the case of chaining two deref coercions together, there can still be strange behavior.
In the following code, the following coercions are available:
Three -> Two(deref coercion)Two -> One(deref coercion)Three -> One(unsize coercion)
With this PR, the compiler decides to do the Three -> Two -> One roundabout coercion, and the code prints Wrap([12]).
(This code ICEs on nightly.)
use core::ops::Deref;
#[derive(Debug)] struct Wrap<T: ?Sized>(T);
type One = Wrap<[u8]>; struct Two; type Three = Wrap<[u8; 0]>;
impl Deref for Three { type Target = Two; fn deref(&self) -> &Self::Target { &Two } }
impl Deref for Two { type Target = One; fn deref(&self) -> &One { &Wrap([12u8]) } }
fn main() {
let x = match 0 {
0 => &Wrap([]) as &Three,
// Commenting the below line causes the code to output Wrap([])
1 => &Two,
_ => &Wrap([34u8]) as &One,
};
println!("{x:?}");
}
unnominating until i get a chance to look more at this PR and probably write up a description that is less impl details. also this seems more like t-types than t-lang to me but I've yet to properly look at this
In case you missed it, the reference has a section on LUB coercions. It's ambiguous on if and how double coercions should work though.
Relevant to the language team
This change is insta-stable, or significant enough to need a team FCP to proceed.
Items that are on lang's radar and will need eventual work or consideration.
Relevant to the types team, which will review and decide on the PR/issue.
labels
the reference has a section on LUB coercions. It's ambiguous on if and how double coercions should work though.
Incidentally, it was work on improving the examples in that section that caused me to notice and file #148283.
As I read the algorithm in that section, I would expect N-coercions to work (modulo, of course, the caveat at the bottom about this being an "obviously informal" description).
cc @rust-lang/lang
unnominating until i get a chance to look more at this PR and probably write up a description that is less impl details. also this seems more like t-types than t-lang to me but I've yet to properly look at this
this is almost certainly fully t-types territory, not t-lang
@BoxyUwU let me know if you want any help here
@jackh726 if you'd be able to take over review of this PR that'd be helpful :> I'm definitely light on time right now.
I have some misc thoughts about this PR. Mostly that I think most adjustments do make sense to compose. I'm also somewhat unsure about whether composing autoderef steps works properly with the new solver's handling of opaque types when autodereffing stuff.
Unsizing does give me concern though 🤔 It seems notable to me that we don't actually support coercing from &Foo to &[u8] in one coercion where Foo: Deref<Target = [u8; 2]>. Or in other words, non-lub coercions can't compose autoderefs and unsizing. Making coerce-lub more powerful here seems undesirable if we don't also increase the flexibility of non-lub coercions.
The interactions of Deref and unsizing also seems kinda funny. We'll try to deref to match the target of the coercion and if that succeeds we won't try to unsize. I wonder if this behaviour can be demonstrated with derive(CoercePointee) where we now have arbitrary types with coercions.
Generally I think not supporting unsizing coercions in coerce-lub if the expr has already been adjusted seems like the right choice to me for now. It seems like a lot of work to get "reasonable" behaviour here to me when other adjustments seem fairly reasonable.
I do think we should support compsing UnsafeFnPointer with other adjustments too as otherwise code like this can't work:
fn main() { fn fndef() {}
match () {
_ if true => fndef,
_ if true => fndef as fn(),
_ => fndef as unsafe fn(),
};}
or:
fn main() { fn fndef() {}
match () {
_ if true => || (),
_ if true => (|| ()) as fn(),
_ => (|| ()) as unsafe fn(),
};}
I wonder if this behaviour can be demonstrated with
derive(CoercePointee)where we now have arbitrary types with coercions.
The reference currently states that transitive coercions are allowed. However, it also states that "this is not fully supported yet".
I believe that this blanket statement that "transitive coercions are allowed" is incoherent/untenable, given that we can set things up so that coercing twice has a different run time behavior than coercing once.
This text was added to the reference was added seemingly without discussion in rust-lang/reference@4c39f37 (Edit: The text came from the RFC in the comment below.)
Transitive coercions were seemingly first mentioned in RFC 401. The RFC was accepted without discussion on this topic.
The tracking issue for the RFC seems to indicate that parts of the RFC was not implemented, without a clear reason stated. It does mention transitive coercions by linking to #18602 and #103259, which does not really give more information.
My impression of this is that transitive coercions were a poorly thought out addition to the RFC, and implementing them was forgotten. My opinion is that we should not support transitive coercions in any form, assuming that the breakage isn't too great. Alternatively, transitive coercions could be allowed on a case-by-case basis, and the reference would need to list all combinations of transitive coercions that are allowed.
Edit: Perhaps there needs to be an exception specifically for doing multiple deref coercions in a row, optionally preceded with a &mut->& coercion. And another exception for reference->pointer, mut->const, and unsizing coercions. And maybe something involving subtyping. Other than that, I don't think other double coercions should be allowed.
I'm going to lang nominate this, mostly because I want to hear from @nikomatsakis on the history here, and also because I want to hear people's thoughts on what they would expect in terms of user-facing behavior.
Nominated for discussion during a lang team meeting.
Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang
labels
This code compiles on stable and requires chaining two deref coercions, although not in a lub:
use std::ops::Deref; struct One; struct Two; struct Three; impl Deref for Two { type Target = One; fn deref(&self) -> &One { &One } } impl Deref for Three { type Target = Two; fn deref(&self) -> &Two { &Two } } fn main() { let x: &One = &Three; }
By the way, this PR's title is incorrect. Autoderef applies to only method call expressions. The PR implements chaining deref coercions.
A proposal for handling lub coercions: Use the lub algorithm in the reference to compute the destination type. Then, from the source type and target type, recompute the coercion needed to do the transformation. If the resulting coercion fails, or is not "equivalent" to the coercion chain generated by the lub, emit an error. (Todo: define "equivalent".)
Support composing two autoderef adjustments Support composing two deref coercion adjustments
This code compiles on stable and requires chaining two deref coercions, although not in a lub...
This variant of your earlier example also compiles on nightly and runs without error:
use core::{ops::Deref, sync::atomic::{AtomicU64, Ordering}};
static X: AtomicU64 = AtomicU64::new(0);
#[derive(Debug)] struct W<T: ?Sized>(T);
type One = W<[u8]>; struct Two; type Three = W<[u8; 0]>;
impl Deref for Three { type Target = Two; fn deref(&self) -> &Self::Target { X.fetch_add(1, Ordering::Relaxed); &Two } }
impl Deref for Two { type Target = One; fn deref(&self) -> &One { X.fetch_add(1, Ordering::Relaxed); &W([0u8]) } }
fn f(x: &Three) -> &One { x }
fn main() { f(&W([0u8; 0])); assert_eq!(0, X.load(Ordering::Relaxed)); // The deref impls were not run, so the unsized coercion was // preferred to the deref coercions. }
It could choose to go &Three -> &Two -> &One via deref coercion, and it would do that if it had no other choice, as you demonstrate, but it instead chooses &Three -> &One via unsized coercion.
This example can be simplified. It ICEs on both stable 1.91.1 and nightly 2025-11-16:
trait Super {} trait Sub: Super {}
struct Thing; impl Super for Thing {} impl Sub for Thing {}
fn main() { let _ = match 0 { 0 => &Thing as &dyn Sub, 1 => &Thing as &dyn Sub, _ => &Thing as &dyn Super, }; }
The error ends with the somewhat nonsensical
can't compose [Deref(None) -> dyn Sub, Borrow(Ref(Not)) -> &dyn Sub, Pointer(Unsize) -> _] and
[Deref(None) -> dyn Sub, Borrow(Ref(Not)) -> &dyn Sub, Pointer(Unsize) -> _]
Nominated for discussion during a lang team meeting.
Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang
labels
Labels
Items that are on lang's radar and will need eventual work or consideration.
This change is insta-stable, or significant enough to need a team FCP to proceed.
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
Relevant to the language team
Relevant to the types team, which will review and decide on the PR/issue.