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

Objects should be upcastable to supertraits #2765

Open
jdm opened this issue Apr 1, 2013 · 42 comments
Open

Objects should be upcastable to supertraits #2765

jdm opened this issue Apr 1, 2013 · 42 comments
Labels
A-trait-object Proposals relating to trait objects. T-lang Relevant to the language team, which will review and decide on the RFC.

Comments

@jdm
Copy link

jdm commented Apr 1, 2013

trait T {
    fn foo(@mut self);
}

struct S {
    unused: int
}

impl T for S {
    fn foo(@mut self) {
    }
}

fn main() {
    let s = @S { unused: 0 };
    let s2 = s as @T;
    let s3 = s2 as @T;
}

error: failed to find an implementation of trait @T for T

@catamorphism
Copy link

Reproduced with 64963d6cbaea86e0d2a58f507e57a76da7512e3e. Nominating for milestone 5, production-ready

@emberian
Copy link
Member

emberian commented Aug 5, 2013

The error as of 6c2dc3c is:

foo.rs:17:13: 17:21 error: can only cast an @-pointer to an @-object, not a trait T
foo.rs:17     let s3 = s2 as @T;
                       ^~~~~~~~
error: aborting due to previous error

which is a bit weird, why shouldn't you be able to cast a trait object to the same trait object?

@glaebhoerl
Copy link
Contributor

IINM there's two ways you could interpret this: one is that it's the identity function (casting something to its own type), the other is that you're looking for an impl T for @T, and trying to make a new @T with that as the vtable, and with s2 as the "hidden object". Last I checked trait objects don't automatically implement their trait (because e.g. with Self it can be impossible), rust-lang/rust#5087.

@emberian
Copy link
Member

emberian commented Aug 5, 2013

I would have assumed it's the identity function

@nikomatsakis
Copy link
Contributor

Updating title to reflect the larger issue (casting from @T to @T is a special case)

@graydon
Copy link

graydon commented Aug 15, 2013

accepted for feature-complete milestone

@bblum
Copy link

bblum commented Aug 22, 2013

This doesn't seem like the identity function to me, as the vtables can be subsets of each other in weird ways (e.g. with multiple supertraits). Doing this properly will require new codegen.

@kvark
Copy link

kvark commented Jan 4, 2014

A simpler testcase:

trait U {}
trait V : U {}
fn foo<'a>(a : &'a V)-> &'a U   {
    a as &U
}

@pnkfelix
Copy link
Member

this need not block 1.0. Assigning P-high as it is important for some use cases.

@nikomatsakis
Copy link
Contributor

To be clear: this is not a subtyping rule but a coercion one. As @bblum says, it may require substituting a new vtable.

@alexcrichton
Copy link
Member

This is on the 1.0 milestone currently, due to @pnkfelix's comment above, I'm removing the milestone assuming that it was an accident.

@little-arhat
Copy link

I'm curious, how we could call supertrait method on subtrait object [1], without being able to upcast such object to supertrait? Does vtable of subtrait include all methods from supertrait? Or how does it work?

[1] — https://github.com/rust-lang/rust/blob/41a79104a412989b802852f9ee6e589b06391d61/src/test/run-pass/trait-inheritance-cast.rs

@little-arhat
Copy link

Is there any way to work around this issue in current rust? transmute can not help here: it seems that transmute messes up with vtables:

trait Foo {
    fn f(&self) -> int;
}

trait Bar : Foo {
    fn g(&self) -> int;
}

struct A {
    x: int
}

impl Foo for A {
    fn f(&self) -> int { 10 }
}

impl Bar for A {
    fn g(&self) -> int { 20 }
}

fn to_foo(x:&Bar) -> &Foo {
    unsafe { std::mem::transmute(x) }
}

pub fn main() {
    let a = &A { x: 3 };
    let abar = a as &Bar;
    let abarasafoo = to_foo(abar);

    assert_eq!(abarasafoo.f(), 20); // g is called 
    assert_eq!(abarasafoo.f(), 10); // g is called

What is layout of trait objects? I think it should be possible to "upcast" subtrait object to supertrait with unsafe manipulation of vtables.

@little-arhat
Copy link

For future references: https://github.com/rust-lang/rust/blob/master/src/librustc/driver/pretty.rs#L137 — here is workaround for this issue.

@arthurprs
Copy link

Sub.

@kFYatek
Copy link

kFYatek commented Feb 22, 2015

My workaround: http://stackoverflow.com/a/28664881/403742

@steveklabnik
Copy link
Member

Triage: https://github.com/rust-lang/rust/issues/5665#issuecomment-31582946 seems to be a decent test-case, and this is still a non-scalar cast today.

@isabelmu
Copy link

isabelmu commented Oct 9, 2016

I've started looking at this. I figured I'd try to clear up the design first. Here's my understanding:

Basics

Trait objects (spelled as &Trait) should be upcastable to trait objects of supertraits (&Supertrait) in as expressions, with lifetime and mutability rules behaving similarly to reference casting.

Smart pointers

Similar conversions should probably be possible for Box<Trait> to Box<SuperTrait>, Rc<Trait> to Rc<Supertrait>, and for other smart pointers. If this can (or needs to) be dealt with separately, I'll defer it for now and maybe create a separate issue, but it seems like the issues around upcasting are the same.

Implementation

Trait objects consist of a vptr and a self-pointer. The vptr points to a vtable, which contains a static array of function addresses. Each method is associated with a statically known offset in that table. To call a method on a trait object, add the offset to the vptr and call the function whose address is stored in that slot, passing the self-pointer as the first argument.

(The vtable also stores some metadata, which currently includes size, alignment, and the address of the destructor. The destructor is treated specially, presumably because any concrete type may implement Drop, and Box<Trait> needs to be able to run the destructor without Trait explicitly inheriting Drop.)

Currently, there is one vtable for each impl of an object-safe trait. There is no particular relationship between different vtables for a type that implements multiple traits. Supertrait method addresses are, however, duplicated into the subtrait's vtable.

To support upcasting, we need a way to get from the subtrait's vtable to the supertrait's vtable. I'm planning on storing supertrait vtables next to subtraits vtables, in a recursive pattern. This would let us use statically known offsets for upcasting, because the offsets are the same for every subtrait impl. The drawback is that superclass vtables would need to be duplicated in a 'diamond' inheritance situation.

(Another consequence is that two trait objects of the same trait could have different vptrs even if they were created from the same concrete type, depending on what path they took up the inheritance chain. This shouldn't matter in practice, since we don't make any guarantees about trait object equivalence.)

An alternative solution would be to store offsets next to each method list, one for each supertrait we can upcast to. This representation is more compact, but I expect it would be slower because of the dependent read.

Questions

  • Should trait object upcasts be explicit (require an as expression) or implicit?
  • Is it OK to duplicate supertrait vtables like I described above?
  • I believe this issue predates the RFC process. Should I file one about any part of this?

@isabelmu
Copy link

isabelmu commented Oct 9, 2016

Looks like RFC 401 answered my first question: this is supposed to be an implicit coercion. See also: rust-lang/rust#18469, the tracking issue for RFC 401, and rust-lang/rust#18600, which deals with subtrait coercions and therefore may be a duplicate of this issue.

RFC 982 (DST coercions) also looks to be related. Tracking issue is rust-lang/rust#18598.

@isabelmu
Copy link

isabelmu commented Oct 9, 2016

RFC pull request 250 (postponed) dealt with this, but alongside proposals for associated fields on traits, opt-in internal vtables, and other features. The upcasting section of that proposal is basically what I had in mind.

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Nov 2, 2016

@typelist sorry for not getting back to you sooner! I have some thoughts about how this should work, but I think the problem can also be subdivided. Likely we should start by writing up an RFC, perhaps just for a subset of the total problem. Let me just leave various notes and let's try to sort it out a bit.

Multiple traits, multiple vtables

First of all, this is related to the topic of supporting trait objects of multiple traits. For example, I would like to support &(Foo + Bar) as a trait object, where Foo and Bar are any two object-safe traits. We already support this is in a very limited way with &(Foo + Send), but that is special-cased because Send does not require a vtable. One would be able to upcast to any subset of those traits. For example, you could upcast from &(Foo + Bar) to &Foo just by dropping one of the vtables.

This is orthogonal from supertrait upcasting, just wanted to throw it out there. In particular, if you have trait Foo: FooBase, then one can imagine permitting an upcast from &(Foo+Bar) to &(FooBase+Bar) and so forth. Similarly, if you had trait Foo: FooBase1 + FooBase2, then you could cast &Foo to &(FooBase1 + FooBase2).

I bring it up though because the implementation strategy here may want to be shared with other trait combinations. Originally, the way I had thought to support this was to have a trait object potentially have >1 vtable (i.e., one for Foo, one for Bar). But of course this is not the only implementation strategy. You could create a "composed" vtable (on the fly) representing Foo+Bar. (These would have to be coallesced across compilation units, like other monomorphizations.) However, to permit upcasts, you would then have to make "subtables" for every supertrait combo -- e.g., the Foo+Bar vtable would also have to have a Foo and a Bar vtable contained within. As the number of traits grows (Foo+Bar+Baz) you wind up with N! vtables for N composed traits. This seemed suboptimal to me now, though I suppose that realistically N will always be pretty darn small.

Note that if we adopted the strategy of making one coallesced vtable, this will affect how supertraits are stored in vtables. If we said that &(FooBase1 + FooBase2) uses two vtables, then the vtable for Foo can just have an embedded vtable for each supertrait. Otherwise, we need to store embedded vtables for all possible combinations (leading again to N! size).

Supertrait upcasting

OK, so, with that out of the way, I think it makes sense to focus squarely on the case of supertrait upcasting where exactly one vtable is needed. We can then extend (orthogonally) to the idea of multiple vtables. So in that case I think the basic strategy you outlined makes sense. You already cited this, but I think that the strategy outlined by @kimundi and @eddyb in RFC 250 is basically correct.

Interaction with the unsize coercion

You are also already onto this one, but yes this is clearly related to the idea of coercing a &T to a &Trait (where T: Trait), which is part of the "unsize coercion". My view is that this is basically added another case to that coercion -- that of supporting &Trait1 to &Trait2 where Trait1: Trait2. And the idea would be to manipulate the vtable(s) from the source reference in such a way as to produce the vtable(s) of the target reference.

Because the vtable must be adjusted, it's clear that this has to be a coercion and not an extension of subtyping, since subtyping would require that there are no repesentation changes. Moreover, by attaching it to the existing unsizing coercion, we don't have to address questions about just when it triggers -- same time as unsizing triggers.

Do we need an RFC to go forward here?

It seems like it can't hurt, but I'd basically just extract the relevant text from RFC 250, which I think was essentially correct. If we're just sticking to supporting single trait object to other trait object, seems harmless enough, and I can't imagine many objections. I would be very happy to work with you and draft it together!

@isabelmu
Copy link

isabelmu commented Nov 8, 2016

Thanks for the response! I started drafting an RFC based on part of RFC 250. Comments or revisions welcome. (Should we keep communicating through this issue, or some other way?)

For the multi-trait case, increasing the size of the trait object does seem like the path of least resistance. It certainly makes partial upcasts (and upcasts like &Subtrait to &(Supertrait1 + Supertrait2)) straightforward.

@nikomatsakis
Copy link
Contributor

@typelist this RFC looks great! I appreciate that you took the time to talk about the motivation, alternatives and downsides.

(We can keep talking over the issue, or feel free to e-mail me or ping me on IRC. I try to keep up with GH notifications, but it can be challenging.)

@qnighy
Copy link

qnighy commented Jul 18, 2017

@typelist @nikomatsakis Any updates on this? I'd like to push this forward.

I took a stat of vtable usage in rustc. Perhaps it is useful when predicting code-size/performance effects of upcasting support. https://gist.github.com/qnighy/20184bd34d8b4c70a302fac86c7bde91

@ctaggart
Copy link

If this was implemented, would this compile? If so, this would be extremely helpful. For example, with TypeScript interop, I could avoid all the ugly Box::from calls.

trait Statement {}

trait IfStatement: Statement {}

struct IfStatementImpl;
impl Statement for IfStatementImpl {}
impl IfStatement for IfStatementImpl {}

fn print_statement(_statement: &Statement){
    println!("print_statement");
}

fn print_if_statement(_if_statement: &IfStatement){
    println!("print_if_statement");
}

fn main() {
    // How come this compiles?
    let ref a = IfStatementImpl {};
    print_statement(a);
    print_if_statement(a);

    // but this does not?
    let ref b: IfStatement = IfStatementImpl {};
    print_statement(b);
    print_if_statement(b);
}

@qnighy
Copy link

qnighy commented Jan 20, 2018

@ctaggart I think 50% yes. let ref b: IfStatement = IfStatementImpl {}; won't yet compile (because it tries to coerce IfStatementImpl into IfStatement and then take a reference of it), but if you change it to let b: &IfStatement = &IfStatementImpl {}; (take a reference and then coerce &IfStatementImpl into &IfStatement), then sub-trait coercion would allow it to compile.

@bstrie
Copy link
Contributor

bstrie commented Nov 14, 2018

@rust-lang/lang, IRC suggests that this is something that should be closed or moved to the RFCs repo.

@Mark-Simulacrum Mark-Simulacrum transferred this issue from rust-lang/rust Sep 18, 2019
@jonas-schievink jonas-schievink added the A-trait-object Proposals relating to trait objects. label Nov 17, 2019
@jonas-schievink jonas-schievink added the T-lang Relevant to the language team, which will review and decide on the RFC. label Nov 17, 2019
@burdges
Copy link

burdges commented Apr 19, 2020

@joshtriplett
Copy link
Member

@rust-lang/lang talked about this issue today. Allowing for evolution of Rust syntax since pre-1.0, to the best we can tell, this is something that you can now do. Closing; please comment if there's still an issue here.

@kennytm
Copy link
Member

kennytm commented Sep 8, 2020

Allowing for evolution of Rust syntax since pre-1.0, this is something that you can now do

do you mean "that you cannot do" 😕. also this issue doesn't seem to have any relationship with syntax.

@Lokathor
Copy link
Contributor

Lokathor commented Sep 8, 2020

Because of the ancient syntax, it's unclear what's being asked for.

The 4 people in the meeting who looked at this all looked at the example code and understood the request to be about casting a reference to a struct type into a reference to a trait type (a trait that the struct type implements).

If the request is about turning a sub-trait reference into a super-trait reference that is indeed not currently possible.

However, since the example code only has one trait and one struct, we did not think that the sub-trait/super-trait matter was involved here.

@scottmcm
Copy link
Member

scottmcm commented Sep 8, 2020

As one of those 4 people, my first thought was the same as yours, @kennytm -- that from the issue header this was about casting one dyn Trait to another one, which certainly is not currently possible.

I guess the later examples (#2765 (comment)) are that, but the one in the OP currently isn't.

I'm not sure what the general intent of issues in this repo is these days. So it doesn't really bother me whether it's open or closed...

@kennytm
Copy link
Member

kennytm commented Sep 8, 2020

@Lokathor the subtrait/supertrait discussion starts from #2765 (comment). In any case the wording above is not clear enough to tell what is going on — because "this is something you can now do" so the issue (which does not mention changing syntax at all) is closed??? can't comprehend.

(To clarify: I'm not saying we should re-open this issue, I'm saying that comment is hard to understand.)

@jdm
Copy link
Author

jdm commented Sep 8, 2020

The original question was about casting a trait reference to the same trait or another trait that should be part of the typeat that point.

@thanatos
Copy link

I'm so confused. I'm coming here from rust-lang/rust#18600 (comment) which closes itself as a duplicate of this one. But the discussion here makes it sound like it isn't a duplicate?

This issue is also referenced by rust-lang/rust#18469

@joshtriplett
Copy link
Member

@scottmcm wrote:

I guess the later examples (#2765 (comment)) are that, but the one in the OP currently isn't.

I think that was exactly the issue: the example in the OP looked like something that's now possible, and it wasn't obvious that this issue was asking for something else.

@joshtriplett joshtriplett reopened this Oct 5, 2020
@joshtriplett
Copy link
Member

Given the amount of time that has passed, and the amount of syntax evolution, could someone involved in the original issue please either point to a current issue that's requesting the same thing using current syntax, or file one?

@jdm
Copy link
Author

jdm commented Oct 5, 2020

Kvark's earlier example still doesn't compile:

trait U {}
trait V : U {}
fn foo<'a>(a : &'a dyn V) -> &'a dyn U {
    a as &dyn U
}
error[E0605]: non-primitive cast: `&'a (dyn V + 'a)` as `&dyn U`
 --> src/main.rs:4:5
  |
4 |     a as &dyn U
  |     ^^^^^^^^^^^ invalid cast
  |
help: borrow the value for the cast to be valid
  |
4 |     &a as &dyn U
  |     ^

error[E0277]: the trait bound `&'a (dyn V + 'a): U` is not satisfied
 --> src/main.rs:4:5
  |
4 |     a as &dyn U
  |     ^ the trait `U` is not implemented for `&'a (dyn V + 'a)`
  |
  = note: required for the cast to the object type `dyn U`

error: aborting due to 2 previous errors

@DarrienG
Copy link

I actually made a post about this on reddit confused why this wasn't working a few days ago here. If you're looking to collect more examples of confusion, here's another one.

@burdges
Copy link

burdges commented Oct 25, 2020

As I noted above, one could develop this outside rustc+core using https://internals.rust-lang.org/t/casting-families-for-fast-dynamic-trait-object-casts-and-multi-trait-objects/12195 You'll need to manually insert compilation units that unify your casting families however.

@nikomatsakis
Copy link
Contributor

Side note that I was thinking that I would like to mentor someone through the implementation steps required here. I think that's the primary blocker. Perhaps I will file a lang team proposal around it first though.

andyyu2004 added a commit to andyyu2004/bit that referenced this issue May 6, 2021
create explicit struct reading pack indexes
use generics rather than trait objects as casting from &Trait to &SuperTrait is not implemented (rust-lang/rfcs#2765)
and its very inconveninent to not be able to pass `&mut BufReadSeek` to a function that expects a `&mut BufRead`.
Furthermore, the main reason we were using dynamic dispatch in the first
place was just to make certain traits (Serialize & hence BitObj) object safe. I think Deserialize was rewritten to use dyn to be consistent with Serialize.
@crlf0710
Copy link
Member

Current tracking issue for this is rust-lang/rust#65991

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-trait-object Proposals relating to trait objects. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests