Use PlaceRef projection abstractions more consistently in rustc_mir #80865
Conversation
rust-highfive
added
the
S-waiting-on-review
oliviacrain
changed the title
camelid
added
A-mir
| @@ -478,7 +478,7 @@ impl<'tcx> Validator<'_, 'tcx> { | |||
| // https://github.com/rust-lang/rust/pull/74945#discussion_r463063247 | |||
| // There may be opportunity for generalization, but this needs to be | |||
| // accounted for. | |||
| if proj_base.is_empty() | |||
| if place_base.projection.is_empty() | |||
There was a problem hiding this comment.
I think the is_empty check should be moved up near self.temps[place_base.local] -- that code only makes sense if there are no projections.
|
Thanks a lot for this PR, @oliviacrain. :) I just left two minor comments; everything else looks perfect! |
|
Thanks for the comments, @RalfJung! I've addressed comments, squashed fixups, and rebased against master. |
|
Thanks for the quick update! However, for the future, it'd be better if you could refrain from force-pushing to a branch until review is done. With a force-push, now I have to re-read the entire PR again, I have no way of checking just the new diff. If there are conflicts, a rebase (+ force-push) is needed, but squashing should never be done until review is complete. If you hadn't rebased I could have used GH's force-push-diff link, but after squash + rebase, all hope of an incremental review is lost.^^ |
|
Ah, so sorry about that. My brain didn't switch from work PR mode... will avoid that in the future. |
|
No worries, different policies in different projects can be confusing. ;) |
| @@ -1007,27 +1007,26 @@ fn place_as_reborrow( | |||
| body: &Body<'tcx>, | |||
| place: Place<'tcx>, | |||
| ) -> Option<&'a [PlaceElem<'tcx>]> { | |||
There was a problem hiding this comment.
This looks to me like the return type should be changed to Option<PlaceRef>... but that's a larger change, no need to do it in this PR.
|
This looks great, thanks for working on this. :) |
|
📌 Commit 65b5e43 has been approved by |
bors
added
S-waiting-on-bors
Use PlaceRef projection abstractions more consistently in rustc_mir PlaceRef contains abstractions for dealing with the `projections` array. This PR uses these abstractions more consistently within the `rustc_mir` crate. See associated issue: rust-lang#80647. r? `@RalfJung`
|
☀️ Test successful - checks-actions |
PlaceRef contains abstractions for dealing with the
projectionsarray. This PR uses these abstractions more consistently within therustc_mircrate.See associated issue: #80647.
r? @RalfJung