Nice work once again, just to let you know about the little bit of duplication I think the name resolver needs a little bit of thought to handle forward declared items within a block like this: https://github.com/Rust-GCC/gccrs/issues/531. It might be as simple as calling ResolveToplevelItems when doing block expr resolution. Depending on how we fix this it might feed back a way to avoid this little bit of duplication. Thanks --Phil On Sun, 22 Aug 2021 at 00:48, Mark Wielaard wrote: > Odd things happen if structs or unions have duplicate field names. > Emit an error when lowering an struct/union item or declaration > statement and a duplicate field name is detected. A new testcase > 'dup_fields.rs' checks an error is actually produced. > --- > > https://code.wildebeest.org/git/user/mjw/gccrs/commit/?h=dup_fields > > gcc/rust/hir/rust-ast-lower-item.h | 27 ++++++++++++++++++++++++ > gcc/rust/hir/rust-ast-lower-stmt.h | 27 ++++++++++++++++++++++++ > gcc/testsuite/rust/compile/dup_fields.rs | 23 ++++++++++++++++++++ > 3 files changed, 77 insertions(+) > create mode 100644 gcc/testsuite/rust/compile/dup_fields.rs > > diff --git a/gcc/rust/hir/rust-ast-lower-item.h > b/gcc/rust/hir/rust-ast-lower-item.h > index bcf83ee63b6..b324d0b5b8b 100644 > --- a/gcc/rust/hir/rust-ast-lower-item.h > +++ b/gcc/rust/hir/rust-ast-lower-item.h > @@ -176,6 +176,25 @@ public: > struct_decl.get_locus ()); > } > > + /* Checks whether the name of a field already exists. Returns true > + and produces an error if so. */ > + static bool struct_field_name_exists (std::vector > &fields, > + HIR::StructField &new_field) > + { > + for (auto &field : fields) > + { > + if (field.get_field_name ().compare (new_field.get_field_name ()) > == 0) > + { > + RichLocation r (new_field.get_locus ()); > + r.add_range (field.get_locus ()); > + rust_error_at (r, "duplicate field name %qs", > + field.get_field_name ().c_str ()); > + return true; > + } > + } > + return false; > + } > + > void visit (AST::StructStruct &struct_decl) override > { > std::vector > generic_params; > @@ -206,6 +225,10 @@ public: > std::unique_ptr > (type), vis, > field.get_locus (), > field.get_outer_attrs ()); > + > + if (struct_field_name_exists (fields, translated_field)) > + return false; > + > fields.push_back (std::move (translated_field)); > return true; > }); > @@ -258,6 +281,10 @@ public: > std::unique_ptr > (type), > vis, variant.get_locus (), > variant.get_outer_attrs ()); > + > + if (struct_field_name_exists (variants, translated_variant)) > + return false; > + > variants.push_back (std::move (translated_variant)); > return true; > }); > diff --git a/gcc/rust/hir/rust-ast-lower-stmt.h > b/gcc/rust/hir/rust-ast-lower-stmt.h > index c4c00ac0bee..1e72c8a2023 100644 > --- a/gcc/rust/hir/rust-ast-lower-stmt.h > +++ b/gcc/rust/hir/rust-ast-lower-stmt.h > @@ -161,6 +161,25 @@ public: > struct_decl.get_locus ()); > } > > + /* Checks whether the name of a field already exists. Returns true > + and produces an error if so. */ > + static bool struct_field_name_exists (std::vector > &fields, > + HIR::StructField &new_field) > + { > + for (auto &field : fields) > + { > + if (field.get_field_name ().compare (new_field.get_field_name ()) > == 0) > + { > + RichLocation r (new_field.get_locus ()); > + r.add_range (field.get_locus ()); > + rust_error_at (r, "duplicate field name %qs", > + field.get_field_name ().c_str ()); > + return true; > + } > + } > + return false; > + } > + > void visit (AST::StructStruct &struct_decl) override > { > std::vector > generic_params; > @@ -191,6 +210,10 @@ public: > std::unique_ptr > (type), vis, > field.get_locus (), > field.get_outer_attrs ()); > + > + if (struct_field_name_exists (fields, translated_field)) > + return false; > + > fields.push_back (std::move (translated_field)); > return true; > }); > @@ -242,6 +265,10 @@ public: > std::unique_ptr > (type), > vis, variant.get_locus (), > variant.get_outer_attrs ()); > + > + if (struct_field_name_exists (variants, translated_variant)) > + return false; > + > variants.push_back (std::move (translated_variant)); > return true; > }); > diff --git a/gcc/testsuite/rust/compile/dup_fields.rs > b/gcc/testsuite/rust/compile/dup_fields.rs > new file mode 100644 > index 00000000000..ab39955eca0 > --- /dev/null > +++ b/gcc/testsuite/rust/compile/dup_fields.rs > @@ -0,0 +1,23 @@ > +struct S { a: i32, b: i32, c: u8, a: i128 } > +// { dg-error "duplicate field" "" { target *-*-* } .-1 } > + > +union U > + { > + a: i32, > + b: i32, > + c: u8, > + b: char // { dg-error "duplicate field" "" { target *-*-* } } > + } > + > +fn main () > +{ > + struct SS { alpha: i32, beta: i32, gamma: u8, gamma: i128 } > + // { dg-error "duplicate field" "" { target *-*-* } .-1 } > + > + union UU > + { > + alpha: i32, beta: i32, > + gamma: u8, beta: char > + // { dg-error "duplicate field" "" { target *-*-* } .-1 } > + } > +} > -- > 2.32.0 > > -- > Gcc-rust mailing list > Gcc-rust@gcc.gnu.org > https://gcc.gnu.org/mailman/listinfo/gcc-rust >