-
Notifications
You must be signed in to change notification settings - Fork 13k
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
A couple simple borrowck cleanups #135250
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
"Elements" are `RegionElement`s. The dense location mapping was removed from the element containers a while ago but didn't rename its use-sites. Most of the old naming only used the mapping, and are better named `location_map`.
rustbot
added
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
Jan 8, 2025
This comment was marked as resolved.
This comment was marked as resolved.
Its original naming hides the fact that it's related to datalog polonius, and bound to be deleted in the near future. It also conflicts with the expected name for the actual NLL location map, and prefixing it with its use will make the differentiation possible.
This is another strangely named struct (and associated fields) that is hard to see was related to datalog polonius.
@bors r+ |
bors
added
S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
Jan 8, 2025
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 8, 2025
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#134228 (Exhaustively handle expressions in patterns) - rust-lang#135194 (triagebot: mark tidy changes with a more specific `A-tidy` label) - rust-lang#135222 (Ensure that we don't try to access fields on a non-struct pattern type) - rust-lang#135250 (A couple simple borrowck cleanups) - rust-lang#135252 (Fix release notes link) - rust-lang#135253 (Revert rust-lang#131365) Failed merges: - rust-lang#135195 (Make `lit_to_mir_constant` and `lit_to_const` infallible) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 8, 2025
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#134228 (Exhaustively handle expressions in patterns) - rust-lang#135194 (triagebot: mark tidy changes with a more specific `A-tidy` label) - rust-lang#135222 (Ensure that we don't try to access fields on a non-struct pattern type) - rust-lang#135250 (A couple simple borrowck cleanups) - rust-lang#135252 (Fix release notes link) - rust-lang#135253 (Revert rust-lang#131365) Failed merges: - rust-lang#135195 (Make `lit_to_mir_constant` and `lit_to_const` infallible) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 9, 2025
Rollup merge of rust-lang#135250 - lqd:simple-cleanups, r=matthewjasper A couple simple borrowck cleanups This PR has a couple simple renamings: - it's been a long time since the mapping from `Location`s to `PointIndex`es was extracted from `RegionElements` into the `DenseLocationMap`, but only the types were renamed at the time. borrowck still refers to this map as `elements`. That's confusing, especially since sometimes we also use the mapping via `LivenessValues`, and makes more sense as `location_map` instead. - to clarify `LocationTable` is not as general as it sounds, and is only for datalog polonius. In this branch I didn't rename the handful of `location_table` fields and params to `polonius_table`, but can do that to differentiate it even more from `location_map`. I did try it locally and it looks worthwhile, so if you'd prefer I can also push it here. (Or we could even switch these datalog types and fields to even more explicit names) - to clarify the incomprehensible `AllFacts`, it is renamed to `PoloniusFacts`. These can be referred to as `facts` within the legacy polonius module, but as `polonius_facts` outside of it to make it clear that they're not about NLLs (nor are they about in-tree polonius but that'll be magically fixed when they're removed in the future) r? `@matthewjasper`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR has a couple simple renamings:
Location
s toPointIndex
es was extracted fromRegionElements
into theDenseLocationMap
, but only the types were renamed at the time. borrowck still refers to this map aselements
. That's confusing, especially since sometimes we also use the mapping viaLivenessValues
, and makes more sense aslocation_map
instead.LocationTable
is not as general as it sounds, and is only for datalog polonius. In this branch I didn't rename the handful oflocation_table
fields and params topolonius_table
, but can do that to differentiate it even more fromlocation_map
. I did try it locally and it looks worthwhile, so if you'd prefer I can also push it here. (Or we could even switch these datalog types and fields to even more explicit names)AllFacts
, it is renamed toPoloniusFacts
. These can be referred to asfacts
within the legacy polonius module, but aspolonius_facts
outside of it to make it clear that they're not about NLLs (nor are they about in-tree polonius but that'll be magically fixed when they're removed in the future)r? @matthewjasper