Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upEfficient single inheritance with traits. #245
Conversation
nrc
self-assigned this
Sep 17, 2014
This comment has been minimized.
This comment has been minimized.
|
Note that actually, one thing about structs has changed cf #142 - the |
bill-myers
reviewed
Sep 18, 2014
|
|
||
| Any concrete data type which implements a closed trait has a vtable pointer as a | ||
| first field in its memory layout (note that nested structs/enums would have such | ||
| a field in any case, other data structures would get an extra field). Since |
This comment has been minimized.
This comment has been minimized.
bill-myers
Sep 18, 2014
What happens if a struct/enum implements multiple unrelated closed traits?
Does it add a vtable pointer for each of them as C++ would for multiple inheritance, bloating the structure?
Or do you plan to assign non-overlapping vtable index sets to all the closed traits in a crate, thus making the total space consumed by vtables quadratic in the size of the source code? (if you have N closed traits of 1 functions, and N structs, each vtable is size N, hence N^2 total space consumed by vtables, mostly consisting of holes)
This comment has been minimized.
This comment has been minimized.
nrc
Sep 18, 2014
Author
Member
Good questions! I will ponder these. If the traits are unrelated, I expect we have to fall back to fat pointers (with a warning) or give an error.
This comment has been minimized.
This comment has been minimized.
bill-myers
Sep 18, 2014
I guess overall quadratic vtable space is probably not going to be a problem in practice, especially if closed traits are used rarely, which they should be.
Thinking about it, even normal Java-style single inheritance causes quadratic vtable space, since every time you create a subclass you have to copy all previous vtable entries.
Also, one can reuse vtable slots for traits that are never implemented together on a single type (this reduces to a graph coloring problem where nodes are trait methods and there is an edge between all methods in the same trait between methods in different traits if any struct implements both traits).
bill-myers
reviewed
Sep 18, 2014
| ``` | ||
| // closed means we can downcast via match and optimise to a thin pointer | ||
| #[closed] | ||
| trait Element { |
This comment has been minimized.
This comment has been minimized.
bill-myers
Sep 18, 2014
It makes no sense to use a closed trait for this.
There should be an "Element" variant of Node, and Element references would refer to the enum and not a trait object.
Same thing for MediaElement.
Also JDM's example includes an "attrs" field in Element, so this is not even equivalent to it.
This comment has been minimized.
This comment has been minimized.
nrc
Sep 18, 2014
Author
Member
Ah, whoops I got the isa relationship here backwards. I think I was playing a little fast and loose with the example, but actually this is just wrong in some places. I'll update.
This comment has been minimized.
This comment has been minimized.
|
ignore my first comment, it's not quite right |
This comment has been minimized.
This comment has been minimized.
bill-myers
commented
Sep 18, 2014
|
Overall, the proposal seems in a reasonable direction. I wonder whether it's a good idea to decide at the moment of declaration whether structs/enums are sized or not: I think it might be better to do so at the point of usage. This is because it seems like there might often be no "natural" choice of sizedness when defining data, so making a choice would be arbitrary and would restrict expressiveness. In general, given a struct/enum hierarchy, one could have a system where the types are referred as 3-tuples of sets of variants D would be the set of variants that can be represented by the discriminant/vtable pointer representation of a Obviously W and R must be contained in D. For instance (here
These conversions are allowed:
Basically, this allows to express all "flavors" of hierarchical data, including the "body" of it without discriminant and even uninitialized memory. Obviously, there would be much better syntax and syntax sugar than the illustrative syntax above. [Note that the W set would be constrained to have an upper bound of size, so specifying the root enum would not be allowed if any generic parameters are introduced in a way that makes size unbounded.] Maybe this could warrant a separate RFC, but I feel it might be interesting to think about this while evaluating this one. |
This comment has been minimized.
This comment has been minimized.
thestinger
commented
Sep 18, 2014
|
@nick29581: Can the old RFC be closed, or is there a good reason to leave both as alternatives? There's not going to be adequate input on any of these inheritance RFCs because there are too many. |
This comment has been minimized.
This comment has been minimized.
|
I prefer to leave the old one open as an alternative - it has some advantages (conciseness in the Sevo use-case, mainly). There's no downside to leaving it open for a while, hopefully we'll make a call on this stuff soon. |
This comment has been minimized.
This comment has been minimized.
thestinger
commented
Sep 18, 2014
|
The downside is that there are a dozen of these RFCs and few people are actually going to invest the time reviewing each one. It's not possible to get consensus on any of these without narrowing down the alternatives first. |
This comment has been minimized.
This comment has been minimized.
|
FWIW I am evidence for @thestinger's claim. I haven't even tried reading, understanding, and evaluating most of these proposals yet. (There's a lot of them, and most of them are huge.) I keep planning to do so, but...
I would rather hope we won't. These seem like exactly the kind of features which it would be better to postpone any final decisions on until later. They're big and complex and interact with other still-baking aspects of the language and there's no critical need to finalize them before 1.0. (Servo is important, but it can muddle through and/or use feature gates. I don't think satisfying Servo's every desire needs to be a criterium for releasing 1.0.) (Note that I'm not suggesting we stop thinking about and discussing it. Thinking forward is important and valuable, and may inform decisions on other parts of the language. We just shouldn't make finishing all of it a condition for 1.0.) |
This comment has been minimized.
This comment has been minimized.
|
@thestinger in the last week, I at least have been reading through all of them, one by one, so I am glad they are not closed. We will eventually start culling once we feel we have a good understanding of the roles of each RFC. We can then produce something more final incorporating bits of all of them which a wider audience can read and critique. |
This comment has been minimized.
This comment has been minimized.
|
Some initial comments:
|
This comment has been minimized.
This comment has been minimized.
|
replying to @nikomatsakis *1. Good, the more I think about this, the more I prefer this to my previous suggestion. A formal model is probably useful. *2. I would prefer to forbid using a tuple variant as a function if it has named fields (I would actually prefer to allow named params for all functions, but that is out of scope). I think using numbers for field names for unnamed fields in initialisers/patterns is a great idea, it complements using them as field names for access well. I should think about how exactly it would work. *4. Ouch. True. I don't see this as too much of a problem (especially with type ascription or allowing trivial casts), but the backwards incompatibility is a pain. *6. Yeah, I think I was assuming implementers must be in the same crate too but didn't write it. I think the action at a distance problem can be solved by requiring the concrete types to be annotated as *8. I'm not clear on this, but I thought that was the problem we discussed in SF - once the borrow expires you have the *10. I'm not sure what this means, sorry. *11. I guess we could, the general solution seems nicer, although the |
CloudiDust
referenced this pull request
Sep 23, 2014
Closed
Change `&` to be a borrow operator. #248
This comment has been minimized.
This comment has been minimized.
|
I have changed quite a bit of the proposal. Unfortunately I did not have time to polish this in the least, so there are some open questions around the details and it is a bit rough. The main change compared to the earlier draft is that I would like to tell the story of this RFC in parts (I've done this in the summary in more depth):
I hope that this addressed the two big (concrete) criticisms of earlier proposals about the near-unification of enums and structs and the inheritance in trait-less impls. |
This comment has been minimized.
This comment has been minimized.
|
I think "enums with top level fields" and "structs with no top level fields" don't make much sense. So instead of making them synonyms, I think we can use the following rules:
|
nrc
referenced this pull request
Sep 28, 2014
Closed
Proposal for C-like nested enums to become scalar data types #331
nrc
added
the
postponed
label
Oct 3, 2014
This comment has been minimized.
This comment has been minimized.
|
Closed as postponed. Issue to be tracked in #349. |
nrc commentedSep 17, 2014
Efficient single inheritance via virtual structs and unification and nesting of structs and enums. In contrast to #142, this RFC uses traits for virtual dispatch rather than a custom system around impls. The parts of the RFC about nested structs and enums are identical to #142.