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

Associated field inheritance #250

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
@Kimundi
Member

Kimundi commented Sep 20, 2014

Yet another servo DOM proposal, by me and @eddyb

This Proposal is about solving the servo DOM design requirements through a combination of a few orthogonal features and extensions that are useful on their own:

  • Allowing upcasting of a trait object
  • Extend trait objects to include associated fields that map to fields of the implementing type
  • Allow opt-in use of an internal vtable for an trait object hierarchy.
  • Provide composition sugar for struct fields, and make it usable for associated fields too.
  • Ability to call the default implementation of a trait method, even if a implementer overrides it.
  • Ability to override associated items of a trait with default impls in a sub trait.

By combining them in the right way, its possible to model something that behaves similar to an inline-vtable single-inheritance OOP system with trait objects.

Rendered view

@Kimundi Kimundi changed the title from Open associated-field-inheritance RFC to Associated-field-inheritance Sep 20, 2014

@Kimundi Kimundi changed the title from Associated-field-inheritance to Associated field inheritance Sep 20, 2014

@CloudiDust

This comment has been minimized.

Contributor

CloudiDust commented Sep 20, 2014

I'll admit I cannot tell if the details perfectly fit with each other or not, and if there are unconsidered corner cases or not. But this proposal feels right.

All the components can be handy even if we do not use them for inheritance. Actually, even if we eventually go with the other inheritance proposals, I can still see people proposing these features separately anyway, and may accidentally create multiple ways to do inheritance in Rust, which is undesirable. So why not consider them as a whole and use them for inheritance? Even if we decide to reject this, at least we will know which of the components we can adopt, and which we cannot. Win.

In my eyes, this proposal makes traits somewhat more like Scala's, and in a good way.

Field mapping is a clever way to avoid possible name conflicts between the implemented traits, and more importantly, make the name choices local and explicit to the implementer. One thing that I don't like about Scala's traits is how their orders in the declaration affect name resolution. And Rust resolves this by always requiring the programmer to explicitly state his/her intention, even go as far as disabling methods if the trait is not in scope. It may seem verbose at times, but I love it. There is less "global state" to track. And to me, field mapping is just following this fine tradition, if a language that hasn't even hit 1.0 can be said to have a tradition, that is. ;)

}
struct MyTuple(uint);
impl Foo for MyStruct {

This comment has been minimized.

@Veedrac

Veedrac Sep 20, 2014

impl Foo for MyTuple?

@arcto

This comment has been minimized.

arcto commented Sep 20, 2014

Associated fields seem nice but I'd like to mention one point where data hiding could be improved:

Users of a trait, as well as default impls, might use fields as if they were plain values. But it's impossible for a trait to know if an implementation needs dynamic behaviour for access or storage of a property.

Therefore associated fields could/should be mappable to getters and setters. It's not just an ergonomic issue. The trait is an abstract description of a type - it wouldn't know requirements of implementations.

Some uses of accessor functions are: hide internal representation, specialised representation, serialisation, lazy loading, lazy evaluation.

Of course, there's one glaring unanswered question: can you take the address of such a field?

@CloudiDust

This comment has been minimized.

Contributor

CloudiDust commented Sep 20, 2014

@arcto: I think associated fields is for one use case and one use case only: where we need to access common bare fields without any magic.

If we need accessor functions, the Rust way would be "just define functions in the traits".

The cost of a function call is not what Rust wants to hide, because its systems language nature. This is also why Rust does not have C#-like properties.

Scala, on the other hand, adheres to the "uniform access principle", which is very nice, but not suitable for Rust.

As they are just plain fields, I don't see why we cannot take their addresses.

@CloudiDust

This comment has been minimized.

Contributor

CloudiDust commented Sep 20, 2014

On the topic of a "Scala-like" (very broadly speaking) proposal, I feel the need to cc @netvl.

@arcto

This comment has been minimized.

arcto commented Sep 20, 2014

Well @CloudiDust, the problem as I see it is that the trait cannot know what an implementation needs. To require a plain value in the implementing type is severely limiting, IMHO. You'd exclude all possible implementations that need to do a calculation to provide a value, for example a lazy eval.

@CloudiDust

This comment has been minimized.

Contributor

CloudiDust commented Sep 20, 2014

@arcto I think that bare fields is very suited for "closed inheritance", in that all implementors will be confined to the same crate/module along with the traits, and it is OK to access the fields directly - like in a DOM or AST structure.

"Open ended" traits (so to speak), on the other hand, will not contain associated fields, and is intended for implementation by a wide variety of types. (That is, the traits we have today.)

The beauty of this solution is, it adds a feature that is actually suitable for a specific use case, but instead of feeling bolted on, it feels very natural. (We have/are going to have associated types/functions/constants on traits, why not fields?)

@CloudiDust

This comment has been minimized.

Contributor

CloudiDust commented Sep 20, 2014

@Kimundi @eddyb Also I think you should put a somewhat more detailed summary in the PR comment. "Yet another servo DOM proposal" is not doing your RFC justice. :)

@netvl

This comment has been minimized.

netvl commented Sep 20, 2014

Frankly, I just couldn't manage to read and/or understand other proposals in full. For example, all proposals about unifying structs and enums look very complex for little gain. I don't know of any language which does something like that, and this certainly would be a barrier for newcomers to Rust. It is a massive change which increases complexity of the language severely.

Proposals which model inheritance via traits, like #223, look much better. They are very lightweight and orthogonal, base on existing set of features and do not complicate the language much.

This proposal also looks pretty lightweight, though it is still more complex than trait-based inheritance, but I'm not sure how I feel about some things like field ordering when using associated fields. Other than that it does look like a very nice alternative providing useful features.

Associated fields do remind me of Scala, though in Scala there really are no "fields". vals and vars there are implemented in terms of methods internally and rely on JVM virtual dispatch - for example, it is possible to override a var with a pair of getter/setter methods, so they are a different thing than associated fields here.

@tbu-

This comment has been minimized.

Contributor

tbu- commented Sep 20, 2014

One other way of providing upcasting would be (at the cost of one dereference), to store the parent traits' vtable pointers directly in the vtable of the trait, so the vtable would contain this:

  • Drop glue
  • Pointer to the supertraits' vtables for the type
  • Associated items
### Associated fields Alternatives
- Deal with abstraction over data as today: Getters, Setter, and the need of virtual calls for trait objects.
- Make mapping not freely choosable, but rather require appropriately named fields to be present in the struct (possibly in the right order as well)

This comment has been minimized.

@rpjohnst

rpjohnst Sep 20, 2014

The most common use of associated fields will likely be to delegate them to a TraitData field defined alongside the trait:

trait Foo {
    foo_data: FooData,
}

struct FooData {
    x: i32,
    y: i32,
}

struct MyFoo {
    foo_data: FooData
}

impl Foo for MyFoo {
    foo_data => self.foo_data
}

(Alternatively, the FooData fields could be used directly as associated fields and all forwarded individually, but that's even more verbose).

Ideally the default would reduce the boilerplate to specifying the required associated fields in exactly one place, and then declaring that the type wants them in exactly one place. And at that point, there's really no point in explicit mappings.

This comment has been minimized.

@CloudiDust

CloudiDust Sep 20, 2014

Contributor

I think two use cases of explicit mapping would be "overlapping fields" (two fields in different traits map to the same one in the struct) and avoiding field name clashes (not necessary with fields in other traits, but also the struct's own fields). But the most common case should be more ergonomic.

Admittedly there are more complexity in implementation. I do wonder if these use cases are better solved by other means. But I like that the implementer is completely free to choose local names.

This comment has been minimized.

@rpjohnst

rpjohnst Sep 21, 2014

Overlapping fields would be better served by having just one trait require the field and having the other trait depend on it (or have them both depend on a third trait with that field). Besides, that's precisely what virtual inheritance does in C++ and the conventional wisdom there is just to fix your design so you don't need it.

Further, while multiple inheritance can be used judiciously, that's quite rare and virtual inheritance even more so. Worse, its implementation details are an absolute nightmare that. Despite being unopposed to MI in general, I am absolutely opposed to anything that complex in the language itself.

Associated fields ought to be very rare and multiple inheritance of them even more so, but if we need to resolve name clashes at all maybe it could be done with a use in the impl? That would be generalizable to method name clashes too, and potentially even let us reuse existing methods directly in impls.

This makes it somewhat harder to find out where a given associated items impl comes from
### Overridable default items Alternatives

This comment has been minimized.

@rpjohnst

rpjohnst Sep 20, 2014

Do we really need overrideable defaults? We do currently have default methods in traits, but introducing multiple arbitrarily-deep overrides seems like needless complexity. It shouldn't be very common, and if it's really needed it could be done explicitly with a generic function called from all the leaf types that want the default behavior.

This comment has been minimized.

@CloudiDust

CloudiDust Sep 20, 2014

Contributor

@rpjohnst The last line can also be said for the currently supported unoveridable defaults. I think wise use of overriding can be a more intuitive choice. But of course, there will be more implementation complexity in the compiler, so we should see if it is worthy. Also, will this feature somehow help easing C++ interop?

This comment has been minimized.

@rpjohnst

rpjohnst Sep 21, 2014

The complexity it adds is not worth anything IMO. My suggested workaround generates exactly the same code (modulo one function inline) and makes everything more explicit without adding any verbosity.

As for C++ interop, this shouldn't make any difference- calling C++ from Rust or vice versa already has all the functions and vtables defined, whether with multiple layers of overrides or the workaround I suggested, so the calling side wouldn't be implementing any defaults, let alone overriding them.

This comment has been minimized.

@eddyb

eddyb Sep 21, 2014

Member

The problem is that it ends up being common when implementing certain patterns.
See libsyntax's fold.rs and visit.rs.
I don't like using a keyword for this myself, especially for the initial implementation - attributes have less dire long-term consequences.

This comment has been minimized.

@Kimundi

Kimundi Sep 22, 2014

Member

Also, overridable methods is pretty much a design requirement for the servon DOM use case.

@rpjohnst

This comment has been minimized.

rpjohnst commented Sep 20, 2014

This feels pretty over-complicated, with a lot of extra attributes and unnecessary details that might limit the language in the future.

I like the idea of associated fields, if they could be made less verbose and complex. I would prefer to do away with field offsets in the vtable- that feels too redundant to me. It may be slightly faster than virtual accessors, but if you really need that performance and multiple inheritance at the same time you may as well do it yourself with unsafe, although pointers to member fields might make that eaiser. I would also prefer to drop #[repr(fixed)] but I'm not sure how without breaking single-inheritance trait-object field access.

It would also be nicer to get rid of #[repr(internal_vtable)] with its required leading associated field, especially if your Vtable<T, Sized? Tr> could be used to implement a Fat<T>-style system instead. That way vptrs could be potentially placed in other locations for e.g. COM interop (maybe with a #[repr(C++)] on traits to fix their vtable layout?).

Field composition sugar would make the most sense if associated fields didn't require explicit mappings, although it could be nice elsewhere regardless.

@CloudiDust

This comment has been minimized.

Contributor

CloudiDust commented Sep 20, 2014

@rpjohnst I think while implementation-wise this proposal may be fairly complex, conceptual-wise it is not. All components are natural extensions to the language and don't feel bolted on (where all other proposals failed, even proposals like #223 give a "for flexible inheritance only" impression, as it is "low level" and its components don't feel like immediately usable day-to-day, though it is actually more flexible).

Of course there can be needlessly complex bits and rough edges in this RFC, and those needed to be addressed.

And yes, a Fat like feature would be better for this proposal too.

@rpjohnst

This comment has been minimized.

rpjohnst commented Sep 21, 2014

Part of the problem with implementation complexity (especially of the generated code) is that it is more conceptually complex. For example, really understanding all the details and gotchas and corner cases of C++ multiple inheritance requires understanding where vptrs are located, where parent class members are stored, how casts work with offsets, etc.

Not to mention this does have significant conceptual complexity with so many moving pieces. Really all Rust needs for inheritance is a way to specify that common fields are at a fixed offset in a structure (Extends or much-simplified associated fields) and a way to specify where the vptr is (Fat based on Vtable, and vtable layout).

#[repr(fixed)], #[repr(internal_vtable)], and associated field mappings are pretty much kluges to make this work with multiple inheritance when a simpler design (conceptually and implementation-wise) would do just fine. Multiple inheritance for COM or C++ interop can be (and would probably need to be) done with manual #[repr(C)] layout and maybe a bit of unsafe.

override, field composition, and callable defaults are all just maybe-nice sugar that can be replaced with little to no boilerplate. At the very least they ought to be separate RFCs from this one, so we can focus on associated fields without distraction here.

@AaronFriel

This comment has been minimized.

AaronFriel commented Sep 21, 2014

I have written and submitted a pull request to address inflexibility with the field composition syntax. The proposed syntax is more extensible, and several potential opportunities to grow the feature are illustrated. (rendered)

An example of this syntax would be, given:

struct A { 
      a: uint, 
      b: uint
}

struct C { 
    x: uint, 
    parent: A, 
    use parent {..}, 
    c: uint
}

This would be equivalent to:

struct A {
    a: uint,
    b: uint
}

struct C {
    x: uint,
    parent: A,
    c: uint
}

But with the additional property that where c:C, c.a and c.b would desugar
to c.parent.a and c.parent.b. It should be observed that the use statement
is a no-op in terms of the field declarations.

@eddyb

This comment has been minimized.

Member

eddyb commented Sep 21, 2014

The inline vtable field is not supposed to be always leading in my original design, the layout would be fixed and then it can take any position.
I recall @Kimundi contested that, but not whether he actually made a case for that.
As for Fat<T>, it can't offer the same functionality (i.e. safe upcasting) without similar vtable primitives - so the choice is not one of implementation complexity.

@CloudiDust

This comment has been minimized.

Contributor

CloudiDust commented Sep 21, 2014

@rpjohnst, fair points. :)

Indeed if we can implement Vtable like Fat, and if there is no need to require overridable defaults like in this RFC, then it is all the better.

Still personally I am quite fond of overridable defaults, but it can wait if possible, and we should focus on associated fields in this RFC.

@CloudiDust

This comment has been minimized.

Contributor

CloudiDust commented Sep 21, 2014

@eddyb, so basically the "overridable defaults" requirement cannot be dropped? Not that I do not want that feature, of course.

@gereeter

This comment has been minimized.

gereeter commented Sep 21, 2014

One interesting point about this proposal is that, like #223, this is very modular, and parts of this can be easily mixed with parts of #223. After reading this, I realized that the rearranging of vtables is also necessary for my proposal, but wasn't explicitly stated. Also, the overridden defaults and ability to call default impls would be incredibly useful in my proposal. Similarly, I this proposal could easily accept things like @glaebhoerl's Cast/Extend/Coercible/HasPrefix system.

@Ericson2314

This comment has been minimized.

Contributor

Ericson2314 commented Sep 21, 2014

This or #223 is absolutely the way to go. Great work!


My only word of caution is that Haskell has grown a number of deriving/default-impl extensions where even together not all problems are solved, and yet there is a ton of redundancy. I worry the default impl / override impl parts of this proposal could open the floodgates and get us in the same experience.

My experience with Haskell led me to believe the one true solution is to relax coherence and decouple choosing a canonical impl from writing a impl at all. That way, I believe, all deriving/default patterns can simply be encoded with a bunch of overlapping "polymorphic instances" (aka impl<T> _ for T where ..) that are mixed and matched to write the canonical instance.

That however is a large proposal in itself, so if there is interest in it, I recommend leaving the override/default impl portions of this RFC out, and tackling them separately.


Also, if we add explicit Vtables, it would be really cool replace trait objects with general existentials:

Type ObjDirect<V: Trait> = exists<T>(Vtable<T, V>, T);
Type ObjIndirect<V: Trait, Ptr: type -> type> = exists<T>(Vtable<T, V>, Ptr<T>);

People, including myself, have mentioned such things before, but explicit Vtables bring us one step closer.

@CloudiDust

This comment has been minimized.

Contributor

CloudiDust commented Sep 21, 2014

@Ericson2314, yes, one of the reasons that I like the "overridable defaults" part is, I foresee that we eventually have to deal with it or variation of it anyway, so it is better to get it right once and for all, and not having a feature that implements "some but not enough" thus create redundancy when we later intoduce other features to deal with the "not enough" problem..

I didn't think of the possibility that override itself is such a "some, but not enough" solution.

If so, we should indeed left out this part from the proposal for now, if possible.

Also, full-on existentials is a nice thing to have. But I thought this should be a backwards compatible extension to DSTs which doesn't create redundancy, like HKTs to the generic systems we now have. Or no? If not, then we should also consider this now.

@CloudiDust

This comment has been minimized.

Contributor

CloudiDust commented Sep 21, 2014

I remember @glaebhoerl also has some thoughts on full-on existentials, so I think I should cc him again.

@CloudiDust

This comment has been minimized.

Contributor

CloudiDust commented Sep 22, 2014

@Ericson2314, still both full-on existentials and enhanced impls may themselves lead to differing coding styles, I suspect, so yes we should be generally cautious when adding those kind of features.

I don't think we should do any of these features including inheritance before 1.0, but if full-on existentials is deemed "maybe desirable", then we should make sure the design of DST is compatiable with that feature. (Basically DSTs will be syntax sugars for some common use cases of that.)

Hope I am not derailing.

@CloudiDust CloudiDust referenced this pull request Sep 22, 2014

Closed

Layout Inheritance #254

@Kimundi

This comment has been minimized.

Member

Kimundi commented Sep 22, 2014

Right, I kinda unintentionally glossed over privacy there.

Due to field composition sugar being entirely structural, almost like a macro, you'd probably handle visibility on a more basic level: Enforce that all fields of the used struct are public, or enforce that the struct shares the same visibility space the the place where it is embedd into (Eg, same or parent module).

This would solve the problem of "leaking" fields. (Though its really just sugar for redeclaring a field)

Associated items of traits currenly always share the the visibility of the trait itself, so

trait X {
    ..A
}

would define X to have associated fields identically named to As

Embedding into a struct is a bit more tricky, as there fields can have actual visibility modifiers. I guess the only sane thing here would be to make embedded fields public/private in batch:

struct C {
    x: uint,
    ..A,
    c: uint
}

vs

struct C {
    pub x: uint,
    pub ..A,
    pub c: uint
}
@AaronFriel

This comment has been minimized.

AaronFriel commented Sep 22, 2014

@aturon, @Kimundi I proposed an alternative syntax that would provide greater control, please see the changes rendered here.

I do not know why my alternative syntax is not being considered, but I believe it permits much greater flexibility and clarity. I took the time to write up a pull request and a modified RFC as I was told was the proper way to participate in the discussion and I am now ignored, which frankly does not feel very welcoming.

That said, I propose a syntax change to allow greater flexibility in the future. This syntax change to the ..PATH production rule is similar to a design proposed on /r/rust, but varies in a few ways. Chiefly, it separates the declaration transcluding fields from a struct from the use of that struct. To respond to your proposal, this is what I would suggest:

struct C {
    pub x: uint,
    pub ..A,
    pub c: uint
}

I propose:

struct C {
    pub x: uint,
    parent: A,
    use parent {pub ..},
    pub c: uint
}

Which would cause c.a to desugar to c.parent.a with visibility overridden by the use.

The greater flexibility and potential extensions illustrated by my draft RFC are not part of it, per se. But I believe that it follows norms that Rust appears to follow more closely, w.r.t. modules and hierarchical paths. That is, the use I propose merely creates syntax sugar in the same manner as use does for modules, by creating aliases for names that would otherwise be in scope with a longer name.

@aturon

This comment has been minimized.

Member

aturon commented Sep 22, 2014

@AaronFriel

I do not know why my alternative syntax is not being considered, but I believe it permits much greater flexibility and clarity. I took the time to write up a pull request and a modified RFC as I was told was the proper way to participate in the discussion and I am now ignored, which frankly does not feel very welcoming.

My apologies, I'm still working my way through the comment thread here and elsewhere, and am focusing on trying to understand the original RFC first before looking at further variations. I'll try to comment on your proposal soon.

@aturon

This comment has been minimized.

Member

aturon commented Sep 22, 2014

@Kimundi

Two questions about override:

  • You say "a type is not allowed to implement two traits that each override the same item and are not in a inherits-from relationship (diamond problem)". But that seems overly restrictive: why not allow such an impl, but force the impl to provide an explicit implementation of the multiply-overridden defaults?
  • Did you give any thought to multiple super traits that define a method with the same name? This is allowed today, and it's not totally clear to me what override would do/mean in such cases.
@aturon

This comment has been minimized.

Member

aturon commented Sep 22, 2014

@Kimundi

Another question about override. What happens with the following:

trait Foo {
    fn foo(&self);
}

trait FooAlt {
    fn foo(&self);
}

trait Bar: Foo {
    override fn foo(&self) { }
}

impl Bar for u8 {}
impl Foo for u8 {}
impl FooAlt for u8 {}

and what about this variant:

trait Foo {
    fn foo(&self);
}

trait FooAlt {
    fn foo(&self);
}

trait Bar: Foo {
    override fn foo(&self) { }
}

trait BarAlt: FooAlt {
    override fn foo(&self) { }
}

impl Bar for u8 {}
impl BarAlt for u8 {}
impl Foo for u8 {}
impl FooAlt for u8 {}
@aturon

This comment has been minimized.

Member

aturon commented Sep 22, 2014

@Kimundi

The associated field sub-RFC seems to require explicit mappings => hooking these up to actual lvalues. But in the DOM example, you write:

struct TextNode {
    vtable: Vtable<TextNode, Node>,

    parent: Rc<Node>,
    first_child: Rc<Node>,
    ...
}
impl Node for TextNode {}

without giving the field implementations for the impl. Was that intentional?

@aturon

This comment has been minimized.

Member

aturon commented Sep 22, 2014

@Kimundi

Did you consider "properties" (as in e.g. C#) as an alternative to associated fields? What do you see as the tradeoffs there?

@Kimundi

This comment has been minimized.

Member

Kimundi commented Sep 22, 2014

@aturon:

  • You are right that rather than preventing you from implementing two traits that conflict in an overriden item, instead forcing you to explicitly implement it would be better. That way you could choose your own behavior, or dispatch to one or both of the supertrait default impls.
  • I had thought about what happens if multiple super traits implemented the same item, but forgot about mentioning it in the RFC text. Basically, I'd propose an optional or mandatory piece of syntax for selecting what parent trait to apply to:
trait Bar { fn foo(); }
trait Baz { fn foo(); }
trait Foo: Bar + Baz {
    override for Bar fn foo()  { ... }
    override for Baz fn foo()  { ... }
}

(Or some other combination, like override ITEM for TRAIT or for TRAIT override ITEM)

  • The missing mapping in the DOM example are sorta unintentional due to not having nailed down that part either. @eddyb was bouncing around the idea that field mappings could default to automatically apply to identically named fields of Self in an impls, which is where that empty impl is coming from.
@aturon

This comment has been minimized.

Member

aturon commented Sep 22, 2014

@Kimundi

I presume that associated fields, like other associated items, are usable within inherent impls?

That would mean you could do the following, inspired by @AaronFriel's PR:

struct A {
    a: uint,
    b: uint,
}
struct C {
    x: uint,
    inner: A,
    c: uint,
}

impl C {
    a: uint => self.inner.a,
    b: uint => self.inner.b,
}
@aturon

This comment has been minimized.

Member

aturon commented Sep 22, 2014

@AaronFriel One advantage of your proposal is that you can get a reference to the inner struct, i.e. given:

struct A { 
   a: uint, 
   b: uint
}

struct C { 
   x: uint, 
   parent: A, 
   use parent {..}, 
   c: uint
}

you can say &some_c_val.parent and wind up with a &A value. So you retain some of the benefits of composition, while providing similar ergonomics to this RFC. It's an interesting idea, and I'll definitely make sure it's discussed at the meeting (if we get to this level of detail).

FWIW, I think the A.. syntax proposed in the RFC is related to @eddyb's variadic generics proposal.

@AaronFriel

This comment has been minimized.

AaronFriel commented Sep 22, 2014

@aturon: I like the idea of putting an associated fields/use declarations in the impl. There still needs to be a way to do wildcard aliasing, a la:

struct C {
   a: uint,
   b: uint,
   inner: A
}

impl C {
    use a {..}
}

I think that makes it clear that you want to bring names in as aliases. But I would really prefer that if that happened, that it be clear that only fields (and not methods) from the used struct are aliased. Something like this would be nice:

impl C {
    use fields a.*
}

Alternative syntaxes I could imagine, for various reasons:

  • use a::fields::* and use a::fields::{pub a, pub b}
  • use fields a::{pub *}

This would allow you to do things like:

impl C {
   use a::methods::{foo, bar}
}

Should module use syntax include a hiding clause in the future, that same syntax could be used here to not make some aliases.

@Kimundi

This comment has been minimized.

Member

Kimundi commented Sep 22, 2014

@AaronFriel: Sorry, didn't mean to ignore you, I was just kinda busy and didn't have the time to look at your proposal in detail yet :(

@aturon: Yes, you could define associated fields that dispatch deeper into a regular field rather than using composition sugar.

I haven't thought much about it yet, but this might be indeed be a way to remove field composition sugar from the picture, and would avoid all the awkwardness about privacy and not being able to refer to the content of the field as its own type.

@Ericson2314

This comment has been minimized.

Contributor

Ericson2314 commented Sep 22, 2014

@CloudiDust So I was only proposing quantifying over the universe of types, not quantifying over any type. One couldn't write exists<n : Nat>([T, ..N]) or anything like that. To me, DSTs are sort of an "auxillary" part of the type system (like lifetimes), whereas existentials are more of a general type-theory notion so I guess I am not too worried about overlap. Also consider not all existentials are DSTs: exists<T>(&T). In a hypothetical Rust you could feed that to to any f : fn<T>(T) -> X(T) as it would be safe to erase T when compiling f.

/derail

Part of me feels a bit funny about all the associated field stuff, in that it basically boils down to a fancy system to ensure methods like fn f (&self) -> &T; don't need to deference so they can be optimized in all cases. If we would had an effect system, I would propose as an alternative:

  • dereferencing is an effect.
  • such pointer-returning methods are defined with a type disallowing such an effect.

Everything else proposed would just be macros/sugar -- all the desired semantics / guaranteeing of optomizability would come from the effect system itself.

@CloudiDust

This comment has been minimized.

Contributor

CloudiDust commented Sep 22, 2014

@Ericson2314, if I am understanding currently,DSTs are special cases of existentals: Trait is exists T. T: Trait, while [T] is exists N. [T, ..N], and str is under the hood exists N. [u8, ..N].

Implentation-wise, each existential quantifier would either be in the fat pointer or in the fat object, and we can mix and match. (exists T. means a vtable ptr, while exists N. means a ptr-sized unsigned int here)

But full on existentals may be too powerful so we can end up having multiple ways to encode the same type, which is undesirable. (But may also not be a problem in practice, as the underlying implementation choices are different, as long as we don't allow too much freedom.)

/derail

An effect system ... huh, another new perspective. Well I am sort of starting to realize why language design is so interesting. :)

But I do think that will be overkill this time.

@CloudiDust

This comment has been minimized.

Contributor

CloudiDust commented Sep 22, 2014

@Ericson2314, actually if there is a need to expose the Fat/Vtable etc anyway, I do think more powerful existentials support can actually be a more uniformed and flexible solution here. The ability that we need here, is to control where the "instantiated" quantifiers are stored.

@CloudiDust

This comment has been minimized.

Contributor

CloudiDust commented Sep 23, 2014

I misused the name quantifier above. And existential types is indeed a complex feature and not going to happen soon if ever. We'd need variadic generics, a new keyword and tons of magic to make it work. (Or actually we'll "reify" the magic that the compiler performs for DSTs.) Not worth it at least for now.

/derail

This has the effect that every supertrait is embedded in the vtable of a given trait, at the cost of slight bloat and duplication for every case of multiple inheritance in a trait hierarchy. For single inheritance-only hierarchies, this algorithm will generate the same vtable elements as today, just reordered in prefix order, and thus will have zero additional bloat.
Once this is done, the compiler needs to allow casts or coercions like `&Trait as &SuperTrait` and implement it by adding a constant pointer offset to the vtable pointer.

This comment has been minimized.

@nrc

nrc Sep 23, 2014

Member

I thought we already allowed such casts?

This comment has been minimized.

@nrc

nrc Sep 23, 2014

Member

Huh, seems I am wrong about this - I thought we could explicit cast but implicit cast, but we can do neither.

@zwarich

This comment has been minimized.

Contributor

zwarich commented Sep 23, 2014

For this to be viable in Servo's DOM use case, we would probably have to avoid generating duplicate copies of parent class methods for each child class, which is what would happen with naive monomorphization and vtable creation. From a discussion on IRC it seemed like this would be possible, at least with some changes to how reflection works.

@eddyb

This comment has been minimized.

Member

eddyb commented Sep 23, 2014

I have seen a few suggestions to make fixed layout a requirement for associated fields. That is not really an option for the more ("idiomatic") use case of being generic over a type implementing a trait exposing some fields.
Not to mention that fixed field layout is incompatible with types not annotated with the same #[repr] - the default is unspecified, allowing field reodering, among other optimizations.
Linting against field access via virtual offsets, or making it opt-in by could be useful if we decide to implement that at all.
I do like more fine-grained control - if we could pin a field and allow the rest to have arbitrary positions, that should be superior to my original scheme, @Kimundi's modification to require leading vtable field instead of fixed layout and Fat.
I am not overly fond of a specific overloading scheme, it just has to work and be unambiguous both to the user and the compiler. That's why I originally required either implementing just the subtrait or providing explicit versions of all the overriden methods, in the impl of the parent trait. The latter would work just like it does today, while the former would have the subtrait impl act as an impl for the parent trait as well, taking the defaults from the subtrait, if overriden.

@aturon

This comment has been minimized.

Member

aturon commented Sep 23, 2014

@eddyb

I am not overly fond of a specific overloading scheme, it just has to work and be unambiguous both to the user and the compiler. That's why I originally required either implementing just the subtrait or providing explicit versions of all the overriden methods, in the impl of the parent trait. The latter would work just like it does today, while the former would have the subtrait impl act as an impl for the parent trait as well, taking the defaults from the subtrait, if overriden.

After a discussion with @eddyb on IRC, I wanted to clarify some of the problems around this feature.

Suppose that trait Foo provides a single method foo with a default, and SubFoo overrides that default. @eddyb points out that:

  1. As proposed, when you see impl Foo for T {}, you don't immediately know which implementation of foo you'll get. You have to see whether there is any impl of a subtrait like SubFoo that might override the impl.
  2. Worse, SubFoo might be defined and implemented in a different crate, in which case its overrides can't possibly apply to the Foo impl. That means the semantics of impl blocks change depending on which crate the block appears in.

In general, it seems better to somehow signal in the impl for Foo that it will be using overridden defaults from SubFoo. @eddyb's comment above gives some possibilities, but doesn't cover the case that Foo has some _non_overridden methods.

Here's another possibility:

impl Foo for T { ... }
and impl SubFoo for T { ... }

That is, we could provide a syntax for tying impl blocks together. This retains the grouping of methods we have today, while making it much more clear when overrides may be applied.

(This is just a sketch; more work would be needed to make sure this is a viable approach.)

@Kimundi

This comment has been minimized.

Member

Kimundi commented Sep 24, 2014

Reading the meeting minutes, I just want to leave a quick comment about the "initial call is virtual, all further are statically dispatched" part: Thats the property if you implement methods on a trait as direct trait methods, which makes them virtual in a trait objects:

trait Foo {
   a: uint, 
   b: uint
   fn foo(&self) -> uint { self.a + self.b }
}
let x: &Foo = ...;
x.foo(); // Virtual call

However, with DST its also possible to directly implement methods on the trait object itself, which leads to statically dispatched code:

trait Foo {
   a: uint, 
   b: uint
}
impl Foo {
   fn foo(&self) -> uint { self.a + self.b }
}
let x: &Foo = ...;
x.foo(); // Static call

So, you get the explicit choice for each method.

@aturon

This comment has been minimized.

Member

aturon commented Sep 24, 2014

@Kimundi

However, with DST its also possible to directly implement methods on the trait object itself, which leads to statically dispatched code.

Note that, given that inherent methods trump trait methods, and that these DST-style impl blocks count as inherent methods, you can almost use this pattern to "override" supertrait methods with statically dispatched methods on a subtrait.

It works fine when calling methods on the objects of the subtrait type. The problem is that if you ever upcast to the supertrait, you'll end up calling the supertrait version of the method instead.

And of course all of this is separate from the explicit default overriding provided in this RFC. You could use them in tandem, but that is not DRY and is error-prone.

@nrc nrc referenced this pull request Oct 3, 2014

Open

Efficient code reuse #349

@aturon aturon added the postponed label Oct 3, 2014

@aturon

This comment has been minimized.

Member

aturon commented Oct 3, 2014

@Kimundi and @eddyb:

First, thanks for the work on this RFC!

We discussed this RFC earlier this week.

Some general feedback:

  • There are a lot of good buildling-blocks here, and in particular there's a fair amount of interest in trait object upcasting and associated fields regardless of what else happens.
  • The general sense was that exposing the mechanics of internal vtables involved a fair amount of pain for relatively little gain. The alternative of an attribute (or perhaps keyword) to request the compiler to switch representations seems much more ergonomic.
  • There's some worry about the default overriding proposal here. In particular:
    • The proposal seems to need a fair amount of mechanics to guarantee basic coherence.
    • You would often end up with dual hierarchies, one at the struct level and one at the trait level.
    • There's no simple way to have a method that is dynamically dispatched at one point in the hierarchy, and statically dispatched when you're low enough in the hierarchy. Getting this behavior appears to require a combination of overriding defaults and DST-style inherent implementations, which unergonomic and somewhat error prone.

While we're quite sympathetic to the goal of addressing this design challenge via orthogonal extensions to existing features, that must be balanced against the simplicity and ergonomics for the resulting design, and we don't feel like this proposal reaches that balance.

In any case, you can read about the plan going forward here; we intend to discuss these designs further in a couple of months. Thanks again for your effort!

Closing as postponed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment