Allow fields to be reordered in struct initialization. (#3346)

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<StringId, int32_t> 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<SemIR::StructTypeField>(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<SemIR::StructTypeField>(src_field_id);
+  for (auto [i, dest_field_id] : llvm::enumerate(dest_elem_fields)) {
     auto dest_field =
         sem_ir.nodes().GetAs<SemIR::StructTypeField>(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<SemIR::StructTypeField>(
+        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<SemIR::StructAccess, SemIR::StructAccess>(
             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;
     }

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 = {}};

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, <error>
+// 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, <error>
 // CHECK:STDOUT: }

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};

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: <function> = fn_decl @MakeI32
+// CHECK:STDOUT:   %MakeF64: <function> = fn_decl @MakeF64
+// CHECK:STDOUT:   %.loc10: type = ptr_type {.a: i32, .b: f64}
+// CHECK:STDOUT:   %F: <function> = 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: <function> = name_reference "MakeF64", file.%MakeF64
+// CHECK:STDOUT:   %.loc11_44.1: init f64 = call %MakeF64.ref()
+// CHECK:STDOUT:   %MakeI32.ref: <function> = 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: }

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)