Lower struct patterns and struct expressions with unnamed fields#121553
Lower struct patterns and struct expressions with unnamed fields#121553frank-king wants to merge 1 commit intorust-lang:masterfrom
Conversation
rustbot
added
S-waiting-on-review
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
@rustbot label F-unnamed_fields |
|
cc #121757, which I believe isn't currently fixed by these changes |
davidtwco
left a comment
davidtwco
left a comment
There was a problem hiding this comment.
Some first pass review comments
| &'tcx ty::FieldDef, | ||
| // The inner-most field type | ||
| Ty<'tcx>, | ||
| )> { |
| match mentioned { | ||
| Some((idx, _need_recheck @ true)) => { | ||
| // Check the only mentioned field of the union again, | ||
| // really collecting the unamed fields. |
There was a problem hiding this comment.
| // really collecting the unamed fields. | |
| // really collecting the unnamed fields. |
| LL | let U { a, b } = u; | ||
| | ^^^^^^^^^^ | ||
| | | ||
| note: there are multiple fields from this union |
There was a problem hiding this comment.
| note: there are multiple fields from this union | |
| note: multiple fields from this union are matched but the union can only be one at a time |
Something a bit more verbose like this might be better.
| @@ -50,6 +50,11 @@ pub struct TypeckResults<'tcx> { | |||
| /// `_(2).field`. | |||
| nested_fields: ItemLocalMap<Vec<(Ty<'tcx>, FieldIdx)>>, | |||
| return self.collect_unmentioned( | ||
| variant.fields.iter().skip(idx.index()).take(usize::MAX), | ||
| collect_unmentioned, | ||
| ) || has_mentioned; |
There was a problem hiding this comment.
I think the implementation of this check is a bit more complex than it needs to be I'm not sure why you need to go into collect_unmentioned here rather than just add to unmentioned_fields from this loop.
There was a problem hiding this comment.
Or rather, if there are unmentioned fields in one call to this function, then we could just queue up a UnionPatUnmentioned like you do for the other errors.
| // In case of the multiple fields from the union are used, | ||
| // we do not collect those fields to report unmentioned errors. | ||
| self.check_struct_variant(nested_adt_def.non_enum_variant(), false) | ||
| .then_some((idx, /* need_recheck */ true)) |
There was a problem hiding this comment.
Similarly, I'm not sure that it's that big of a deal if we report an error for unmentioned fields from two unnamed struct types of an unnamed union (instead of just one of them). It may not be worth the additional complexity this adds.
|
☔ The latest upstream changes (presumably #122041) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@frank-king any updates on this? thanks |
|
I'm sorry that I haven't got some time to work on this. I'm going to close it. |
6 participants
This implements #49804.
Goals:
Non-Goals (will be in the next PRs)
r? @davidtwco