-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use PlaceRef more consistently instead of loosely coupled local+projection #80624
Conversation
r? @oli-obk (rust-highfive has picked a reviewer for you, use r? to override) |
r=me with the FIXMEs removed |
@bors r=oli-obk |
📌 Commit afa7408 has been approved by |
⌛ Testing commit afa7408 with merge 77fa8e0314833fe7a8be22d6c94a3a1a5eab7ff3... |
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
Not sure what this is, hopefully spurious? @bors retry |
⌛ Testing commit afa7408 with merge bf497e5b25b3cd9dc3eaa7796c08f4d2f55d5599... |
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
Same failure again...?!? No idea what to make of this. @bors retry |
☀️ Test successful - checks-actions |
❤️ I had this in my list of pending things, thanks @RalfJung for fixing it. |
@spastorino this is not done yet, it's just a start: #80647. |
Instead of working directly with the
projections
array, useiter_projections
andlast_projection
. This avoids having to construct newPlaceRef
from the pieces everywhere.I only did this for a few files, to see how people think about this. If y'all are happy with this, I'll open an E-mentor issue to complete this. I grepped for
Place::ty_from
to find the places that need adjusting -- this could miss some, but I am not sure what else to grep for.