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

Trait based inheritance #223

Closed
wants to merge 6 commits into from

Conversation

Projects
None yet
@gereeter
Copy link

commented Sep 1, 2014

This is largely based upon #9 and #91, but fleshed out to make an actual inheritance proposal.

Moreover, in comparison to other proposals for inheritance, the design should work
well with existing Rust features and follow Rust's philosophies:

* There should be no new ways to acheive the same behavior. For example, virtual calls

This comment has been minimized.

Copy link
@pczarn

pczarn Sep 1, 2014

s/acheive/achieve/

same pointer as the pointer to the upcasted object. This allows not just upcasting a single
object, but also upcasting an array of objects, all without any special computation.

To achieve this, a `#[first_field]` attribute is introduced, which is applied to some field of

This comment has been minimized.

Copy link
@nrc

nrc Sep 1, 2014

Member

There is already an attribute (crepr or something similar) which forces the fields in a struct to be stored in the order they are specified. Would that be enough or do you still need first_field?

This comment has been minimized.

Copy link
@gereeter

gereeter Sep 1, 2014

Author

That would be enough, but it would also be overkill - it would still be nice for the compiler to be free to reorder the rest of the fields.

@rpjohnst

This comment has been minimized.

Copy link

commented Sep 5, 2014

👍 This is how Rust inheritance should work. This level of flexibility could even allow for bindings with other object models- various C++ layouts (potentially even with multiple inheritance), COM, etc.

@zwarich

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2014

How will safe downcasting work with lifetime parameters? In the example, Node has no lifetime parameter but TextNode and ElementNode do. As described, it seems that downcasting with transmute will just introduce a new lifetime parameter out of thin air, which isn't correct.

For downcasting to be safe it seems that all members of a class hierarchy need to have identical lifetime parameters, the copy of a parent class in a child class needs to be instantiated with the same parameters as the child, and the downcasting operation needs to respect these parameters.

@gereeter

This comment has been minimized.

Copy link
Author

commented Sep 6, 2014

I believe the issue with these lifetimes that seem to pop out of thin air was fixed by #192. Basically, because TextNode<'a> only implements Extend<NodeData>, not Cast<NodeData>, there is no way to convert from a TextNode<'a> to NodeData in an owned context. The are only two ways to get NodeData from a TextNode<'a>. First, when stored behind a reference, the two representations can be switched. However, this is perfectly safe, as the text must outlive the reference. Second, the TextNode<'a> could be converted to a trait object of Node. Due to #192, this must also be safe, as the trait object must be annotated with an explicit lifetime bound.

I'll try to update the examples to make this more clear.

@CloudiDust

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2014

+1 to this proposal.

As pointed out in the proposal, the traits here can be the low level building blocks, providing maximum flexibility, while commonly used inheritance patterns can be codified in macros. (Sort of like Ecmascript 6's class syntax vs the underlying prototype-based OO which is both more flexiable and more "alien".)

It will make people think twice before using inheritance, simply because the relative verbosity and the "ugliness" of macro invocations compared to dedicated syntax.

I consider this a plus and don't think we need to artifically limit inheritance in any other way. (IMHO, #142 needs limitations partly because the syntax there is too lightweight and pretty. That can be both an advantage and a disadvantage.)

that something of type `A` can be converted to something of type `B` safely with a
simple `transmute`. To actually use this ability, a function is added to the standard
library as follows:

This comment has been minimized.

Copy link
@bluss

bluss Sep 7, 2014

Cast is almost the wrong name for the trait, because casting in both Rust and C is a conversion preserving the value, not something like a blind transmute.

@zwarich

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2014

@gereeter That approach doesn't work with multiple lifetime parameters, does it? Also, what about non-lifetime type parameters?

@gereeter

This comment has been minimized.

Copy link
Author

commented Sep 7, 2014

@zwarich The same approach should work fine for multiple lifetime parameters: in the trait object case, the object would have to have all the lifetime bounds, and in the reference case, any reference would have to outlive all lifetimes involved.

For non-lifetime type parameters, the parameter would be part of the RTTI.

}
```

## Summary example

This comment has been minimized.

Copy link
@nrc

nrc Sep 8, 2014

Member

For easy comparison with the other proposals for handling DOM-data structures, could you show how https://gist.github.com/jdm/9900569 would look encoded with this proposal please?

If no impl is specified for a given parameterized type, the identity impl (`A: Cast<A>`) is assumed.

To utilize this system for upcasting, we introduce another trait, which encodes the subtype or
starts-with relationship. The bound `A: Entend<B>` represents that statement that, when viewed

This comment has been minimized.

Copy link
@jdm

jdm Sep 8, 2014

s/Entend/Extend/

@brson brson assigned alexcrichton and unassigned nrc Sep 9, 2014

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Sep 11, 2014

@gereeter, could you add an example of how https://gist.github.com/jdm/9900569 might be encoded with this RFC? The example at the end looks similar, but not quite the same.

@alexcrichton alexcrichton reopened this Sep 11, 2014

@alexcrichton alexcrichton force-pushed the rust-lang:master branch from 6357402 to e0acdf4 Sep 11, 2014

fn dump<'a>(node: NodeBox<'a>) {
if let Ok(text_node): Option<&TextNodeBox<'a>> = downcast_copy(node) {

This comment has been minimized.

Copy link
@ben0x539

ben0x539 Sep 12, 2014

Is this cleverly crossborrowing node into a shared reference? (Probably should be Some(...), not Ok(...))

@nielsle

This comment has been minimized.

Copy link

commented Sep 13, 2014

Bikeshedding. Could the following syntax work? Here @Coerce should "magically" ensure that the layout is fixed.

@coerce(Node)
struct MyNode { node: Node,  .... }

Perhaps this could open the door for future self inspection syntax..

@coerce([f32,.. 3])
struct MyVec { x1: f32, x2:f32, x3:f32 }

let xs = MyVec {x1:0, x2:0, x3:0}
for x in cast(xs).iter() { .....}

```
impl <U, T> Cast<Bundle<U>> for Bundle<U, T> {}
impl <U: Cast<U2>, T: Extend<T2>> Cast<Bundle<U2, T2>> for Bundle<U, T> {}

This comment has been minimized.

Copy link
@glaebhoerl

glaebhoerl Sep 14, 2014

Contributor

Is this correct? It seems like it should either be T: Cast<T2> rather than Extend or impl Extend rather than Cast. (T: Extend<T2> does not imply that Bundle<U2, T2> and Bundle<U, T> have the same representation, which is what impl Cast is stating.)


## Downcasting (safe RTTI)
The only item left on the list of requirements for inheritance is the problem of downcasting. To deal with this,
yet another trait is introduced, `Typed`. This trait has two properties that make it unique:

This comment has been minimized.

Copy link
@glaebhoerl

glaebhoerl Sep 14, 2014

Contributor

This is essentially the same thing as the existing Any trait1.

1 Except that as the current Any is implemented, it is known to be impled by all types T: 'static, which is obviously wrong.

This comment has been minimized.

Copy link
@huonw

huonw Sep 15, 2014

Member

obviously

It's not to me, could you explain in a little more detail?

This comment has been minimized.

Copy link
@glaebhoerl

glaebhoerl Sep 15, 2014

Contributor
fn evil_id<T: 'static>(mut arg: T) -> T {
    if let Some(an_int) = (&mut arg as &mut Any).downcast_mut() {
        *an_int = 666i;
    }
    arg
}

I shouldn't be able to do this. I should have to specify T: Any. (I should be infer from the absence of an Any bound that the function won't do something like this. 'static should only mean 'static.)

This comment has been minimized.

Copy link
@reem

reem Sep 16, 2014

'static should mean 'static and impl<T: 'static> Trait for T should also mean that you always get Trait when you have 'static - I see no reason why this is at all unreasonable.

This comment has been minimized.

Copy link
@glaebhoerl

glaebhoerl Sep 16, 2014

Contributor

I agree with what you wrote, but I was never implying otherwise.

The unreasonable part is having impl<T: 'static> Any for T in particular. We should have compiler-generated impls for each type individually, but not a blanket impl for all types. The Any bound should be satisfiable by any concrete type, but not by abstract type parameters except if one explicitly writes T: Any. (As this RFC also states.)

This comment has been minimized.

Copy link
@reem

reem Sep 16, 2014

I'm still unsure why this actually needed.

Wouldn't

impl <T: 'static> Typed for T {
   static TYPE_ID: TypeId::of::<Self>()
}

do the same job as an impl for all types? I'm not clear on why it's important that I have to write specifically T: Typed.

This comment has been minimized.

Copy link
@glaebhoerl

glaebhoerl Sep 16, 2014

Contributor

So that contracts are explicit and type signatures actually mean something. If we allow dynamically casting any abstract type to any other type, then a function requiring T: 'static could do just about literally anything (see above evil_id). It should be possible to know that a generic function only manipulates / makes use of its type parameters in accordance with the trait bounds which it specifies. I don't know how else to rephrase this to make it clear why this is desirable.

This comment has been minimized.

Copy link
@reem

reem Sep 16, 2014

Does this mean you are proposing we should remove Any?

This comment has been minimized.

Copy link
@glaebhoerl

glaebhoerl Sep 16, 2014

Contributor

No, I just think it should be implemented differently. I would prefer (as, again, this RFC we're commenting also proposes) to remove the blanket impl<T: 'static> Any for T impl (it's actually AnyPrivate but that's unimportant), and instead, whenever you write:

struct Foo;

the compiler automatically derives:

impl Any for Foo { ... }

* Everything implements `Typed`. However, this is not "obvious" to the type system - although all *concrete* types
implement `Typed`, it cannot be inferred that a generic type variable implements `Typed`.
* Instead of having methods, the "virtual table" for `Typed` is type information. This type information should be

This comment has been minimized.

Copy link
@glaebhoerl

glaebhoerl Sep 14, 2014

Contributor

This is essentially saying that Typed would have an associated static rather than associated fns:

trait Typed {
    static TYPE_ID: TypeId;
}

Associated statics are part of the proposal for associated items by @aturon.

To use this type information, three functions are exposed that implement downcasting:

```
fn is_instance<A: Cast<B>, B: Typed>(value: &B) -> bool {

This comment has been minimized.

Copy link
@glaebhoerl

glaebhoerl Sep 14, 2014

Contributor

Unless I'm missing something, I believe all of these also need to require A: Typed.

Edit: Also, is the Cast requirement buying us anything here? As far as I can tell it's enforcing that the casts be strictly _down_casts, but I don't see why we couldn't or shouldn't allow dynamically-checked casts between arbitrary pairs of types. (The _down_cast restriction can be enforced by clients further out by requiring Cast themselves, if that's what they want to do.)

This comment has been minimized.

Copy link
@gereeter

gereeter Sep 16, 2014

Author

The Cast requirement is there to ensure that it is even possible for the downcast to succeed - since the actually transformation is done via a transmute, the only way it could possibly be safe is if there is a Cast relationship. Technically, it isn't needed, but there isn't any reason not to throw out downcasts that will never succeed.

I believe you are correct about A: Typed.

* Instead of using a `#[first_field]` attribute, one could write `struct Child: Parent` and have the compiler automatically
add a `super` field that is placed first.
* Instead of using a `#[first_field]` attribute, the compiler could just detect a special field name like `super` and declare
that to be the first field.

This comment has been minimized.

Copy link
@glaebhoerl

glaebhoerl Sep 14, 2014

Contributor

I think this variation would be preferable (we could also use super as a modifier). Generally attributes are at least supposed to be "only metadata"; if it has an effect on which things typecheck, then it's probably better for it to be a first-class language construct.

* Rename `Cast` to `Coerce`, `Coercible`, `Transmute`, `SameRepr`, `Convert`, or `Upcast`.
* Rename `Extend` to `HasPrefix` or `StartsWith`.
* Rename `Bundle` to `Fat`, `Thin`, or `BehindPointer`.
* Rename `Typed` to `HasType`, `Typable`, or `RTTI`.

This comment has been minimized.

Copy link
@glaebhoerl

glaebhoerl Sep 14, 2014

Contributor

My preferences here would be:

#[first_field]   -> super
mem::transmute() -> mem::force_transmute()
Cast/Coercible   -> Transmute
cast()/coerce()  -> transmute()
HasPrefix/Extend -> Extends
Any/Typed        -> Dynamic

Rationale:

  • (#[first_field] vs. super: See above.)
  • The word Rust uses for no-op bit-level reinterpretations is "transmute". So it makes sense to use this word here, because that's what we're doing.1 The safety of the plain transmute() function would be assured by the presence of Transmute impls, while the current unrestricted, unsafe version would be renamed to e.g. force_transmute().
  • Using the word "extend" here is a great idea! I think Sub: Extends<Super> reads better.
  • It makes tons of sense for the trait used to obtain dynamic typing to be called Dynamic. And e.g. Box<Dynamic> reads very well.

1 The earlier name Coercible has already mislead some people into thinking that it has something to do with "coercions", by which Rust means "automatic compiler-inserted non-representation-preserving conversions", when in fact it doesn't.

This comment has been minimized.

Copy link
@vadimcn

vadimcn Sep 15, 2014

Contributor

Why super and not struct Child: Parent? The latter would rhyme better with trait inheritance, IMHO.

This comment has been minimized.

Copy link
@huonw

huonw Sep 15, 2014

Member

If it were super field_name: Parent (I assume this is what @glaebhoerl meant?) then it's easy to explicitly access things using the parent (this isn't necessary a good thing in all circumstances), and you don't get any 'surprise'/implicit fields being introduced.

This comment has been minimized.

Copy link
@glaebhoerl

glaebhoerl Sep 15, 2014

Contributor

I assume this is what @glaebhoerl meant

Yes. Basically I like this option because it's the most minimal one (apart from an attribute, which I think is too minimal) which accomplishes what we need.

We could have something like struct Child: Parent, but that has other implications which are out of scope with respect to our goals here. If it's just e.g. sugar for lifting the fields of Parent directly into Child without implying subtyping voodoo, then I wouldn't necessarily mind having it, but I think it would be best discussed as a separate proposal.

This comment has been minimized.

Copy link
@vadimcn

vadimcn Sep 15, 2014

Contributor

@huonw: I would assume you'd just access them via self, same like parent methods under trait inheritance. I doubt that anyone who'd programmed in the last 20 years would be surprised. Besides, if Parent fields have to be accessed via super, what happens with deeper hierarchies? self.super.super.super.super.field ?

@glaebhoerl, can you please give an example of other implications of strict Child: Parent?

This comment has been minimized.

Copy link
@glaebhoerl

glaebhoerl Sep 16, 2014

Contributor

Well... I already did. It would also force us to think about e.g. how to handle name shadowing of fields, which is a complication that doesn't currently exist. And the struct Child: Parent syntax would lead people to believe it also enables things like passing &Child where &Parent is expected, which is a whole other can of worms.

The point is only that none of this is necessary for the purposes of this RFC. All that we need here is for Child: Extend<Parent> to be satisfied, and all that we need for that is for the physical layout in memory of the Child struct to begin with Parent, and all that we need for that is to be able to specify which field comes first in the struct. We don't otherwise need to change how existing structs already work in any other way.

But this is really a minor detail of the design and I don't care that strongly about it, so I'm not going to argue it further.

This comment has been minimized.

Copy link
@reem

reem Sep 16, 2014

How about just a straight rename of #[first_field] to #[super] with no additional semantics?

This comment has been minimized.

Copy link
@glaebhoerl

This comment has been minimized.

Copy link
@vadimcn

vadimcn Sep 22, 2014

Contributor

Well... I already did. It would also force us to think about e.g. how to handle name shadowing of fields, which is a complication that doesn't currently exist.

Don't we already have this problem with traits? And it must have been resolved somehow, so we could do the same here.

And the struct Child: Parent syntax would lead people to believe it also enables things like passing &Child where &Parent is expected, which is a whole other can of worms.

This RFC already allows for that, though requires an explicit cast. Or am I reading it wrong? Implicit casts would be a backwards-compatible addition, so they could be done later.

BTW, I totally agree with attributes being "too optional" to be part of a major language feature like this.

To support zero cost upcasting, it is important for the data stored in a superclass to come
before any of the subclass's data, so that the pointer to the subclass object is exactly the
same pointer as the pointer to the upcasted object. This allows not just upcasting a single
object, but also upcasting an array of objects, all without any special computation.

This comment has been minimized.

Copy link
@huonw

huonw Sep 15, 2014

Member

An array of pointers to objects, I assume?

This comment has been minimized.

Copy link
@gereeter

gereeter Sep 16, 2014

Author

Probably - I could image a situation where substructs are used to add methods and not data and you could store everything by value, but pointers will be the common case. Also, note that it doesn't have to be just a pointer to an "object" as would be enforced by other proposals - there could be more data associated with the pointer, such as if a choice is made to use fat pointers.

impl <T> Cast<[T, ..1]> for T {}
impl <T> Cast<T> for [T, ..1] {}
impl Cast<int> for uint {}

This comment has been minimized.

Copy link
@huonw

huonw Sep 15, 2014

Member

Do we actually want -128i8 to magically turn into 128u8? Also, would something like Extend<u8> for u16 be sensible (it seems nearly as semantically sensible as u8 <-> i8, modulo endianness)?

This comment has been minimized.

Copy link
@glaebhoerl

glaebhoerl Sep 15, 2014

Contributor

It's not really "magical" if you invoke cast() (or transmute(), whatever we name it) explicitly. This kind of thing is the whole point.

This comment has been minimized.

Copy link
@huonw

huonw Sep 15, 2014

Member

Well, the cast could be nested deep inside some other data structure (e.g. HashMap<..., Vec<(uint, u8)>>) so it may not be immediately obvious what's happening, but yes, I take your point.

This comment has been minimized.

Copy link
@gereeter

gereeter Sep 16, 2014

Author

It may not be immediately obvious what is happening, but the ability to convert HashMap<..., Vec<uint, u8>> to HashMap<..., Vec<int, i8>> could in fact be very helpful.

* This proposal is very large. This is somewhat a side effect of trying to make everything
useful for even Rust code that doesn't touch inheritance.
* The RTTI may not be as efficient as it could be. This section is the least well thought out
section of the whole proposal, and may require O(n) processing of type information. However,

This comment has been minimized.

Copy link
@huonw

huonw Sep 15, 2014

Member

What is the n here?

This comment has been minimized.

Copy link
@gereeter

gereeter Sep 16, 2014

Author

Number of classes in the inheritance chain between the actual class and the class you want to cast to, or total number of classes in the inheritance chain if the downcast fails, I think. I'm not certain about this, but I remember reading that some implementations of RTTI, to support open inheritance, use a linked list of classes that has to be linearly scanned to find if the target class is present.

@aturon aturon force-pushed the rust-lang:master branch from 4c0bebf to b1d1bfd Sep 16, 2014

@netvl netvl referenced this pull request Sep 20, 2014

Closed

Associated field inheritance #250

@nwin

This comment has been minimized.

Copy link

commented Sep 21, 2014

How about renaming #[first_field] to #[parent]? The RFC seems to be conceptional close to Self where parent objects are mostly saved in a slot called parent. So it would just seem natural to mark the parent object with #[parent].

@vadimcn

This comment has been minimized.

Copy link

commented on active/0000-trait-based-inheritance.md in a8034be Sep 22, 2014

... or VTable ?

@CloudiDust CloudiDust referenced this pull request Sep 22, 2014

Closed

Layout Inheritance #254

@brson

This comment has been minimized.

Copy link
Contributor

commented Sep 23, 2014

The consensus among the core team is that while this is an orthogonal set of abstractions, in actual use they do not present an ergonomic user experience.

Discussion

@brson brson closed this Sep 23, 2014

wycats pushed a commit to wycats/rust-rfcs that referenced this pull request Mar 5, 2019

Merge pull request rust-lang#223 from emberjs/locks-patch-2
Add `isEqual` named export to`@ember/utils`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.