diff --git a/toolchain/check/convert.cpp b/toolchain/check/convert.cpp index 1985e7b3a132..e67b38392c15 100644 --- a/toolchain/check/convert.cpp +++ b/toolchain/check/convert.cpp @@ -408,8 +408,8 @@ static auto ConvertStructToStruct(Context& context, SemIR::StructType src_type, } // Check that the structs are the same size. - // TODO: Check the field names are the same up to permutation, compute the - // permutation, and use it below. + // TODO: If not, include the name of the first source field that doesn't + // exist in the destination or vice versa in the diagnostic. if (src_elem_fields.size() != dest_elem_fields.size()) { CARBON_DIAGNOSTIC(StructInitElementCountMismatch, Error, "Cannot initialize struct of {0} element(s) from struct " @@ -420,6 +420,16 @@ static auto ConvertStructToStruct(Context& context, SemIR::StructType src_type, return SemIR::NodeId::BuiltinError; } + // Prepare to look up fields in the source by index. + llvm::SmallDenseMap src_field_indexes; + if (src_type.fields_id != dest_type.fields_id) { + for (auto [i, field_id] : llvm::enumerate(src_elem_fields)) { + auto [it, added] = src_field_indexes.insert( + {context.nodes().GetAs(field_id).name_id, i}); + CARBON_CHECK(added) << "Duplicate field in source structure"; + } + } + // If we're forming an initializer, then we want an initializer for each // element. Otherwise, we want a value representation for each element. // Perform a final destination store if we're performing an in-place @@ -436,22 +446,40 @@ static auto ConvertStructToStruct(Context& context, SemIR::StructType src_type, // of the source. // TODO: Annotate diagnostics coming from here with the element index. CopyOnWriteBlock new_block(sem_ir, literal_elems_id, src_elem_fields.size()); - for (auto [i, src_field_id, dest_field_id] : - llvm::enumerate(src_elem_fields, dest_elem_fields)) { - auto src_field = sem_ir.nodes().GetAs(src_field_id); + for (auto [i, dest_field_id] : llvm::enumerate(dest_elem_fields)) { auto dest_field = sem_ir.nodes().GetAs(dest_field_id); - if (src_field.name_id != dest_field.name_id) { - CARBON_DIAGNOSTIC( - StructInitFieldNameMismatch, Error, - "Mismatched names for field {0} in struct initialization: " - "source has field name `{1}`, destination has field name `{2}`.", - size_t, llvm::StringRef, llvm::StringRef); - context.emitter().Emit(value.parse_node(), StructInitFieldNameMismatch, - i + 1, sem_ir.strings().Get(src_field.name_id), - sem_ir.strings().Get(dest_field.name_id)); - return SemIR::NodeId::BuiltinError; + + // Find the matching source field. + auto src_field_index = i; + if (src_type.fields_id != dest_type.fields_id) { + auto src_field_it = src_field_indexes.find(dest_field.name_id); + if (src_field_it == src_field_indexes.end()) { + if (literal_elems_id.is_valid()) { + CARBON_DIAGNOSTIC( + StructInitMissingFieldInLiteral, Error, + "Missing value for field `{0}` in struct initialization.", + llvm::StringRef); + context.emitter().Emit(value.parse_node(), + StructInitMissingFieldInLiteral, + sem_ir.strings().Get(dest_field.name_id)); + } else { + CARBON_DIAGNOSTIC(StructInitMissingFieldInConversion, Error, + "Cannot convert from struct type `{0}` to `{1}`: " + "missing field `{2}` in source type.", + std::string, std::string, llvm::StringRef); + context.emitter().Emit(value.parse_node(), + StructInitMissingFieldInConversion, + sem_ir.StringifyType(value.type_id()), + sem_ir.StringifyType(target.type_id), + sem_ir.strings().Get(dest_field.name_id)); + } + return SemIR::NodeId::BuiltinError; + } + src_field_index = src_field_it->second; } + auto src_field = sem_ir.nodes().GetAs( + src_elem_fields[src_field_index]); // TODO: This call recurses back into conversion. Switch to an iterative // approach. @@ -459,7 +487,7 @@ static auto ConvertStructToStruct(Context& context, SemIR::StructType src_type, ConvertAggregateElement( context, value.parse_node(), value_id, src_field.field_type_id, literal_elems, inner_kind, target.init_id, dest_field.field_type_id, - target.init_block, i); + target.init_block, src_field_index); if (init_id == SemIR::NodeId::BuiltinError) { return SemIR::NodeId::BuiltinError; } diff --git a/toolchain/check/testdata/struct/fail_assign_nested.carbon b/toolchain/check/testdata/struct/fail_assign_nested.carbon index d3699c43acee..2185addfdf1a 100644 --- a/toolchain/check/testdata/struct/fail_assign_nested.carbon +++ b/toolchain/check/testdata/struct/fail_assign_nested.carbon @@ -4,7 +4,7 @@ // // AUTOUPDATE -// CHECK:STDERR: fail_assign_nested.carbon:[[@LINE+3]]:27: ERROR: Mismatched names for field 1 in struct initialization: source has field name `b`, destination has field name `a`. +// CHECK:STDERR: fail_assign_nested.carbon:[[@LINE+3]]:27: ERROR: Missing value for field `a` in struct initialization. // CHECK:STDERR: var x: {.a: {}} = {.b = {}}; // CHECK:STDERR: ^ var x: {.a: {}} = {.b = {}}; diff --git a/toolchain/check/testdata/struct/fail_field_name_mismatch.carbon b/toolchain/check/testdata/struct/fail_field_name_mismatch.carbon index 4f4e59a9d5f0..ec2c4898f5b6 100644 --- a/toolchain/check/testdata/struct/fail_field_name_mismatch.carbon +++ b/toolchain/check/testdata/struct/fail_field_name_mismatch.carbon @@ -4,11 +4,16 @@ // // AUTOUPDATE -// CHECK:STDERR: fail_field_name_mismatch.carbon:[[@LINE+3]]:27: ERROR: Mismatched names for field 1 in struct initialization: source has field name `b`, destination has field name `a`. +// CHECK:STDERR: fail_field_name_mismatch.carbon:[[@LINE+3]]:27: ERROR: Missing value for field `a` in struct initialization. // CHECK:STDERR: var x: {.a: i32} = {.b = 1}; // CHECK:STDERR: ^ var x: {.a: i32} = {.b = 1}; +// CHECK:STDERR: fail_field_name_mismatch.carbon:[[@LINE+3]]:20: ERROR: Cannot convert from struct type `{.a: i32}` to `{.b: i32}`: missing field `b` in source type. +// CHECK:STDERR: var y: {.b: i32} = x; +// CHECK:STDERR: ^ +var y: {.b: i32} = x; + // CHECK:STDOUT: file "fail_field_name_mismatch.carbon" { // CHECK:STDOUT: %.loc10_16: type = struct_type {.a: i32} // CHECK:STDOUT: %x.var: ref {.a: i32} = var "x" @@ -17,4 +22,9 @@ var x: {.a: i32} = {.b = 1}; // CHECK:STDOUT: %.loc10_27.1: type = struct_type {.b: i32} // CHECK:STDOUT: %.loc10_27.2: {.b: i32} = struct_literal (%.loc10_26) // CHECK:STDOUT: assign %x.var, +// CHECK:STDOUT: %.loc15: type = struct_type {.b: i32} +// CHECK:STDOUT: %y.var: ref {.b: i32} = var "y" +// CHECK:STDOUT: %y: ref {.b: i32} = bind_name "y", %y.var +// CHECK:STDOUT: %x.ref: ref {.a: i32} = name_reference "x", %x +// CHECK:STDOUT: assign %y.var, // CHECK:STDOUT: } diff --git a/toolchain/check/testdata/struct/fail_field_type_mismatch.carbon b/toolchain/check/testdata/struct/fail_field_type_mismatch.carbon index 8f5e4f71a08c..e3012e41943d 100644 --- a/toolchain/check/testdata/struct/fail_field_type_mismatch.carbon +++ b/toolchain/check/testdata/struct/fail_field_type_mismatch.carbon @@ -4,7 +4,7 @@ // // AUTOUPDATE -// CHECK:STDERR: fail_field_type_mismatch.carbon:[[@LINE+3]]:29: ERROR: Mismatched names for field 1 in struct initialization: source has field name `b`, destination has field name `a`. +// CHECK:STDERR: fail_field_type_mismatch.carbon:[[@LINE+3]]:29: ERROR: Missing value for field `a` in struct initialization. // CHECK:STDERR: var x: {.a: i32} = {.b = 1.0}; // CHECK:STDERR: ^ var x: {.a: i32} = {.b = 1.0}; diff --git a/toolchain/check/testdata/struct/reorder_fields.carbon b/toolchain/check/testdata/struct/reorder_fields.carbon new file mode 100644 index 000000000000..68f44f0a4ffa --- /dev/null +++ b/toolchain/check/testdata/struct/reorder_fields.carbon @@ -0,0 +1,60 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// AUTOUPDATE + +fn MakeI32() -> i32; +fn MakeF64() -> f64; + +fn F() -> {.a: i32, .b: f64} { + let x: {.a: i32, .b: f64} = {.b = MakeF64(), .a = MakeI32()}; + let y: {.b: f64, .a: i32} = x; + return y; +} + +// CHECK:STDOUT: file "reorder_fields.carbon" { +// CHECK:STDOUT: %MakeI32: = fn_decl @MakeI32 +// CHECK:STDOUT: %MakeF64: = fn_decl @MakeF64 +// CHECK:STDOUT: %.loc10: type = ptr_type {.a: i32, .b: f64} +// CHECK:STDOUT: %F: = fn_decl @F +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @MakeI32() -> i32; +// CHECK:STDOUT: +// CHECK:STDOUT: fn @MakeF64() -> f64; +// CHECK:STDOUT: +// CHECK:STDOUT: fn @F() -> %return: {.a: i32, .b: f64} { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: %.loc11_27: type = struct_type {.a: i32, .b: f64} +// CHECK:STDOUT: %MakeF64.ref: = name_reference "MakeF64", file.%MakeF64 +// CHECK:STDOUT: %.loc11_44.1: init f64 = call %MakeF64.ref() +// CHECK:STDOUT: %MakeI32.ref: = name_reference "MakeI32", file.%MakeI32 +// CHECK:STDOUT: %.loc11_60.1: init i32 = call %MakeI32.ref() +// CHECK:STDOUT: %.loc11_62.1: type = struct_type {.b: f64, .a: i32} +// CHECK:STDOUT: %.loc11_62.2: {.b: f64, .a: i32} = struct_literal (%.loc11_44.1, %.loc11_60.1) +// CHECK:STDOUT: %.loc11_60.2: ref i32 = temporary_storage +// CHECK:STDOUT: %.loc11_60.3: ref i32 = temporary %.loc11_60.2, %.loc11_60.1 +// CHECK:STDOUT: %.loc11_60.4: i32 = bind_value %.loc11_60.3 +// CHECK:STDOUT: %.loc11_44.2: ref f64 = temporary_storage +// CHECK:STDOUT: %.loc11_44.3: ref f64 = temporary %.loc11_44.2, %.loc11_44.1 +// CHECK:STDOUT: %.loc11_44.4: f64 = bind_value %.loc11_44.3 +// CHECK:STDOUT: %.loc11_62.3: {.a: i32, .b: f64} = struct_value %.loc11_62.2, (%.loc11_60.4, %.loc11_44.4) +// CHECK:STDOUT: %x: {.a: i32, .b: f64} = bind_name "x", %.loc11_62.3 +// CHECK:STDOUT: %.loc12_27: type = struct_type {.b: f64, .a: i32} +// CHECK:STDOUT: %.loc11_62.4: type = ptr_type {.b: f64, .a: i32} +// CHECK:STDOUT: %x.ref: {.a: i32, .b: f64} = name_reference "x", %x +// CHECK:STDOUT: %.loc12_31.1: f64 = struct_access %x.ref, member1 +// CHECK:STDOUT: %.loc12_31.2: i32 = struct_access %x.ref, member0 +// CHECK:STDOUT: %.loc12_31.3: {.b: f64, .a: i32} = struct_value %x.ref, (%.loc12_31.1, %.loc12_31.2) +// CHECK:STDOUT: %y: {.b: f64, .a: i32} = bind_name "y", %.loc12_31.3 +// CHECK:STDOUT: %y.ref: {.b: f64, .a: i32} = name_reference "y", %y +// CHECK:STDOUT: %.loc13_10.1: i32 = struct_access %y.ref, member1 +// CHECK:STDOUT: %.loc13_10.2: ref i32 = struct_access %return, member1 +// CHECK:STDOUT: %.loc13_10.3: init i32 = initialize_from %.loc13_10.1 to %.loc13_10.2 +// CHECK:STDOUT: %.loc13_10.4: f64 = struct_access %y.ref, member0 +// CHECK:STDOUT: %.loc13_10.5: ref f64 = struct_access %return, member0 +// CHECK:STDOUT: %.loc13_10.6: init f64 = initialize_from %.loc13_10.4 to %.loc13_10.5 +// CHECK:STDOUT: %.loc13_10.7: init {.a: i32, .b: f64} = struct_init %y.ref, (%.loc13_10.3, %.loc13_10.6) +// CHECK:STDOUT: return %.loc13_10.7 +// CHECK:STDOUT: } diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index 55a584b819f3..3d995b949b74 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -152,7 +152,8 @@ CARBON_DIAGNOSTIC_KIND(IndexOutOfBounds) CARBON_DIAGNOSTIC_KIND(InvalidMainRunSignature) CARBON_DIAGNOSTIC_KIND(SelfOutsideImplicitParameterList) CARBON_DIAGNOSTIC_KIND(StructInitElementCountMismatch) -CARBON_DIAGNOSTIC_KIND(StructInitFieldNameMismatch) +CARBON_DIAGNOSTIC_KIND(StructInitMissingFieldInLiteral) +CARBON_DIAGNOSTIC_KIND(StructInitMissingFieldInConversion) CARBON_DIAGNOSTIC_KIND(TupleIndexIntegerLiteral) CARBON_DIAGNOSTIC_KIND(TupleInitElementCountMismatch) CARBON_DIAGNOSTIC_KIND(ReturnStatementDisallowExpression)