Reject extra fields in MGCA struct const arguments#157886
Conversation
|
HIR ty lowering was modified cc @fmease |
rustbot
added
S-waiting-on-review
|
r? @davidtwco rustbot has assigned @davidtwco. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| for init in inits { | ||
| if !variant_def.fields.iter().any(|field_def| field_def.name == init.field.name) { | ||
| let err = tcx.dcx().struct_span_err( | ||
| init.field.span, | ||
| format!("struct expression has no field named `{}`", init.field), | ||
| ); | ||
| return ty::Const::new_error(tcx, err.emit()); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Currently, lower_const_arg_struct handled missing and duplicate initializers, but it did not check for extra fields. This checkd for fields that do not exist on the target variant.
| if !variant_def.fields.iter().any(|field_def| field_def.name == init.field.name) { | ||
| let err = tcx.dcx().struct_span_err( | ||
| init.field.span, | ||
| format!("struct expression has no field named `{}`", init.field), |
There was a problem hiding this comment.
This message seems a little off... the expression does have the field; it's the struct definition that does not. We should match the wording for the normal (non-MGCA) message: "struct S has no field named x".
There was a problem hiding this comment.
hmm.. Now I think of it, should have not written expressions. I will update it, Thanks.
| let variant_idx = adt_def.variant_index_with_id(variant_did).as_u32(); | ||
|
|
||
| for init in inits { | ||
| if !variant_def.fields.iter().any(|field_def| field_def.name == init.field.name) { |
There was a problem hiding this comment.
Seems not ideal for performance to have an O(n^2) loop here. Probably fine in most cases but would scale badly for really large structs that sometimes appear in generated code. Can you check what the equivalent code for non-MGCA struct exprs does? Not critical to match it here but would be nice (though for all I know, it's also O(n^2)...).
There was a problem hiding this comment.
I will take a look. We also have a similar O(n^2) loop below this for handling missing and duplicate initializers, so I have made a note of it. Thanks for the suggestion. I willl also look into non-MGCA struct expressions handling.
There was a problem hiding this comment.
It's true, so don't worry too much about it. It'd probably be good for us to fix both occurrences in the future, but you don't have to now.
5 participants
closes: #154538
r? @BoxyUwU