carbon-lang icon indicating copy to clipboard operation
carbon-lang copied to clipboard

Add vtable pointers to class layout

Open dwblaikie opened this issue 1 year ago • 2 comments

A small step to virtual functions - adding vtable pointers to the layout, but not initializing or otherwise using them at this stage.

A few open design questions I'd love feedback on:

  • Is this the right/good enough SemIR representation for now? This patch adds a is_dynamic attribute to SemIR::Class and populates/flags it based on the flag of the base class, or if any virtual function is declared in the class (or, at least that's my intent). Some other options include:
    • Each Class could store a ClassId (or TypeId?) of the (possibly indirect, possibly self) base class that is the first one that is dynamic/has a vtable pointer
    • Could make the property narrower, like has vtable pointer and have it true only on the type that introduces the vtable - then derived classes would have to walk their base classes to check if they're the one that needs to define the vtable pointer or not
  • Should the vtable be the first element in the type? If there's a non-dynamic base type, we could have a layout that's {<non-dynamic base type>, vtable ptr, <derived members>}? Derived types would still be able to uniquely identify where their vtable pointer is just fine... - and the vtable pointer is, in a sense, a member of that intermediate type, so it does seem a bit strange to force it to the front - but I guess it's probably more efficient in some ways?

Open to any other suggestions/advice/thoughts on the direction, etc.

dwblaikie avatar Oct 14 '24 18:10 dwblaikie

  • Is this the right/good enough SemIR representation for now? This patch adds a is_dynamic attribute to SemIR::Class and populates/flags it based on the flag of the base class, or if any virtual function is declared in the class (or, at least that's my intent).

I think storing this flag is reasonable; it seems like a property of the class that we will want to query relatively frequently. One other question we will frequently need to answer is "where is the vptr?" -- eg, when forming a virtual call or initializing an instance of the class -- so perhaps it'd make sense to also store some information about that, when we get to the point of wanting to access the vptr.

  • Should the vtable be the first element in the type? If there's a non-dynamic base type, we could have a layout that's {<non-dynamic base type>, vtable ptr, <derived members>}? Derived types would still be able to uniquely identify where their vtable pointer is just fine... - and the vtable pointer is, in a sense, a member of that intermediate type, so it does seem a bit strange to force it to the front - but I guess it's probably more efficient in some ways?

I think we've said that we want the derived <-> base conversion to be a no-op, which would force the vptr to not be at the start of the object in this case. However, that would force a significant ABI difference from C++, which might be motivation to reconsider and accept that derived <-> base conversions might add an offset. @josh11b may have further thoughts.

How hard would this be to revisit in future?

zygoloid avatar Oct 14 '24 20:10 zygoloid

  • Is this the right/good enough SemIR representation for now? This patch adds a is_dynamic attribute to SemIR::Class and populates/flags it based on the flag of the base class, or if any virtual function is declared in the class (or, at least that's my intent).

I think storing this flag is reasonable; it seems like a property of the class that we will want to query relatively frequently. One other question we will frequently need to answer is "where is the vptr?" -- eg, when forming a virtual call or initializing an instance of the class -- so perhaps it'd make sense to also store some information about that, when we get to the point of wanting to access the vptr.

Fair enough - yeah, can add more/change how we're doing this later. But sounds like you're on board with this as a first pass.

  • Should the vtable be the first element in the type? If there's a non-dynamic base type, we could have a layout that's {<non-dynamic base type>, vtable ptr, <derived members>}? Derived types would still be able to uniquely identify where their vtable pointer is just fine... - and the vtable pointer is, in a sense, a member of that intermediate type, so it does seem a bit strange to force it to the front - but I guess it's probably more efficient in some ways?

I think we've said that we want the derived <-> base conversion to be a no-op, which would force the vptr to not be at the start of the object in this case. However, that would force a significant ABI difference from C++, which might be motivation to reconsider and accept that derived <-> base conversions might add an offset. @josh11b may have further thoughts.

Oh, fair point... I'll see about making the change now to put the vptr after the base (I expect this'll be easy enough, and can flip back/forth in this review if folks have opinions).

How hard would this be to revisit in future?

Seems easy enough - don't have a stable ABI, so I think we can move it around as we see fit.

Mostly I'm asking pretty short-sightedly about design questions, just whatever questions/answers you folks want answered now/set us in the direction you'd like to head.

dwblaikie avatar Oct 14 '24 21:10 dwblaikie

I'd like to see some check tests here -- just to see the object representation type in the SemIR for some simple cases.

Looks like we do actually already have coverage of this part -- as you were :)

zygoloid avatar Oct 15 '24 23:10 zygoloid

(Maybe also a fail_todo_something case for initializing a class with a vptr, to show that it fails -- I'd expect a "missing initializer for field" error for now.)

Yep, on the money. Test added.

dwblaikie avatar Oct 16 '24 18:10 dwblaikie