Add improved error when a field is redefined in a struct constructor

Fixes #2381 If a struct type is initialized with one of it's fields repeated it will currently issue an error at the use site. However we would like to give the rust error code and (like rustc) show both the specifications for the field to help the user diagnose the issue. We move the check after the index for the field has been established so we can look up if the field has already been defined and get it's location. We update the visit method to return if it has handled an error otherwise we then output a second fatal error as not all the field in the specification have been processed. gcc/rust/ChangeLog: * typecheck/rust-hir-type-check-struct-field.h: Allow visit to return a bool * typecheck/rust-hir-type-check-struct.cc: Improve check of repeat fields gcc/testsuite/ChangeLog: * rust/compile/repeated_constructor_fields.rs: Added case with constructor field repeated Signed-off-by: Robert Goss <[email protected]>
Rust-GCC · Jan 17, 2024 · 0bf6ce3 · 0bf6ce3
1 parent 6d85a80
commit 0bf6ce3
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 32 deletions.
diff --git a/gcc/rust/typecheck/rust-hir-type-check-struct-field.h b/gcc/rust/typecheck/rust-hir-type-check-struct-field.h
@@ -35,9 +35,9 @@ class TypeCheckStructExpr : public TypeCheckBase
 protected:
   void resolve (HIR::StructExprStructFields &struct_expr);
 
-  void visit (HIR::StructExprFieldIdentifierValue &field);
-  void visit (HIR::StructExprFieldIndexValue &field);
-  void visit (HIR::StructExprFieldIdentifier &field);
+  bool visit (HIR::StructExprFieldIdentifierValue &field);
+  bool visit (HIR::StructExprFieldIndexValue &field);
+  bool visit (HIR::StructExprFieldIdentifier &field);
 
 private:
   TypeCheckStructExpr (HIR::Expr *e);

diff --git a/gcc/rust/typecheck/rust-hir-type-check-struct.cc b/gcc/rust/typecheck/rust-hir-type-check-struct.cc
@@ -102,19 +102,26 @@ TypeCheckStructExpr::resolve (HIR::StructExprStructFields &struct_expr)
       switch (field->get_kind ())
 	{
 	case HIR::StructExprField::StructExprFieldKind::IDENTIFIER:
-	  visit (static_cast<HIR::StructExprFieldIdentifier &> (*field.get ()));
+	  ok = visit (
+	    static_cast<HIR::StructExprFieldIdentifier &> (*field.get ()));
 	  break;
 
 	case HIR::StructExprField::StructExprFieldKind::IDENTIFIER_VALUE:
-	  visit (
+	  ok = visit (
 	    static_cast<HIR::StructExprFieldIdentifierValue &> (*field.get ()));
 	  break;
 
 	case HIR::StructExprField::StructExprFieldKind::INDEX_VALUE:
-	  visit (static_cast<HIR::StructExprFieldIndexValue &> (*field.get ()));
+	  ok = visit (
+	    static_cast<HIR::StructExprFieldIndexValue &> (*field.get ()));
 	  break;
 	}
 
+      if (!ok)
+	{
+	  return;
+	}
+
       if (resolved_field_value_expr == nullptr)
 	{
 	  rust_fatal_error (field->get_locus (),
@@ -238,24 +245,30 @@ TypeCheckStructExpr::resolve (HIR::StructExprStructFields &struct_expr)
   resolved = struct_def;
 }
 
-void
+bool
 TypeCheckStructExpr::visit (HIR::StructExprFieldIdentifierValue &field)
 {
-  auto it = fields_assigned.find (field.field_name.as_string ());
-  if (it != fields_assigned.end ())
-    {
-      rust_fatal_error (field.get_locus (), "used more than once");
-      return;
-    }
-
   size_t field_index;
   TyTy::StructFieldType *field_type;
   bool ok = variant->lookup_field (field.field_name.as_string (), &field_type,
 				   &field_index);
   if (!ok)
     {
       rust_error_at (field.get_locus (), "unknown field");
-      return;
+      return true;
+    }
+
+  auto it = adtFieldIndexToField.find (field_index);
+  if (it != adtFieldIndexToField.end ())
+    {
+      rich_location repeat_location (line_table, field.get_locus ());
+      auto prev_field_locus = it->second->get_locus ();
+      repeat_location.add_range (prev_field_locus);
+
+      rust_error_at (repeat_location, ErrorCode::E0062,
+		     "field %<%s%> specified more than once",
+		     field.field_name.as_string ().c_str ());
+      return false;
     }
 
   TyTy::BaseType *value = TypeCheckExpr::Resolve (field.get_value ().get ());
@@ -273,26 +286,35 @@ TypeCheckStructExpr::visit (HIR::StructExprFieldIdentifierValue &field)
       fields_assigned.insert (field.field_name.as_string ());
       adtFieldIndexToField[field_index] = &field;
     }
+
+  return true;
 }
 
-void
+bool
 TypeCheckStructExpr::visit (HIR::StructExprFieldIndexValue &field)
 {
   std::string field_name (std::to_string (field.get_tuple_index ()));
-  auto it = fields_assigned.find (field_name);
-  if (it != fields_assigned.end ())
-    {
-      rust_fatal_error (field.get_locus (), "used more than once");
-      return;
-    }
 
   size_t field_index;
   TyTy::StructFieldType *field_type;
   bool ok = variant->lookup_field (field_name, &field_type, &field_index);
   if (!ok)
     {
       rust_error_at (field.get_locus (), "unknown field");
-      return;
+      return true;
+    }
+
+  auto it = adtFieldIndexToField.find (field_index);
+  if (it != adtFieldIndexToField.end ())
+    {
+      rich_location repeat_location (line_table, field.get_locus ());
+      auto prev_field_locus = it->second->get_locus ();
+      repeat_location.add_range (prev_field_locus);
+
+      rust_error_at (repeat_location, ErrorCode::E0062,
+		     "field %<%s%> specified more than once",
+		     field_name.c_str ());
+      return false;
     }
 
   TyTy::BaseType *value = TypeCheckExpr::Resolve (field.get_value ().get ());
@@ -310,26 +332,34 @@ TypeCheckStructExpr::visit (HIR::StructExprFieldIndexValue &field)
       fields_assigned.insert (field_name);
       adtFieldIndexToField[field_index] = &field;
     }
+
+  return true;
 }
 
-void
+bool
 TypeCheckStructExpr::visit (HIR::StructExprFieldIdentifier &field)
 {
-  auto it = fields_assigned.find (field.get_field_name ().as_string ());
-  if (it != fields_assigned.end ())
-    {
-      rust_fatal_error (field.get_locus (), "used more than once");
-      return;
-    }
-
   size_t field_index;
   TyTy::StructFieldType *field_type;
   bool ok = variant->lookup_field (field.get_field_name ().as_string (),
 				   &field_type, &field_index);
   if (!ok)
     {
       rust_error_at (field.get_locus (), "unknown field");
-      return;
+      return true;
+    }
+
+  auto it = adtFieldIndexToField.find (field_index);
+  if (it != adtFieldIndexToField.end ())
+    {
+      rich_location repeat_location (line_table, field.get_locus ());
+      auto prev_field_locus = it->second->get_locus ();
+      repeat_location.add_range (prev_field_locus);
+
+      rust_error_at (repeat_location, ErrorCode::E0062,
+		     "field %<%s%> specified more than once",
+		     field.get_field_name ().as_string ().c_str ());
+      return false;
     }
 
   // we can make the field look like a path expr to take advantage of existing
@@ -358,6 +388,8 @@ TypeCheckStructExpr::visit (HIR::StructExprFieldIdentifier &field)
       fields_assigned.insert (field.get_field_name ().as_string ());
       adtFieldIndexToField[field_index] = &field;
     }
+
+  return true;
 }
 
 } // namespace Resolver

diff --git a/gcc/testsuite/rust/compile/repeated_constructor_fields.rs b/gcc/testsuite/rust/compile/repeated_constructor_fields.rs
@@ -0,0 +1,10 @@
+struct Foo {
+    x: i32,
+}
+
+fn main() {
+    let x = Foo {
+        x: 0,
+        x: 0, // { dg-error "field 'x' specified more than once" }
+    };
+}