[Place 2.0] Convert Place's projection to a boxed slice #63420
Conversation
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| } else { | ||
| &arg_place.projection[..arg_place.projection.len() - 1] | ||
| let base_proj = match arg_place.projection { | ||
| box [ref base_proj @ .., ProjectionElem::Field(..)] | |
There was a problem hiding this comment.
move the ref base_proj @ into the patterns above
|
one more nit |
spastorino
force-pushed
the
place2_5
branch
from
8e8e96c
to
0a140c8
Compare
spastorino
force-pushed
the
place2_5
branch
from
0a140c8
to
28db2c9
Compare
|
@bors r+ |
|
📌 Commit 28db2c9 has been approved by |
bors
added
S-waiting-on-bors
[Place 2.0] Convert Place's projection to a boxed slice This is still work in progress, it's not compiling right now I need to review a bit more to see what's going on but wanted to open the PR to start discussing it. r? @oli-obk
|
☀️ Test successful - checks-azure |
|
📣 Toolstate changed by #63420! Tested on commit a6946a8. 💔 clippy-driver on windows: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk @phansch @flip1995 @yaahc, @rust-lang/infra). |
Tested on commit rust-lang/rust@a6946a8. Direct link to PR: <rust-lang/rust#63420> 💔 clippy-driver on windows: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk @phansch @flip1995 @yaahc, @rust-lang/infra). 💔 clippy-driver on linux: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk @phansch @flip1995 @yaahc, @rust-lang/infra). 💔 miri on windows: test-fail → build-fail (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra). 💔 miri on linux: test-fail → build-fail (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra).
| self.eval_static_to_mplace(place_static)?.into() | ||
| } | ||
| }; | ||
| let mut op = match &place.base { |
There was a problem hiding this comment.
When Place 2.0 is done, will we be able to get rid of the mutability here again?
I think there was no mutability when this started, and it would be nice to be able to get back to clean code with immutable variables.
Or maybe this is just a matter of rewriting the final for elem in place.projection.iter() into a fold?
There was a problem hiding this comment.
It's a matter of writing it using fold, gonna send a PR about it as soon as I can resync and compile my rustc copy.
There was a problem hiding this comment.
Hmmm in order to avoid the mut there I'd need to desugar the ? operator, because it won't work inside fold. Do you think it worth? any better idea?.
There was a problem hiding this comment.
Thanks for taking a look!
Does try_fold help?
There was a problem hiding this comment.
Yes, that's what I needed #64473
Fix rustc breaking change: convert to Place's new boxed slice projection Addressed breaking changes from rust-lang PR rust-lang/rust#63420 I'm not entirely sure the semantics are preserved as I don't have much knowledge about MIR yet. So this code was largely reverse-engineered from the PR above. I wouldn't be surprised if I did something wrong :). I followed the instructions to pull latest rustc from master and verified the build break I was seeing in my PR for cast-lossless in Travis CI. With these changes, it compiles again and all tests pass. Fixes rust-lang/rust#64440 changelog: none
This is still work in progress, it's not compiling right now I need to review a bit more to see what's going on but wanted to open the PR to start discussing it.
r? @oli-obk