Skip to content

Commit

Permalink
Add vtable pointers to class layout (#4407)
Browse files Browse the repository at this point in the history 
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.

---------

Co-authored-by: Richard Smith <[email protected]>
  • Loading branch information
@dwblaikie @zygoloid
dwblaikie and zygoloid authored Oct 16, 2024
1 parent 3c58fb7 commit dfed743
Show file tree
Hide file tree
Showing 14 changed files with 317 additions and 14 deletions.
9 changes: 8 additions & 1 deletion common/array_stack.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,17 @@ class ArrayStack {
// Appends a value to the top array on the stack.
auto AppendToTop(ValueT value) -> void {
CARBON_CHECK(!array_offsets_.empty(),
"Must call PushArray before PushValue.");
"Must call PushArray before AppendToTop.");
values_.push_back(value);
}

// Prepends a value to the top array on the stack.
auto PrependToTop(ValueT value) -> void {
CARBON_CHECK(!array_offsets_.empty(),
"Must call PushArray before PrependToTop.");
values_.insert(values_.begin() + array_offsets_.back(), value);
}

// Adds multiple values to the top array on the stack.
auto AppendToTop(llvm::ArrayRef<ValueT> values) -> void {
CARBON_CHECK(!array_offsets_.empty(),
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/context.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,7 @@ class TypeCompleter {
case SemIR::BuiltinInstKind::BoundMethodType:
case SemIR::BuiltinInstKind::WitnessType:
case SemIR::BuiltinInstKind::SpecificFunctionType:
case SemIR::BuiltinInstKind::VtableType:
return MakeCopyValueRepr(type_id);

case SemIR::BuiltinInstKind::StringType:
Expand Down
37 changes: 32 additions & 5 deletions toolchain/check/handle_class.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,12 @@ auto HandleParseNode(Context& context, Parse::BaseDeclId node_id) -> bool {
.index = SemIR::ElementIndex(
context.args_type_info_stack().PeekCurrentBlockContents().size())});

if (base_info.type_id != SemIR::TypeId::Error) {
auto base_class_info = context.classes().Get(
context.types().GetAs<SemIR::ClassType>(base_info.type_id).class_id);
class_info.is_dynamic |= base_class_info.is_dynamic;
}

// Add a corresponding field to the object representation of the class.
// TODO: Consider whether we want to use `partial T` here.
// TODO: Should we diagnose if there are already any fields?
Expand Down Expand Up @@ -643,14 +649,36 @@ static auto CheckCompleteAdapterClassType(Context& context,
// Checks that the specified finished class definition is valid and builds and
// returns a corresponding complete type witness instruction.
static auto CheckCompleteClassType(Context& context, Parse::NodeId node_id,
SemIR::ClassId class_id,
SemIR::InstBlockId fields_id)
-> SemIR::InstId {
SemIR::ClassId class_id) -> SemIR::InstId {
auto& class_info = context.classes().Get(class_id);
if (class_info.adapt_id.is_valid()) {
auto fields_id = context.args_type_info_stack().Pop();

return CheckCompleteAdapterClassType(context, node_id, class_id, fields_id);
}

bool defining_vtable_ptr = class_info.is_dynamic;
if (class_info.base_id.is_valid()) {
auto base_info = context.insts().GetAs<SemIR::BaseDecl>(class_info.base_id);
// TODO: If the base class is template dependent, we will need to decide
// whether to add a vptr as part of instantiation.
if (auto* base_class_info = TryGetAsClass(context, base_info.base_type_id);
base_class_info && base_class_info->is_dynamic) {
defining_vtable_ptr = false;
}
}

if (defining_vtable_ptr) {
context.args_type_info_stack().AddFrontInstId(
context.AddInstInNoBlock<SemIR::StructTypeField>(
Parse::NodeId::Invalid,
{.name_id = SemIR::NameId::Vptr,
.field_type_id = context.GetPointerType(
context.GetBuiltinType(SemIR::BuiltinInstKind::VtableType))}));
}

auto fields_id = context.args_type_info_stack().Pop();

return context.AddInst<SemIR::CompleteTypeWitness>(
node_id,
{.type_id = context.GetBuiltinType(SemIR::BuiltinInstKind::WitnessType),
Expand All @@ -659,13 +687,12 @@ static auto CheckCompleteClassType(Context& context, Parse::NodeId node_id,

auto HandleParseNode(Context& context, Parse::ClassDefinitionId node_id)
-> bool {
auto fields_id = context.args_type_info_stack().Pop();
auto class_id =
context.node_stack().Pop<Parse::NodeKind::ClassDefinitionStart>();

// The class type is now fully defined. Compute its object representation.
auto complete_type_witness_id =
CheckCompleteClassType(context, node_id, class_id, fields_id);
CheckCompleteClassType(context, node_id, class_id);
auto& class_info = context.classes().Get(class_id);
class_info.complete_type_witness_id = complete_type_witness_id;

Expand Down
9 changes: 9 additions & 0 deletions toolchain/check/handle_function.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,15 @@ static auto BuildFunctionDecl(Context& context,
.Case(KeywordModifierSet::Impl,
SemIR::Function::VirtualModifier::Impl)
.Default(SemIR::Function::VirtualModifier::None);
if (virtual_modifier != SemIR::Function::VirtualModifier::None &&
parent_scope_inst) {
if (auto class_decl = parent_scope_inst->TryAs<SemIR::ClassDecl>()) {
auto& class_info = context.classes().Get(class_decl->class_id);
CARBON_CHECK(virtual_modifier != SemIR::Function::VirtualModifier::Impl ||
class_info.is_dynamic);
class_info.is_dynamic = true;
}
}
if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Interface)) {
// TODO: Once we are saving the modifiers for a function, add check that
// the function may only be defined if it is marked `default` or `final`.
Expand Down
7 changes: 7 additions & 0 deletions toolchain/check/inst_block_stack.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ class InstBlockStack {
insts_stack_.AppendToTop(inst_id);
}

// Adds the given instruction ID to the front of the block at the top of the
// stack.
auto AddFrontInstId(SemIR::InstId inst_id) -> void {
CARBON_CHECK(!empty(), "no current block");
insts_stack_.PrependToTop(inst_id);
}

// Returns whether the current block is statically reachable.
auto is_current_block_reachable() -> bool {
return id_stack_.back() != SemIR::InstBlockId::Unreachable;
Expand Down
2 changes: 2 additions & 0 deletions toolchain/check/testdata/basics/builtin_insts.carbon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
// CHECK:STDOUT: instSpecificFunctionType: {kind: BuiltinInst, arg0: SpecificFunctionType, type: typeTypeType}
// CHECK:STDOUT: instNamespaceType: {kind: BuiltinInst, arg0: NamespaceType, type: typeTypeType}
// CHECK:STDOUT: instWitnessType: {kind: BuiltinInst, arg0: WitnessType, type: typeTypeType}
// CHECK:STDOUT: instVtableType: {kind: BuiltinInst, arg0: VtableType, type: typeTypeType}
// CHECK:STDOUT: 'inst+0': {kind: Namespace, arg0: name_scope0, arg1: inst<invalid>, type: type(instNamespaceType)}
// CHECK:STDOUT: constant_values:
// CHECK:STDOUT: instTypeType: templateConstant(instTypeType)
Expand All @@ -53,6 +54,7 @@
// CHECK:STDOUT: instSpecificFunctionType: templateConstant(instSpecificFunctionType)
// CHECK:STDOUT: instNamespaceType: templateConstant(instNamespaceType)
// CHECK:STDOUT: instWitnessType: templateConstant(instWitnessType)
// CHECK:STDOUT: instVtableType: templateConstant(instVtableType)
// CHECK:STDOUT: 'inst+0': templateConstant(inst+0)
// CHECK:STDOUT: symbolic_constants: {}
// CHECK:STDOUT: inst_blocks:
Expand Down
5 changes: 4 additions & 1 deletion toolchain/check/testdata/class/fail_modifiers.carbon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ fn AbstractWithDefinition.G() {
// CHECK:STDOUT: %F: %F.type = struct_value () [template]
// CHECK:STDOUT: %G.type: type = fn_type @G [template]
// CHECK:STDOUT: %G: %G.type = struct_value () [template]
// CHECK:STDOUT: %.4: type = ptr_type <vtable> [template]
// CHECK:STDOUT: %.5: type = struct_type {.<vptr>: %.4} [template]
// CHECK:STDOUT: %.6: <witness> = complete_type_witness %.5 [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: imports {
Expand Down Expand Up @@ -195,7 +198,7 @@ fn AbstractWithDefinition.G() {
// CHECK:STDOUT: class @AbstractWithDefinition {
// CHECK:STDOUT: %F.decl: %F.type = fn_decl @F [template = constants.%F] {} {}
// CHECK:STDOUT: %G.decl: %G.type = fn_decl @G [template = constants.%G] {} {}
// CHECK:STDOUT: %.loc92: <witness> = complete_type_witness %.1 [template = constants.%.2]
// CHECK:STDOUT: %.loc92: <witness> = complete_type_witness %.5 [template = constants.%.6]
// CHECK:STDOUT:
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .Self = constants.%AbstractWithDefinition
Expand Down
175 changes: 170 additions & 5 deletions toolchain/check/testdata/class/virtual_modifiers.carbon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
// TIP: To dump output, run:
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/class/virtual_modifiers.carbon

// --- modifiers.carbon

package Modifiers;

base class Base {
virtual fn H();
Expand All @@ -23,7 +26,32 @@ abstract class Abstract {
impl fn L();
}

// CHECK:STDOUT: --- virtual_modifiers.carbon
// --- todo_fail_later_base.carbon

package FailLaterBase;

import Modifiers;

base class Derived {
virtual fn F();
extend base: Modifiers.Base;
}

// --- fail_todo_init.carbon

package Init;

import Modifiers;

fn F() {
// TODO: The vptr shouldn't be counted for programmer-facing behavior.
// CHECK:STDERR: fail_todo_init.carbon:[[@LINE+3]]:27: error: cannot initialize class with 1 field(s) from struct with 0 field(s).
// CHECK:STDERR: var v: Modifiers.Base = {};
// CHECK:STDERR: ^~
var v: Modifiers.Base = {};
}

// CHECK:STDOUT: --- modifiers.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %Base: type = class_type @Base [template]
Expand All @@ -32,8 +60,9 @@ abstract class Abstract {
// CHECK:STDOUT: %H: %H.type = struct_value () [template]
// CHECK:STDOUT: %I.type: type = fn_type @I [template]
// CHECK:STDOUT: %I: %I.type = struct_value () [template]
// CHECK:STDOUT: %.2: type = struct_type {} [template]
// CHECK:STDOUT: %.3: <witness> = complete_type_witness %.2 [template]
// CHECK:STDOUT: %.2: type = ptr_type <vtable> [template]
// CHECK:STDOUT: %.3: type = struct_type {.<vptr>: %.2} [template]
// CHECK:STDOUT: %.4: <witness> = complete_type_witness %.3 [template]
// CHECK:STDOUT: %Abstract: type = class_type @Abstract [template]
// CHECK:STDOUT: %J.type: type = fn_type @J [template]
// CHECK:STDOUT: %J: %J.type = struct_value () [template]
Expand Down Expand Up @@ -70,7 +99,7 @@ abstract class Abstract {
// CHECK:STDOUT: class @Base {
// CHECK:STDOUT: %H.decl: %H.type = fn_decl @H [template = constants.%H] {} {}
// CHECK:STDOUT: %I.decl: %I.type = fn_decl @I [template = constants.%I] {} {}
// CHECK:STDOUT: %.loc16: <witness> = complete_type_witness %.2 [template = constants.%.3]
// CHECK:STDOUT: %.loc8: <witness> = complete_type_witness %.3 [template = constants.%.4]
// CHECK:STDOUT:
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .Self = constants.%Base
Expand All @@ -82,7 +111,7 @@ abstract class Abstract {
// CHECK:STDOUT: %J.decl: %J.type = fn_decl @J [template = constants.%J] {} {}
// CHECK:STDOUT: %K.decl: %K.type = fn_decl @K [template = constants.%K] {} {}
// CHECK:STDOUT: %L.decl: %L.type = fn_decl @L [template = constants.%L] {} {}
// CHECK:STDOUT: %.loc24: <witness> = complete_type_witness %.2 [template = constants.%.3]
// CHECK:STDOUT: %.loc16: <witness> = complete_type_witness %.3 [template = constants.%.4]
// CHECK:STDOUT:
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .Self = constants.%Abstract
Expand All @@ -101,3 +130,139 @@ abstract class Abstract {
// CHECK:STDOUT:
// CHECK:STDOUT: impl fn @L();
// CHECK:STDOUT:
// CHECK:STDOUT: --- todo_fail_later_base.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %Derived: type = class_type @Derived [template]
// CHECK:STDOUT: %F.type: type = fn_type @F [template]
// CHECK:STDOUT: %.1: type = tuple_type () [template]
// CHECK:STDOUT: %F: %F.type = struct_value () [template]
// CHECK:STDOUT: %Base: type = class_type @Base [template]
// CHECK:STDOUT: %.2: type = ptr_type <vtable> [template]
// CHECK:STDOUT: %.3: type = struct_type {.<vptr>: %.2} [template]
// CHECK:STDOUT: %.4: <witness> = complete_type_witness %.3 [template]
// CHECK:STDOUT: %.5: type = ptr_type %.3 [template]
// CHECK:STDOUT: %.6: type = unbound_element_type %Derived, %Base [template]
// CHECK:STDOUT: %.7: type = struct_type {.<vptr>: %.2, .base: %Base} [template]
// CHECK:STDOUT: %.8: <witness> = complete_type_witness %.7 [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: imports {
// CHECK:STDOUT: %Core: <namespace> = namespace file.%Core.import, [template] {
// CHECK:STDOUT: import Core//prelude
// CHECK:STDOUT: import Core//prelude/operators
// CHECK:STDOUT: import Core//prelude/types
// CHECK:STDOUT: import Core//prelude/operators/arithmetic
// CHECK:STDOUT: import Core//prelude/operators/as
// CHECK:STDOUT: import Core//prelude/operators/bitwise
// CHECK:STDOUT: import Core//prelude/operators/comparison
// CHECK:STDOUT: import Core//prelude/types/bool
// CHECK:STDOUT: }
// CHECK:STDOUT: %Modifiers: <namespace> = namespace file.%Modifiers.import, [template] {
// CHECK:STDOUT: .Base = %import_ref.1
// CHECK:STDOUT: import Modifiers//default
// CHECK:STDOUT: }
// CHECK:STDOUT: %import_ref.1: type = import_ref Modifiers//default, inst+3, loaded [template = constants.%Base]
// CHECK:STDOUT: %import_ref.2 = import_ref Modifiers//default, inst+4, unloaded
// CHECK:STDOUT: %import_ref.3 = import_ref Modifiers//default, inst+5, unloaded
// CHECK:STDOUT: %import_ref.4 = import_ref Modifiers//default, inst+9, unloaded
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .Core = imports.%Core
// CHECK:STDOUT: .Modifiers = imports.%Modifiers
// CHECK:STDOUT: .Derived = %Derived.decl
// CHECK:STDOUT: }
// CHECK:STDOUT: %Core.import = import Core
// CHECK:STDOUT: %Modifiers.import = import Modifiers
// CHECK:STDOUT: %Derived.decl: type = class_decl @Derived [template = constants.%Derived] {} {}
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: class @Derived {
// CHECK:STDOUT: %F.decl: %F.type = fn_decl @F [template = constants.%F] {} {}
// CHECK:STDOUT: %Modifiers.ref: <namespace> = name_ref Modifiers, imports.%Modifiers [template = imports.%Modifiers]
// CHECK:STDOUT: %Base.ref: type = name_ref Base, imports.%import_ref.1 [template = constants.%Base]
// CHECK:STDOUT: %.loc8: %.6 = base_decl %Base, element0 [template]
// CHECK:STDOUT: %.loc9: <witness> = complete_type_witness %.7 [template = constants.%.8]
// CHECK:STDOUT:
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .Self = constants.%Derived
// CHECK:STDOUT: .F = %F.decl
// CHECK:STDOUT: .base = %.loc8
// CHECK:STDOUT: extend name_scope4
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: class @Base {
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .Self = imports.%import_ref.2
// CHECK:STDOUT: .H = imports.%import_ref.3
// CHECK:STDOUT: .I = imports.%import_ref.4
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: virtual fn @F();
// CHECK:STDOUT:
// CHECK:STDOUT: --- fail_todo_init.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %F.type: type = fn_type @F [template]
// CHECK:STDOUT: %.1: type = tuple_type () [template]
// CHECK:STDOUT: %F: %F.type = struct_value () [template]
// CHECK:STDOUT: %Base: type = class_type @Base [template]
// CHECK:STDOUT: %.2: type = ptr_type <vtable> [template]
// CHECK:STDOUT: %.3: type = struct_type {.<vptr>: %.2} [template]
// CHECK:STDOUT: %.4: <witness> = complete_type_witness %.3 [template]
// CHECK:STDOUT: %.5: type = ptr_type %.3 [template]
// CHECK:STDOUT: %.6: type = struct_type {} [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: imports {
// CHECK:STDOUT: %Core: <namespace> = namespace file.%Core.import, [template] {
// CHECK:STDOUT: import Core//prelude
// CHECK:STDOUT: import Core//prelude/operators
// CHECK:STDOUT: import Core//prelude/types
// CHECK:STDOUT: import Core//prelude/operators/arithmetic
// CHECK:STDOUT: import Core//prelude/operators/as
// CHECK:STDOUT: import Core//prelude/operators/bitwise
// CHECK:STDOUT: import Core//prelude/operators/comparison
// CHECK:STDOUT: import Core//prelude/types/bool
// CHECK:STDOUT: }
// CHECK:STDOUT: %Modifiers: <namespace> = namespace file.%Modifiers.import, [template] {
// CHECK:STDOUT: .Base = %import_ref.1
// CHECK:STDOUT: import Modifiers//default
// CHECK:STDOUT: }
// CHECK:STDOUT: %import_ref.1: type = import_ref Modifiers//default, inst+3, loaded [template = constants.%Base]
// CHECK:STDOUT: %import_ref.2 = import_ref Modifiers//default, inst+4, unloaded
// CHECK:STDOUT: %import_ref.3 = import_ref Modifiers//default, inst+5, unloaded
// CHECK:STDOUT: %import_ref.4 = import_ref Modifiers//default, inst+9, unloaded
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .Core = imports.%Core
// CHECK:STDOUT: .Modifiers = imports.%Modifiers
// CHECK:STDOUT: .F = %F.decl
// CHECK:STDOUT: }
// CHECK:STDOUT: %Core.import = import Core
// CHECK:STDOUT: %Modifiers.import = import Modifiers
// CHECK:STDOUT: %F.decl: %F.type = fn_decl @F [template = constants.%F] {} {}
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: class @Base {
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .Self = imports.%import_ref.2
// CHECK:STDOUT: .H = imports.%import_ref.3
// CHECK:STDOUT: .I = imports.%import_ref.4
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @F() {
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: %Modifiers.ref: <namespace> = name_ref Modifiers, imports.%Modifiers [template = imports.%Modifiers]
// CHECK:STDOUT: %Base.ref: type = name_ref Base, imports.%import_ref.1 [template = constants.%Base]
// CHECK:STDOUT: %v.var: ref %Base = var v
// CHECK:STDOUT: %v: ref %Base = bind_name v, %v.var
// CHECK:STDOUT: %.loc11: %.6 = struct_literal ()
// CHECK:STDOUT: assign %v.var, <error>
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
2 changes: 2 additions & 0 deletions toolchain/lower/file_context.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,8 @@ static auto BuildTypeForInst(FileContext& context, SemIR::BuiltinInst inst)
case SemIR::BuiltinInstKind::WitnessType:
// Return an empty struct as a placeholder.
return llvm::StructType::get(context.llvm_context());
case SemIR::BuiltinInstKind::VtableType:
return llvm::Type::getVoidTy(context.llvm_context());
}
}

Expand Down
Loading

0 comments on commit dfed743

Please sign in to comment.