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

Layout Inheritance #254

Closed
wants to merge 1 commit into from
Closed

Conversation

rpjohnst
Copy link

The main idea here came from this issue. Structs can inherit structs without subtyping; traits can inherit structs to force implementors to inherit them as well; match statements are used for downcasting; #9 fat objects gives thin pointers; some extra tweaks to make everything work together.

Rendered

@CloudiDust
Copy link
Contributor

I think having structs/traits inheriting from each other in this way is actually neat, but the problem is that, it invokes wrong expections (subtyping relationship). Blame all those mainstream languages that conflate inheritance and subtyping.

So personally I still prefer the associated fields syntax in #250, think they can be incorporated into this RFC. (And the biggest difference between this and #250 would be around the design of Fat/Vtable and match.)

@CloudiDust
Copy link
Contributor

To expand a bit:

It is okay to have traits inherit traits, because there is "sort of" subtyping happening, and of course it is okay for traits to be implemented for structs.

But the inhertance between structs/structs, structs/traits, traits/structs is "private inheritance" in C++ speak, which doesn't involve subtyping. Composition is generally favored over this type of inheritance, and no other mainstream language with class-based OOP has this type of inheritance and it is seen as one of C++'s misfeatures.

Arguably, "private inheritance" is what inheritance should have been, as it is not conflated with subtyping.

But in a world where "private inheritance" is generally frowned upon, we should not introduce them into Rust, especially not with syntax that look like "public inheritance", or people will ask "why is there no subtyping? That's surprising and a design flaw!" when it is actually not.

@rpjohnst
Copy link
Author

The reason this sort-of "private inheritance" works better than composition in this particular case is that Rust's struct layout is undefined. The alternatives are #[repr(C)] which is unsafe, #[first_field] which is type-system-affecting metadata, or associated fields which are rather overcomplicated for the single inheritance scenario (and I'd argue in general as well).

I'd be open to an alternate syntax that looks less like inheritance, but honestly I don't see it as that big of a problem- the semantics are quite nice and don't have the problems of private inheritance. Besides, trait-inheriting-struct is quite intuitive relative to trait-inheriting-trait, and at that point everyone is expecting Rust to be different anyway.

@CloudiDust
Copy link
Contributor

This just occurred to me: what about replacing the : symbol with <-?

I know of no language that use this for public inheritance. (They go with : or < or even extends.)

Thus we convey the message that "this is inheritance, but different from the one you are used to."

But this may conflict with monadic do if we add them later. (Not a big problem because the context is different.)

@rpjohnst
Copy link
Author

Total bikeshed, but we might also be able to use +:

struct A {}
struct B + A {}

It's a little odd compared to the syntax for multiple bounds, but it doesn't look like inheritance.

@CloudiDust
Copy link
Contributor

+ is commutative, or some would think it is "concatenation of B and A", while it is actually more like "concatenation of A and these extra fields", so struct B + A {} feels weird.

struct B = A + {}? But personally I prefer ++ as concatenation.

So I still prefer struct B <- A {}.

@CloudiDust
Copy link
Contributor

As traits can inherit from both traits and structs, both : and <- will become unusable when a trait does inherit from both other traits and structs at the same time.

So maybe we can use this:

struct StructA {...}
struct StructB {...}
trait TraitA {...}
trait TraitB {...}
trait TraitC: TraitA + TraitB [StructA, StructB] {...}
struct StructC: [StructA, StructB] {...}
impl TraitC for StructC {...}

The [] around "parent structs" is not optional even if there is only one "parent struct".

@arcto
Copy link

arcto commented Sep 22, 2014

I'm not convinced that the unification of structs is such a useful feature. If you are reaching in to your parent's struct (so to speak) why not be clear about it by prepending the parent/super field to the expression? I don't quite see the need to mix all the fields into one big soup. What about accidental name-clashes? What's the motivation?

@ftxqxd
Copy link
Contributor

ftxqxd commented Sep 22, 2014

Although I think that this is a nice simple way of adding inheritance, I do have a problem with it: it forces one to create a ‘parent’ struct even if it is never used for anything but inheritance. For example, if I wanted a number of Shapes all with x and y fields, I would need to create a parent Shape struct as well as a corresponding trait:

// What am I supposed to call this struct?
struct ShapeFields {
    x: f64,
    y: f64,
}

trait Shape: ShapeFields {
    fn translate(&mut self, dx: f64, dy: f64) {  /* do stuff with self.x, self.y */ }
    ...
}

struct Square: ShapeFields { ... }
impl Shape for Square { ... }
// etc.

If traits can have structs (and their fields) ‘tied’ to them, it makes more sense to me to just effectively integrate the struct/its fields into the trait itself:

trait Shape {
    x: f64,
    y: f64,
    fn translate(&mut self, dx: f64, dy: f64) { /* do stuff with self.x, self.y */ }
    ...
}

struct Square { x: f64, y: f64, ... }
impl Shape for Square { ... }
// etc.

Which is exactly the approach that #250 takes. That approach does have the downside of having to repeat the fields in both the struct and trait definitions, but this also has a good side: it makes the fields of a struct instantly viewable just from reading the struct declaration, and also provides more flexibility: one can name the fields however one chooses to, use a tuple struct (since we now have tuple indexing), or even take the fields from multiple levels of fields (e.g., foo.bar.baz). Additionally, it provides things like field aliases. In my opinion it also integrates better with traits, which is the closest thing we have to inheritance right now.

But there are quite a few things I like about this proposal. I like its simplicity: the idea of traits inheriting structs is an easily understandable one. The problem I mentioned above doesn’t apply when you do need to use the ‘base’ struct. The idea of fat objects, although not originating from this RFC, is a nice one that seems to fall out naturally from DST. So overall, this is a reasonable form of inheritance, even if I do have my doubts.

@CloudiDust
Copy link
Contributor

After some experiment with the syntax, I found that, the syntax felt too right and lightweight, it urged me to group my code into a hierarchy, and provide an "Ops" trait for each struct (!).

EDIT: the last part was because I found myself wanting to write a method-matching "interface" and implement it on the struct. That's standard Java style, "program to the interface" - why was I suddenly wanting to do Java in Rust?

In other words, it was encouraging me to write code in a (pseudo-) classic OOP style. Alfter a while, I also expected the struct B [A] syntax (a variation of struct B: [A] I proposed earlier) to do public inheritance and assumed that all methods of the "parent" was available on the "child", The syntax was not helping.

We want inheritance, but we do not want to encourage it, and this proposal (or any "struct inheriting" proposal for that matter) is making inheritance (or illusions of it - the method part) too easy.

@CloudiDust
Copy link
Contributor

@P1start Actually field mapping has other upsides, it creates psychological distance between the trait and the struct that has it implemented for, and also requires more typing, thus making programmers less likely to use inheritance - or rather, to use "associated fields" and "field mapping".

Why I said "or rather"?

Because associated fields and field mappings in #250 just doesn't feel like C++ style inheritance, but another set of features that happens to support the same use case as inheritance. While this proposal or any other "struct inheriting" proposal feels like "intentionally crippled C++ style inheritance that doesn't even support subtyping and method-inheritance".

Note: We intentionally cripple the feature for good reason, but it feels crippled, and tempting.

That's mainly psychological of course, but I do believe this is important, and maybe why I thought #250 felt right when I read it for the first time: Rust-style, and there is no artificial "limiting" feeling there, but you just don't want to use it everywhere.

But the overridable defaults part in #250 does raise my worries now, as they are still very C++ like and may be easily misused.

EDIT: Adjusted and clarified some expressions. And also, maybe after a while I can get over the false C++ dejavu and accept this. ;)

@rpjohnst
Copy link
Author

On "looks too much like C++ inheritance," other proposals have all suggested macros to close the gap anyway.

This is associated fields with less design overhead: it reuses constructs like structs for fields and traits for vptrs, without redundantly expanding them with vptrs in structs or fields in traits/impls. The new feature is the association of traits and structs, and that's all this does.

@CloudiDust
Copy link
Contributor

@rpjohnst: Yes. But macros require !s and don't feel first class, and they may not even have to be in the std lib.

Maybe that's just me, but to me the relative verbosity of #250 and #223 when they are without macros, is a feature.

The problem with this RFC is it feels too right, with too little overhead, so it encourages an "inheritance-like" design, which I personally don't like. But I also don't speak for anybody else. :)

@rpjohnst
Copy link
Author

As long as inheritance is appreciably more verbose than just using traits, more verbosity is only a bad thing. The other side is that efficient inheritance should be less verbose than its inefficient workarounds.

My problem with more verbose proposals is not that they are more verbose per se, but that they are less orthogonal and more verbose because of it. This proposal takes the property all the RFCs are trying to encode ("this type shares these common fields") and encodes it without making existing features do more than their one job.

@CloudiDust
Copy link
Contributor

Where to put common fields is also a matter of perspective.

On one hand, we can argue that "traits do not have fields currently, but structs do, so we should encode common fields in structs and let others inherit them".

But on the other hand, we can argue that "It is structs' job to store actual data, and traits' job to form interface contract, and common fields are a form of contract, so they should not be in structs, but in traits."

I do believe both are internally consistent. So the rest is subjective preference. Also I see that you believe this RFC is verbose enough while I disagree. :)

Also, field mapping can be justified because Rust traditionally does not tend to make decisions for client code when traits are involved. When two trait method names clash, the client code must explicitly choose which method to call, unlike many other languages which automatically picks one.

Is field mapping more complex? Yes. Worth it? I think so, but I can also see why it can be undesired.

I do hope some aspects of #250's internals can be simplified, especially overridable defaults should be dropped for now, and hope it can also better utilize existing facilities like match.

@arcto
Copy link

arcto commented Sep 22, 2014

I don't like the idea of fields as interface. The how a property is implemented/stored/calculated is very much an implementation detail

The trait is just an abstract description of an interface. If an implementation wants to calculate a property dynamically instead of storing it in a field it should have the capacity to do so. If traits can require fields then all of this is lost.

I want traits to be limited to specifying behavioural contract -- not implementation.

@rpjohnst
Copy link
Author

@CloudiDust The fact that some group of fields is shared between types is indeed an interface contract, and it is in this proposal represented by a trait requiring a struct of those fields- again, this is associated fields with less semantic and syntactic overhead. Field grouping with structs, interface with traits.

@arcto Rust is a systems language. At that level, "implementation details" are frequently very important parts of a type's interface. Of course it's nice when you can be more general than that, but the inheritance proposals exist because that's not always the case. All of them introduce some way to specify struct layout as some kind of common interface. (Note that you can always regain that generality by implementing a less specific trait without a struct for the more specific trait with the struct.)

@arcto
Copy link

arcto commented Sep 22, 2014

@rpjohnst I think it's important not to confuse inheritance which defines an IS-A relationship, with composition which defines a HAS-A. Traits with fields are clearly composition, not inheritance.

Leaking implementation details has nothing to do with systems programming. Systems programming is about precise control, such as giving the implementer direct access to the hardware and to the representation.

Generics is what you use to specify types from the outside of an implementation: MyAbstractType<uint>.

@rpjohnst
Copy link
Author

@arcto I'm not sure what you mean. My "struct inheritance" is has-a as it doesn't create a subtyping relationship, and a trait inheriting a struct merely enforces that has-a relationship as part of its interface. How would you prefer to specify the interface of "a pointer to any type with these fields at offset x"? That's the sort of property about representation we need to encode.

@arcto
Copy link

arcto commented Sep 22, 2014

@rpjohnst Wasn't the original requirement worded like this "cheap field access from internal methods"? That says nothing about access to fields from the outside at a particular offset.
I must be the one who has misunderstood the issue because I don't see a problem here; let the interface supply an (possibly inlineable) accessor to the value of interest.

@rpjohnst
Copy link
Author

If you leave layout undefined and give cheap field access only from internal methods, those methods must be monomorphized. For accessor methods, this is the exact problem we have now - non-inlineable virtual calls for field access. For bigger methods, this is less bad, but it does still lead to virtual calls and binary bloat even when there's no need for a semantic or representational difference.

Extending cheap field access to all code (especially default methods) solves this by including field offsets as part of the interface (which I still think is a fine thing to do in a systems language), so accessors can be inlined (or bypassed completely) and common methods don't need to be virtual and duplicated in the binary just to change the field offsets they use.

There is a middle ground suggested in another RFC and mentioned in this one's alternatives, of putting field offsets in vtables, but that sacrifices both performance and interfaces-not-including-implementation-details. May as well go all the way so we have both options, especially since offsets-in-vtables could be done with associated static pointers-to-members.

@aturon
Copy link
Member

aturon commented Mar 5, 2015

@nikomatsakis I notice you're marked as the shepherd here, but haven't commented. What are your thoughts on this RFC at this point?

@nikomatsakis
Copy link
Contributor

I think it's clear that this RFC ought to be postponed under issue #349, like the other RFCs targeting this particular problem (in fact, I kind of thought I already had done so). I expect we'll be revisiting this after 1.0 as there is still a real need for these sorts of features though. Therefore, I'm closing the PR now and adding this RFC to the list of postponed RFCs on #349. Thanks!

@nrc nrc mentioned this pull request Mar 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants