From aeba87833584e920dcde5be20dc458226e1d2ac2 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Thu, 17 Jul 2025 14:58:34 -0400 Subject: [PATCH] Clean up some TODO and other comments (#5813) --- toolchain/check/convert.cpp | 2 - toolchain/check/convert.h | 2 +- toolchain/check/handle_operator.cpp | 1 - .../testdata/class/virtual_modifiers.carbon | 112 +++++++++--------- toolchain/lower/testdata/class/virtual.carbon | 11 +- 5 files changed, 62 insertions(+), 66 deletions(-) diff --git a/toolchain/check/convert.cpp b/toolchain/check/convert.cpp index 37e00f0d81e6..01e226df61aa 100644 --- a/toolchain/check/convert.cpp +++ b/toolchain/check/convert.cpp @@ -604,8 +604,6 @@ static auto ConvertStructToClass( auto result_id = ConvertStructToStructOrClass( context, src_type, dest_struct_type, value_id, target, - // TODO: Pass down the specific_id of the passed in - // dest_vtable_ptr_inst_id, or from the dest_type.specific_id. dest_vtable_ptr_inst_id); if (need_temporary) { diff --git a/toolchain/check/convert.h b/toolchain/check/convert.h index 01f3a63cf88e..651f3fb16b2e 100644 --- a/toolchain/check/convert.h +++ b/toolchain/check/convert.h @@ -60,7 +60,7 @@ struct ConversionTarget { // TODO: The `vtable_id` parameter is too much of a special case here, and // should be removed - once partial classes are implemented, the vtable pointer // initialization will be done not in this conversion, but during initialization -// of the object of non-partial class time from the object of partial class +// of the object of non-partial class type from the object of partial class // type. auto Convert(Context& context, SemIR::LocId loc_id, SemIR::InstId expr_id, ConversionTarget target, diff --git a/toolchain/check/handle_operator.cpp b/toolchain/check/handle_operator.cpp index 511c005472c3..7e4caf2ebf98 100644 --- a/toolchain/check/handle_operator.cpp +++ b/toolchain/check/handle_operator.cpp @@ -325,7 +325,6 @@ auto HandleParseNode(Context& context, Parse::PrefixOperatorPartialId node_id) context.emitter().Emit(node_id, PartialOnFinal, inner_type.type_id); } - // TODO: Add diagnostics for partial applied to non-base/abstract types. AddInstAndPush( context, node_id, {.type_id = SemIR::TypeType::TypeId, .inner_id = inner_type.inst_id}); diff --git a/toolchain/check/testdata/class/virtual_modifiers.carbon b/toolchain/check/testdata/class/virtual_modifiers.carbon index 721a264c3e94..5d0dfe0656c5 100644 --- a/toolchain/check/testdata/class/virtual_modifiers.carbon +++ b/toolchain/check/testdata/class/virtual_modifiers.carbon @@ -124,11 +124,9 @@ base class Base { fn F() { var i: i32 = 3; - // TODO: These should initialize element1 (.m), not element0 (the vptr) var b1: Base = {.m2 = i, .m1 = i}; var b2: Base = {.m2 = 3, .m1 = 5}; - // This one is good, though. b1.m2 = 4; } @@ -1458,74 +1456,74 @@ var v: Base(T1) = {}; // CHECK:STDOUT: %b1.var_patt: %pattern_type.bcc = var_pattern %b1.patt [concrete] // CHECK:STDOUT: } // CHECK:STDOUT: %b1.var: ref %Base = var %b1.var_patt -// CHECK:STDOUT: %i.ref.loc14_25: ref %i32 = name_ref i, %i -// CHECK:STDOUT: %i.ref.loc14_34: ref %i32 = name_ref i, %i -// CHECK:STDOUT: %.loc14_35.1: %struct_type.m2.m1.68c = struct_literal (%i.ref.loc14_25, %i.ref.loc14_34) -// CHECK:STDOUT: %.loc14_35.2: ref %ptr.454 = class_element_access %b1.var, element0 -// CHECK:STDOUT: %.loc14_35.3: init %ptr.454 = initialize_from @Base.%vtable_ptr to %.loc14_35.2 [concrete = constants.%Base.vtable_ptr] -// CHECK:STDOUT: %.loc14_34: %i32 = bind_value %i.ref.loc14_34 -// CHECK:STDOUT: %.loc14_35.4: ref %i32 = class_element_access %b1.var, element2 -// CHECK:STDOUT: %.loc14_35.5: init %i32 = initialize_from %.loc14_34 to %.loc14_35.4 -// CHECK:STDOUT: %.loc14_25: %i32 = bind_value %i.ref.loc14_25 -// CHECK:STDOUT: %.loc14_35.6: ref %i32 = class_element_access %b1.var, element1 -// CHECK:STDOUT: %.loc14_35.7: init %i32 = initialize_from %.loc14_25 to %.loc14_35.6 -// CHECK:STDOUT: %.loc14_35.8: init %Base = class_init (%.loc14_35.3, %.loc14_35.5, %.loc14_35.7), %b1.var -// CHECK:STDOUT: %.loc14_3: init %Base = converted %.loc14_35.1, %.loc14_35.8 -// CHECK:STDOUT: assign %b1.var, %.loc14_3 -// CHECK:STDOUT: %Base.ref.loc14: type = name_ref Base, file.%Base.decl [concrete = constants.%Base] +// CHECK:STDOUT: %i.ref.loc13_25: ref %i32 = name_ref i, %i +// CHECK:STDOUT: %i.ref.loc13_34: ref %i32 = name_ref i, %i +// CHECK:STDOUT: %.loc13_35.1: %struct_type.m2.m1.68c = struct_literal (%i.ref.loc13_25, %i.ref.loc13_34) +// CHECK:STDOUT: %.loc13_35.2: ref %ptr.454 = class_element_access %b1.var, element0 +// CHECK:STDOUT: %.loc13_35.3: init %ptr.454 = initialize_from @Base.%vtable_ptr to %.loc13_35.2 [concrete = constants.%Base.vtable_ptr] +// CHECK:STDOUT: %.loc13_34: %i32 = bind_value %i.ref.loc13_34 +// CHECK:STDOUT: %.loc13_35.4: ref %i32 = class_element_access %b1.var, element2 +// CHECK:STDOUT: %.loc13_35.5: init %i32 = initialize_from %.loc13_34 to %.loc13_35.4 +// CHECK:STDOUT: %.loc13_25: %i32 = bind_value %i.ref.loc13_25 +// CHECK:STDOUT: %.loc13_35.6: ref %i32 = class_element_access %b1.var, element1 +// CHECK:STDOUT: %.loc13_35.7: init %i32 = initialize_from %.loc13_25 to %.loc13_35.6 +// CHECK:STDOUT: %.loc13_35.8: init %Base = class_init (%.loc13_35.3, %.loc13_35.5, %.loc13_35.7), %b1.var +// CHECK:STDOUT: %.loc13_3: init %Base = converted %.loc13_35.1, %.loc13_35.8 +// CHECK:STDOUT: assign %b1.var, %.loc13_3 +// CHECK:STDOUT: %Base.ref.loc13: type = name_ref Base, file.%Base.decl [concrete = constants.%Base] // CHECK:STDOUT: %b1: ref %Base = bind_name b1, %b1.var // CHECK:STDOUT: name_binding_decl { // CHECK:STDOUT: %b2.patt: %pattern_type.bcc = binding_pattern b2 [concrete] // CHECK:STDOUT: %b2.var_patt: %pattern_type.bcc = var_pattern %b2.patt [concrete] // CHECK:STDOUT: } // CHECK:STDOUT: %b2.var: ref %Base = var %b2.var_patt -// CHECK:STDOUT: %int_3.loc15: Core.IntLiteral = int_value 3 [concrete = constants.%int_3.1ba] +// CHECK:STDOUT: %int_3.loc14: Core.IntLiteral = int_value 3 [concrete = constants.%int_3.1ba] // CHECK:STDOUT: %int_5: Core.IntLiteral = int_value 5 [concrete = constants.%int_5.64b] -// CHECK:STDOUT: %.loc15_35.1: %struct_type.m2.m1.5f2 = struct_literal (%int_3.loc15, %int_5) -// CHECK:STDOUT: %.loc15_35.2: ref %ptr.454 = class_element_access %b2.var, element0 -// CHECK:STDOUT: %.loc15_35.3: init %ptr.454 = initialize_from @Base.%vtable_ptr to %.loc15_35.2 [concrete = constants.%Base.vtable_ptr] -// CHECK:STDOUT: %impl.elem0.loc15_35.1: %.9c3 = impl_witness_access constants.%ImplicitAs.impl_witness.c75, element0 [concrete = constants.%Convert.956] -// CHECK:STDOUT: %bound_method.loc15_35.1: = bound_method %int_5, %impl.elem0.loc15_35.1 [concrete = constants.%Convert.bound.4e6] -// CHECK:STDOUT: %specific_fn.loc15_35.1: = specific_function %impl.elem0.loc15_35.1, @Convert.2(constants.%int_32) [concrete = constants.%Convert.specific_fn] -// CHECK:STDOUT: %bound_method.loc15_35.2: = bound_method %int_5, %specific_fn.loc15_35.1 [concrete = constants.%bound_method.a25] -// CHECK:STDOUT: %int.convert_checked.loc15_35.1: init %i32 = call %bound_method.loc15_35.2(%int_5) [concrete = constants.%int_5.0f6] -// CHECK:STDOUT: %.loc15_35.4: init %i32 = converted %int_5, %int.convert_checked.loc15_35.1 [concrete = constants.%int_5.0f6] -// CHECK:STDOUT: %.loc15_35.5: ref %i32 = class_element_access %b2.var, element2 -// CHECK:STDOUT: %.loc15_35.6: init %i32 = initialize_from %.loc15_35.4 to %.loc15_35.5 [concrete = constants.%int_5.0f6] -// CHECK:STDOUT: %impl.elem0.loc15_35.2: %.9c3 = impl_witness_access constants.%ImplicitAs.impl_witness.c75, element0 [concrete = constants.%Convert.956] -// CHECK:STDOUT: %bound_method.loc15_35.3: = bound_method %int_3.loc15, %impl.elem0.loc15_35.2 [concrete = constants.%Convert.bound.b30] -// CHECK:STDOUT: %specific_fn.loc15_35.2: = specific_function %impl.elem0.loc15_35.2, @Convert.2(constants.%int_32) [concrete = constants.%Convert.specific_fn] -// CHECK:STDOUT: %bound_method.loc15_35.4: = bound_method %int_3.loc15, %specific_fn.loc15_35.2 [concrete = constants.%bound_method.047] -// CHECK:STDOUT: %int.convert_checked.loc15_35.2: init %i32 = call %bound_method.loc15_35.4(%int_3.loc15) [concrete = constants.%int_3.822] -// CHECK:STDOUT: %.loc15_35.7: init %i32 = converted %int_3.loc15, %int.convert_checked.loc15_35.2 [concrete = constants.%int_3.822] -// CHECK:STDOUT: %.loc15_35.8: ref %i32 = class_element_access %b2.var, element1 -// CHECK:STDOUT: %.loc15_35.9: init %i32 = initialize_from %.loc15_35.7 to %.loc15_35.8 [concrete = constants.%int_3.822] -// CHECK:STDOUT: %.loc15_35.10: init %Base = class_init (%.loc15_35.3, %.loc15_35.6, %.loc15_35.9), %b2.var [concrete = constants.%Base.val] -// CHECK:STDOUT: %.loc15_3: init %Base = converted %.loc15_35.1, %.loc15_35.10 [concrete = constants.%Base.val] -// CHECK:STDOUT: assign %b2.var, %.loc15_3 -// CHECK:STDOUT: %Base.ref.loc15: type = name_ref Base, file.%Base.decl [concrete = constants.%Base] +// CHECK:STDOUT: %.loc14_35.1: %struct_type.m2.m1.5f2 = struct_literal (%int_3.loc14, %int_5) +// CHECK:STDOUT: %.loc14_35.2: ref %ptr.454 = class_element_access %b2.var, element0 +// CHECK:STDOUT: %.loc14_35.3: init %ptr.454 = initialize_from @Base.%vtable_ptr to %.loc14_35.2 [concrete = constants.%Base.vtable_ptr] +// CHECK:STDOUT: %impl.elem0.loc14_35.1: %.9c3 = impl_witness_access constants.%ImplicitAs.impl_witness.c75, element0 [concrete = constants.%Convert.956] +// CHECK:STDOUT: %bound_method.loc14_35.1: = bound_method %int_5, %impl.elem0.loc14_35.1 [concrete = constants.%Convert.bound.4e6] +// CHECK:STDOUT: %specific_fn.loc14_35.1: = specific_function %impl.elem0.loc14_35.1, @Convert.2(constants.%int_32) [concrete = constants.%Convert.specific_fn] +// CHECK:STDOUT: %bound_method.loc14_35.2: = bound_method %int_5, %specific_fn.loc14_35.1 [concrete = constants.%bound_method.a25] +// CHECK:STDOUT: %int.convert_checked.loc14_35.1: init %i32 = call %bound_method.loc14_35.2(%int_5) [concrete = constants.%int_5.0f6] +// CHECK:STDOUT: %.loc14_35.4: init %i32 = converted %int_5, %int.convert_checked.loc14_35.1 [concrete = constants.%int_5.0f6] +// CHECK:STDOUT: %.loc14_35.5: ref %i32 = class_element_access %b2.var, element2 +// CHECK:STDOUT: %.loc14_35.6: init %i32 = initialize_from %.loc14_35.4 to %.loc14_35.5 [concrete = constants.%int_5.0f6] +// CHECK:STDOUT: %impl.elem0.loc14_35.2: %.9c3 = impl_witness_access constants.%ImplicitAs.impl_witness.c75, element0 [concrete = constants.%Convert.956] +// CHECK:STDOUT: %bound_method.loc14_35.3: = bound_method %int_3.loc14, %impl.elem0.loc14_35.2 [concrete = constants.%Convert.bound.b30] +// CHECK:STDOUT: %specific_fn.loc14_35.2: = specific_function %impl.elem0.loc14_35.2, @Convert.2(constants.%int_32) [concrete = constants.%Convert.specific_fn] +// CHECK:STDOUT: %bound_method.loc14_35.4: = bound_method %int_3.loc14, %specific_fn.loc14_35.2 [concrete = constants.%bound_method.047] +// CHECK:STDOUT: %int.convert_checked.loc14_35.2: init %i32 = call %bound_method.loc14_35.4(%int_3.loc14) [concrete = constants.%int_3.822] +// CHECK:STDOUT: %.loc14_35.7: init %i32 = converted %int_3.loc14, %int.convert_checked.loc14_35.2 [concrete = constants.%int_3.822] +// CHECK:STDOUT: %.loc14_35.8: ref %i32 = class_element_access %b2.var, element1 +// CHECK:STDOUT: %.loc14_35.9: init %i32 = initialize_from %.loc14_35.7 to %.loc14_35.8 [concrete = constants.%int_3.822] +// CHECK:STDOUT: %.loc14_35.10: init %Base = class_init (%.loc14_35.3, %.loc14_35.6, %.loc14_35.9), %b2.var [concrete = constants.%Base.val] +// CHECK:STDOUT: %.loc14_3: init %Base = converted %.loc14_35.1, %.loc14_35.10 [concrete = constants.%Base.val] +// CHECK:STDOUT: assign %b2.var, %.loc14_3 +// CHECK:STDOUT: %Base.ref.loc14: type = name_ref Base, file.%Base.decl [concrete = constants.%Base] // CHECK:STDOUT: %b2: ref %Base = bind_name b2, %b2.var // CHECK:STDOUT: %b1.ref: ref %Base = name_ref b1, %b1 // CHECK:STDOUT: %m2.ref: %Base.elem = name_ref m2, @Base.%.loc6 [concrete = @Base.%.loc6] -// CHECK:STDOUT: %.loc18_5: ref %i32 = class_element_access %b1.ref, element2 +// CHECK:STDOUT: %.loc16_5: ref %i32 = class_element_access %b1.ref, element2 // CHECK:STDOUT: %int_4: Core.IntLiteral = int_value 4 [concrete = constants.%int_4.0c1] -// CHECK:STDOUT: %impl.elem0.loc18: %.9c3 = impl_witness_access constants.%ImplicitAs.impl_witness.c75, element0 [concrete = constants.%Convert.956] -// CHECK:STDOUT: %bound_method.loc18_9.1: = bound_method %int_4, %impl.elem0.loc18 [concrete = constants.%Convert.bound.ac3] -// CHECK:STDOUT: %specific_fn.loc18: = specific_function %impl.elem0.loc18, @Convert.2(constants.%int_32) [concrete = constants.%Convert.specific_fn] -// CHECK:STDOUT: %bound_method.loc18_9.2: = bound_method %int_4, %specific_fn.loc18 [concrete = constants.%bound_method.1da] -// CHECK:STDOUT: %int.convert_checked.loc18: init %i32 = call %bound_method.loc18_9.2(%int_4) [concrete = constants.%int_4.940] -// CHECK:STDOUT: %.loc18_9: init %i32 = converted %int_4, %int.convert_checked.loc18 [concrete = constants.%int_4.940] -// CHECK:STDOUT: assign %.loc18_5, %.loc18_9 -// CHECK:STDOUT: %Op.bound.loc15: = bound_method %b2.var, constants.%Op.345 +// CHECK:STDOUT: %impl.elem0.loc16: %.9c3 = impl_witness_access constants.%ImplicitAs.impl_witness.c75, element0 [concrete = constants.%Convert.956] +// CHECK:STDOUT: %bound_method.loc16_9.1: = bound_method %int_4, %impl.elem0.loc16 [concrete = constants.%Convert.bound.ac3] +// CHECK:STDOUT: %specific_fn.loc16: = specific_function %impl.elem0.loc16, @Convert.2(constants.%int_32) [concrete = constants.%Convert.specific_fn] +// CHECK:STDOUT: %bound_method.loc16_9.2: = bound_method %int_4, %specific_fn.loc16 [concrete = constants.%bound_method.1da] +// CHECK:STDOUT: %int.convert_checked.loc16: init %i32 = call %bound_method.loc16_9.2(%int_4) [concrete = constants.%int_4.940] +// CHECK:STDOUT: %.loc16_9: init %i32 = converted %int_4, %int.convert_checked.loc16 [concrete = constants.%int_4.940] +// CHECK:STDOUT: assign %.loc16_5, %.loc16_9 +// CHECK:STDOUT: %Op.bound.loc14: = bound_method %b2.var, constants.%Op.345 // CHECK:STDOUT: %Op.specific_fn.1: = specific_function constants.%Op.345, @Op.3(constants.%Base) [concrete = constants.%Op.specific_fn.083] -// CHECK:STDOUT: %bound_method.loc15_3: = bound_method %b2.var, %Op.specific_fn.1 -// CHECK:STDOUT: %addr.loc15: %ptr.11f = addr_of %b2.var -// CHECK:STDOUT: %no_op.loc15: init %empty_tuple.type = call %bound_method.loc15_3(%addr.loc15) -// CHECK:STDOUT: %Op.bound.loc14: = bound_method %b1.var, constants.%Op.345 +// CHECK:STDOUT: %bound_method.loc14_3: = bound_method %b2.var, %Op.specific_fn.1 +// CHECK:STDOUT: %addr.loc14: %ptr.11f = addr_of %b2.var +// CHECK:STDOUT: %no_op.loc14: init %empty_tuple.type = call %bound_method.loc14_3(%addr.loc14) +// CHECK:STDOUT: %Op.bound.loc13: = bound_method %b1.var, constants.%Op.345 // CHECK:STDOUT: %Op.specific_fn.2: = specific_function constants.%Op.345, @Op.3(constants.%Base) [concrete = constants.%Op.specific_fn.083] -// CHECK:STDOUT: %bound_method.loc14: = bound_method %b1.var, %Op.specific_fn.2 -// CHECK:STDOUT: %addr.loc14: %ptr.11f = addr_of %b1.var -// CHECK:STDOUT: %no_op.loc14: init %empty_tuple.type = call %bound_method.loc14(%addr.loc14) +// CHECK:STDOUT: %bound_method.loc13: = bound_method %b1.var, %Op.specific_fn.2 +// CHECK:STDOUT: %addr.loc13: %ptr.11f = addr_of %b1.var +// CHECK:STDOUT: %no_op.loc13: init %empty_tuple.type = call %bound_method.loc13(%addr.loc13) // CHECK:STDOUT: %Op.bound.loc12: = bound_method %i.var, constants.%Op.e6a // CHECK:STDOUT: %Op.specific_fn.3: = specific_function constants.%Op.e6a, @Op.3(constants.%i32) [concrete = constants.%Op.specific_fn.014] // CHECK:STDOUT: %bound_method.loc12_3.3: = bound_method %i.var, %Op.specific_fn.3 diff --git a/toolchain/lower/testdata/class/virtual.carbon b/toolchain/lower/testdata/class/virtual.carbon index 43eac2e515a4..3d4c18005756 100644 --- a/toolchain/lower/testdata/class/virtual.carbon +++ b/toolchain/lower/testdata/class/virtual.carbon @@ -37,7 +37,8 @@ fn Create() { var b: Classes.Base = {}; var i: Classes.Intermediate = {.base = {}}; var d: Classes.Derived = {.base = {.base = {}}}; - // TODO: Support vptr initialization without explicit source initializers. + // Implicit initialization creates an object with the unformed state, which + // doesn't include vptr initialization. var d2: Classes.Derived; } @@ -254,14 +255,14 @@ fn Make() { // CHECK:STDOUT: !7 = !DILocation(line: 7, column: 3, scope: !4) // CHECK:STDOUT: !8 = !DILocation(line: 8, column: 3, scope: !4) // CHECK:STDOUT: !9 = !DILocation(line: 9, column: 3, scope: !4) -// CHECK:STDOUT: !10 = !DILocation(line: 11, column: 3, scope: !4) +// CHECK:STDOUT: !10 = !DILocation(line: 12, column: 3, scope: !4) // CHECK:STDOUT: !11 = !DILocation(line: 8, column: 33, scope: !4) // CHECK:STDOUT: !12 = !DILocation(line: 9, column: 28, scope: !4) // CHECK:STDOUT: !13 = !DILocation(line: 9, column: 37, scope: !4) // CHECK:STDOUT: !14 = !DILocation(line: 6, column: 1, scope: !4) -// CHECK:STDOUT: !15 = distinct !DISubprogram(name: "Use", linkageName: "_CUse.Create", scope: null, file: !3, line: 14, type: !5, spFlags: DISPFlagDefinition, unit: !2) -// CHECK:STDOUT: !16 = !DILocation(line: 15, column: 3, scope: !15) -// CHECK:STDOUT: !17 = !DILocation(line: 14, column: 1, scope: !15) +// CHECK:STDOUT: !15 = distinct !DISubprogram(name: "Use", linkageName: "_CUse.Create", scope: null, file: !3, line: 15, type: !5, spFlags: DISPFlagDefinition, unit: !2) +// CHECK:STDOUT: !16 = !DILocation(line: 16, column: 3, scope: !15) +// CHECK:STDOUT: !17 = !DILocation(line: 15, column: 1, scope: !15) // CHECK:STDOUT: ; ModuleID = 'member_init.carbon' // CHECK:STDOUT: source_filename = "member_init.carbon" // CHECK:STDOUT: