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

RFC for "fat objects" for DSTs #9

Closed
wants to merge 3 commits into from

Conversation

Projects
None yet
8 participants
@MicahChalmer
Copy link

MicahChalmer commented Mar 15, 2014

This is intended as an alternative to #5.

@cmr

This comment has been minimized.

Copy link
Member

cmr commented Mar 16, 2014

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Mar 16, 2014

This is similar to what I was suggesting to @pcwalton, though I was thinking of explicit vtable fields.
+1 nonetheless, saved me from writing my own RFC.

@dobkeratops

This comment has been minimized.

Copy link

dobkeratops commented Apr 2, 2014

I had liked eddyb's suggestion r.e. explicit vtable fields alot, I liked the idea that it would be a versatile building block - for example vtables could be embedded in 'class objects' holding one vtable and many instances, or they could be changed dynamically for state-machines.

(This is something i'd always wanted to do in C++ but was always discouraged in a team environment by the hacky nature of poking the first word (every compiler i've seen has the vtable there but its not specifically defined to be I think..))

Something like an intrinsic type: VTable<Type, Trait> .. an intrinsic to construct the 'fatpointer' for a vcall, which could be put in a deref operator.

eg, as_trait_object<Type,Trait>(data:&Type, vtable:&VTable<Type,Trait>)->&Trait /* safe because it knows where the table & data are compatible */

It wasn't so unpleasant to write this with transmute as it is now, I think you could make convinient library code and have it work safely and still be minimal features above whats there now

@MicahChalmer

This comment has been minimized.

Copy link
Author

MicahChalmer commented Apr 6, 2014

I've amended this to add a translation of jdm's servo example. While it can be translated, there are some annoying aspects of it that suggest some needed revisions to this RFC if it were to move forward.

@MicahChalmer

This comment has been minimized.

Copy link
Author

MicahChalmer commented Apr 6, 2014

@dobkeratops wouldn't the ability to switch vtables at run time be limited to cases where the subclasses did not add any of their own fields? Under that limitation, what you have is basically an enum (that similarity was the observation that led to #11, which is quite interesting.)

@cmr

This comment has been minimized.

Copy link
Member

cmr commented Apr 6, 2014

I do not think being able to change vtables at runtime is a good idea. We
can get nice optimizations when the vtable ptr is pointsToConstantMemory.

On Sun, Apr 6, 2014 at 5:16 PM, Micah Chalmer notifications@github.comwrote:

@dobkeratops https://github.com/dobkeratops wouldn't the ability to
switch vtables at run time be limited to cases where the subclasses did not
add any of their own fields? Under that limitation, what you have is
basically an enum (that similarity was the observation that led to #11#11,
which is quite interesting.)


Reply to this email directly or view it on GitHubhttps://github.com//pull/9#issuecomment-39681165
.

http://octayn.net/

@MicahChalmer

This comment has been minimized.

Copy link
Author

MicahChalmer commented Apr 6, 2014

I think he meant switching subclasses in-place by replacing a pointer to one constant vtable with a pointer to another, rather than changing the vtables themselves.

Having said that, I don't quite see the usefulness of that.

@dobkeratops

This comment has been minimized.

Copy link

dobkeratops commented Apr 6, 2014

The use is state machines; a vtable can hold responses to certain events for example. As I understand enums and vtables are orthogobal (closed set of types, open methods,, vs open set ot types, closed methods). The 'subclasses' would indeed have the same data layout, but represent different states of the same object.

People have one picture of OOP from C++.. its interesting to compare with more dynamic behavior (objective-C, even CLOS)

well, it will be interesting to see what happens with enums..

we used to do all sorts of tricks with function pointer tables in C, then C++ came along providing sugar for one specific case.. thats why I found the possibility of something more versatile interesting.

I realise of course we can still use function pointer tables, and macros could probably streamline custom cases.

I suppose if you do just go the route of mimicking some of what C++ does (i can see single inheritance + traits would be a nice combination) it would be easier to interoperate with C++, and translate some subset of their classes across.

We can get nice optimizations when the vtable ptr is pointsToConstantMemory.

is this an attribute of the pointer or the vtable itself? i'm thinking of merely swapping the pointer with other vtables - they would still be in constant memory, having still been 'rolled' by the compiler. Other times you'd still have the mutable/immutable information for optimizing I hope

@brson

This comment has been minimized.

Copy link
Contributor

brson commented May 7, 2014

Sorry for taking so long. This is a very interesting, elegant, and high quality proposal, an alternative 'virtual struct' design (#5). Since it builds on DST, and is probably a post 1.0 feature we can afford to take our time deciding on it. We will consider this seriously in designing single inheritance though. Thanks.

@ben0x539

This comment has been minimized.

Copy link

ben0x539 commented Jun 26, 2014

I really like how orthogonal this proposal is. (I'd be happy if we could find another word than fat for the user interface for this, though)

@gereeter gereeter referenced this pull request Sep 1, 2014

Closed

Trait based inheritance #223

@nrc nrc self-assigned this Sep 4, 2014

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

@nrc

This comment has been minimized.

Copy link
Member

nrc commented Sep 11, 2014

@MicahChalmer or @pnkfelix could you show how https://gist.github.com/jdm/9900569 would be encoded under this proposal please?

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

@pnkfelix

This comment has been minimized.

// Methods defined here have access to the fields and methods of NodeFields via `self` [5]
fn frobify_children(&self) {
// .children is a static offset field access--no extra pointer indirection [1]
for node in children.iter() {

This comment has been minimized.

@pnkfelix

pnkfelix Sep 12, 2014

Member

typo: should be self.children.iter() here

fn frobify(&self) {
// We know only elements have attributes as children, so we can downcast [3]
let parent_element:&Element = downcast(parent.unwrap()).unwrap();
println("Frobifying the attribtue of {}: {}={}", parent_element.name, self.name, self.value);

This comment has been minimized.

@pnkfelix

pnkfelix Sep 12, 2014

Member

nit: "attribute", not "attribtue"

}
}
trait Node:NodeFields {
fn as_element<'a>(&self) -> Option<&'a Element> {

This comment has been minimized.

@pnkfelix

pnkfelix Sep 12, 2014

Member

this is almost certainly missing the lifetime 'a on &self. Otherwise it desugars to:
fn as_element<'a,'b>(&'b self) -> Option<&'a Element>, which can only be implemented by either returning None or a Some of a &'static Element.

// but implies it exists (the method is not pure virtual)
}
}

This comment has been minimized.

@pnkfelix

pnkfelix Sep 12, 2014

Member

You left out the as_element(&self) implementation (i.e. override) for Element. Presumably it would look something like this:

impl<T:Element> Node for T {
    fn as_element<'a>(&'a self) -> Option<&'a Element> {
        Some(self as &Element)
    }
}

where there is nothing new about the above (i.e. its all just DST fat pointers; there are no fat objects here). Right?

fn more_code() {
// We assume we need videoElement to be a fat object. It would need to be
// to be stored in another element, for instance.
let videoElement:Rc<Fat<HTMLVideoElement,Element>> = Rc::new(fat(/*...*/));

This comment has been minimized.

@pnkfelix

pnkfelix Sep 12, 2014

Member

This line doesn't seem right to me. You said up above that given Fat<U,T>, then T must be a concrete type, and U must be an existential variant of that type. But here you have written Fat<HTMLVideoElement,Element>, and while HTMLVideoElement itself has not been given a definition, we know that the occurrence of Element there cannot be right, since Element is a trait, not a concrete type.

I suspect that either:

  • you meant to write Fat<Element,HTMLVideoElement> (i.e. you just swapped the order of the arguments), and thus HTMLVideoElement is implicitly a concrete type, or
  • in keeping with the conventions you established above, you actually meant to write Fat<HTMLVideoElement, HTMLVIdeoElementFields>, and implicitly HTMLVideoElement is some trait that indirectly extends the trait Element.

It seems to me like the second bullet is the more likely one, but it would be good to clarify either way.


Some comments on this:

First, note that for functions that take borrowed pointers as parameters, we still use fat pointers. There really won't be much reason to do otherwise--the cost of the virtual call will be roughly the same, and using the fat pointer is more flexible because you can always convert a thin pointer into a fat one, but not vice versa. The reason to use thin pointers is if you're going have more pointers than objects, thin pointers to fat objects saves space. This would usually arise for struct fields rather than function parameters--if you're nesting function calls deeply enough for this to matter, you've probably got other problems.

This comment has been minimized.

@pnkfelix

pnkfelix Sep 12, 2014

Member

"using the fat pointer is more flexible because you can always convert a thin pointer into a fat one, but not vice versa"

Flexibility is in the eye of the beholder here, no? In particular: Taking the fat pointer provides flexibility for the clients of a method (because they will be able to call it regardless of which they are given), but it restricts the method itself: the method itself will not be able to convert the given fat pointer to a DST object into a thin pointer to a fat object.

I assume that some methods will need to work with the thin pointers (namely methods expected to construct new instances of types holding thin pointers to fat objects). So the rule of thumb to always to take a fat pointer for borrowed pointer parameters will not always apply.

@MicahChalmer

This comment has been minimized.

Copy link
Author

MicahChalmer commented Sep 13, 2014

I did some corrections to the errors @pnkfelix noted. I think the RFC as a whole could be improved by choosing a different version of single inheritance. But I don't know that it's necessary--I think the main idea has reached where it needs to reach.

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

@Ericson2314 Ericson2314 referenced this pull request Sep 18, 2014

Closed

Allocator RFC, take II #244

@rpjohnst rpjohnst referenced this pull request Sep 22, 2014

Closed

Layout Inheritance #254

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Sep 25, 2014

The team did a recent review of all of the RFC PR's around inheritance:

During that meeting we determined that this RFC has a promising idea, so promising that it was then duplicated into other RFC's that are under consideration.

Since the good idea have propagated elsewhere, and this RFC on its own does not resolve all of the issues that we need to address (in particular, note the issue with upcasting from a trait to a super-trait via the same thin pointer, noted on a different discuss thread), we are going to close this RFC PR. Note again that this is not because we think the core idea itself is bad, but rather because we think this PR has served its purpose well. You can see some critiques of the details of this RFC here, but that is relatively unimportant.

@pnkfelix pnkfelix closed this Sep 25, 2014

withoutboats pushed a commit to withoutboats/rfcs that referenced this pull request Jan 15, 2017

Merge pull request rust-lang#9 from MarkusJais/master
fixed 2 typos in documention for Task

dhardy pushed a commit to dhardy/rfcs that referenced this pull request Jul 5, 2017

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.