Add improved error when no fields in initializer

[robertgoss]

Fixes #2389

If a struct type with a variant that has fields is initialized with some fields the expression HIR StructExprStructFields is checked that all the fields are assigned. However, if no fields are initialized the HIR StructExprStruct is generated. This doesn't check if the struct is a unit during typechekc and only fails at the compile stage with a ICE.

Add a check at the typecheck stage that makes sure the struct does not have a variant with fields and give an error message based on the rustc one.

We have also updated the message given in the case where one field was present to list the missing fields and match more closely the new message.

gcc/rust/ChangeLog:

* typecheck/rust-hir-type-check-expr.cc (TypeCheckExpr::visit) Add additional check
* typecheck/rust-hir-type-check-struct-field.h: A helper method to make error added
* typecheck/rust-hir-type-check-expr-struct.cc (TypeCheckStructExpr::resolve) Update message

gcc/testsuite/ChangeLog:

* rust/compile/missing_constructor_fields.rs: Added case with no initializers

Signed-off-by: Robert Goss [email protected]

[P-E-P]

LGTM. Great work!

This looks like your first PR. Could you please add a message stating that you've read and acknowledge the developer certificate of origins?

[robertgoss]

Hi I have read and acknowledge the developer certificate of origins - do i need to modify the commit to add a signed off tag at the end?

[P-E-P]

do i need to modify the commit to add a signed off tag at the end?

Mmh actually yes please! But what bothers me is that the CI should have caught this 🤔

[robertgoss]

do i need to modify the commit to add a signed off tag at the end?

Mmh actually yes please! But what bothers me is that the CI should have caught this 🤔

Should be correct now

[CohenArthur]

thanks for working on this :) I checked it out locally and realized that this only errors out for one of the missing fields. If we do have the first field, like in the following example:

struct Foo {
    x: i32,
    y: i32,
    z: i32,
}

fn main() {
    let x = Foo { x: 15 };
}

then we don't hit your error, but the one in rust-hir-type-check-struct.cc at line 153. I think it might be worth modifying this error instead so that it reports which fields are missing?

[CohenArthur]

Suggested change

	"missing field %s in initializer of %<%s%>",
	"missing field %s in initializer of %qs",

this is a shorter version of the same thing

[robertgoss]

then we don't hit your error, but the one in rust-hir-type-check-struct.cc at line 153. I think it might be worth modifying this error instead so that it reports which fields are missing?

Yes as this was my first PR I was a bit nervy about expanding my change too much happy to do the same change to those ones as well

[philberty]

I like this change but i would prefer if you didn't use std::stringstream, it would be ideal if you gather like std::vector then you check if that is non empty as the error condition.

Minor nit if no one else cares then i would go for it but it tends to be more useful to keep track of the error nodes instead of building the error message in one go as we might do something else with that information down the line.

[robertgoss]

I like this change but i would prefer if you didn't use std::stringstream, it would be ideal if you gather like std::vector then you check if that is non empty as the error condition.

Sure that sounds sensible and I can make the std::vector of the names. The manual processing to format them into a string I didn't like but wasn't aware of an available method to do that.

Minor nit if no one else cares then i would go for it but it tends to be more useful to keep track of the error nodes instead of building the error message in one go as we might do something else with that information down the line.

I can have a look at this. I was basing if off some of the other error handling I had seen in the same area.

[CohenArthur]

LGTM :)