Procházet zdrojové kódy

Skip vptr when performing object initialization (#4490)

The cehck for vptr could be done differently (is this class dynamic and
its base class non-dynamic), and if we change the layout (to put the
vptr after any base) then this code will break (could add an assertion
that the vptr isn't present apart from at the start of the field list if
that'd seem worthwhile).

There's still later issues with lowering (if this patch causes crashes
in lowering, do they result in fuzz failures/need to be avoided before
this change is submitted?)
David Blaikie před 1 rokem
rodič
revize
d79a374ad4

+ 15 - 4
toolchain/check/convert.cpp

@@ -392,6 +392,12 @@ static auto ConvertStructToStructOrClass(Context& context,
   auto& sem_ir = context.sem_ir();
   auto& sem_ir = context.sem_ir();
   auto src_elem_fields = sem_ir.inst_blocks().Get(src_type.fields_id);
   auto src_elem_fields = sem_ir.inst_blocks().Get(src_type.fields_id);
   auto dest_elem_fields = sem_ir.inst_blocks().Get(dest_type.fields_id);
   auto dest_elem_fields = sem_ir.inst_blocks().Get(dest_type.fields_id);
+  bool dest_has_vptr =
+      !dest_elem_fields.empty() &&
+      sem_ir.insts()
+              .GetAs<SemIR::StructTypeField>(dest_elem_fields.front())
+              .name_id == SemIR::NameId::Vptr;
+  auto dest_elem_fields_size = dest_elem_fields.size() - dest_has_vptr;
 
 
   auto value = sem_ir.insts().Get(value_id);
   auto value = sem_ir.insts().Get(value_id);
   auto value_loc_id = sem_ir.insts().GetLocId(value_id);
   auto value_loc_id = sem_ir.insts().GetLocId(value_id);
@@ -411,14 +417,14 @@ static auto ConvertStructToStructOrClass(Context& context,
   // Check that the structs are the same size.
   // Check that the structs are the same size.
   // TODO: If not, include the name of the first source field that doesn't
   // TODO: If not, include the name of the first source field that doesn't
   // exist in the destination or vice versa in the diagnostic.
   // exist in the destination or vice versa in the diagnostic.
-  if (src_elem_fields.size() != dest_elem_fields.size()) {
+  if (src_elem_fields.size() != dest_elem_fields_size) {
     CARBON_DIAGNOSTIC(
     CARBON_DIAGNOSTIC(
         StructInitElementCountMismatch, Error,
         StructInitElementCountMismatch, Error,
         "cannot initialize {0:class|struct} with {1} field{1:s} from struct "
         "cannot initialize {0:class|struct} with {1} field{1:s} from struct "
         "with {2} field{2:s}",
         "with {2} field{2:s}",
         BoolAsSelect, IntAsSelect, IntAsSelect);
         BoolAsSelect, IntAsSelect, IntAsSelect);
     context.emitter().Emit(value_loc_id, StructInitElementCountMismatch,
     context.emitter().Emit(value_loc_id, StructInitElementCountMismatch,
-                           ToClass, dest_elem_fields.size(),
+                           ToClass, dest_elem_fields_size,
                            src_elem_fields.size());
                            src_elem_fields.size());
     return SemIR::InstId::BuiltinError;
     return SemIR::InstId::BuiltinError;
   }
   }
@@ -449,14 +455,19 @@ static auto ConvertStructToStructOrClass(Context& context,
   // of the source.
   // of the source.
   // TODO: Annotate diagnostics coming from here with the element index.
   // TODO: Annotate diagnostics coming from here with the element index.
   auto new_block =
   auto new_block =
-      literal_elems_id.is_valid()
+      literal_elems_id.is_valid() && !dest_has_vptr
           ? SemIR::CopyOnWriteInstBlock(sem_ir, literal_elems_id)
           ? SemIR::CopyOnWriteInstBlock(sem_ir, literal_elems_id)
           : SemIR::CopyOnWriteInstBlock(
           : SemIR::CopyOnWriteInstBlock(
                 sem_ir, SemIR::CopyOnWriteInstBlock::UninitializedBlock{
                 sem_ir, SemIR::CopyOnWriteInstBlock::UninitializedBlock{
-                            src_elem_fields.size()});
+                            dest_elem_fields.size()});
   for (auto [i, dest_field_id] : llvm::enumerate(dest_elem_fields)) {
   for (auto [i, dest_field_id] : llvm::enumerate(dest_elem_fields)) {
     auto dest_field =
     auto dest_field =
         sem_ir.insts().GetAs<SemIR::StructTypeField>(dest_field_id);
         sem_ir.insts().GetAs<SemIR::StructTypeField>(dest_field_id);
+    if (dest_field.name_id == SemIR::NameId::Vptr) {
+      // TODO: Initialize the vptr to point to a vtable.
+      new_block.Set(i, SemIR::InstId::BuiltinError);
+      continue;
+    }
 
 
     // Find the matching source field.
     // Find the matching source field.
     auto src_field_index = i;
     auto src_field_index = i;

+ 6 - 8
toolchain/check/testdata/class/virtual_modifiers.carbon

@@ -37,17 +37,13 @@ base class Derived {
   extend base: Modifiers.Base;
   extend base: Modifiers.Base;
 }
 }
 
 
-// --- fail_todo_init.carbon
+// --- init.carbon
 
 
 package Init;
 package Init;
 
 
 import Modifiers;
 import Modifiers;
 
 
 fn F() {
 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 from struct with 0 fields [StructInitElementCountMismatch]
-  // CHECK:STDERR:   var v: Modifiers.Base = {};
-  // CHECK:STDERR:                           ^~
   var v: Modifiers.Base = {};
   var v: Modifiers.Base = {};
 }
 }
 
 
@@ -190,7 +186,7 @@ fn F() {
 // CHECK:STDOUT:
 // CHECK:STDOUT:
 // CHECK:STDOUT: virtual fn @F();
 // CHECK:STDOUT: virtual fn @F();
 // CHECK:STDOUT:
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- fail_todo_init.carbon
+// CHECK:STDOUT: --- init.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %F.type: type = fn_type @F [template]
 // CHECK:STDOUT:   %F.type: type = fn_type @F [template]
@@ -243,8 +239,10 @@ fn F() {
 // CHECK:STDOUT:   %Base.ref: type = name_ref Base, imports.%import_ref.1 [template = constants.%Base]
 // 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.var: ref %Base = var v
 // CHECK:STDOUT:   %v: ref %Base = bind_name v, %v.var
 // CHECK:STDOUT:   %v: ref %Base = bind_name v, %v.var
-// CHECK:STDOUT:   %.loc11: %.6 = struct_literal ()
-// CHECK:STDOUT:   assign %v.var, <error>
+// CHECK:STDOUT:   %.loc7_28.1: %.6 = struct_literal ()
+// CHECK:STDOUT:   %.loc7_28.2: init %Base = class_init (<error>), %v.var [template = <error>]
+// CHECK:STDOUT:   %.loc7_29: init %Base = converted %.loc7_28.1, %.loc7_28.2 [template = <error>]
+// CHECK:STDOUT:   assign %v.var, %.loc7_29
 // CHECK:STDOUT:   return
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: