Skip to content
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

Lifetime cleanups #130294

Merged
merged 10 commits into from
Sep 14, 2024
Merged

Lifetime cleanups #130294

merged 10 commits into from
Sep 14, 2024

Conversation

nnethercote
Copy link
Contributor

The last commit is very opinionated, let's see how we go.

r? @oli-obk

This does change the external interface, but not in a way that will
cause any breakage because external users don't mention the lifetimes.
Giving them more typical names.
- Replace non-standard names like 's, 'p, 'rg, 'ck, 'parent, 'this, and
  'me with vanilla 'a. These are cases where the original name isn't
  really any more informative than 'a.
- Replace names like 'cx, 'mir, and 'body with vanilla 'a when the lifetime
  applies to multiple fields and so the original lifetime name isn't
  really accurate.
- Put 'tcx last in lifetime lists, and 'a before 'b.
@rustbot
Copy link
Collaborator

rustbot commented Sep 13, 2024

Could not assign reviewer from: oli-obk.
User(s) oli-obk are either the PR author, already assigned, or on vacation, and there are no other candidates.
Use r? to specify someone else to assign.

@rustbot
Copy link
Collaborator

rustbot commented Sep 13, 2024

r? @lcnr

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 13, 2024
@rustbot
Copy link
Collaborator

rustbot commented Sep 13, 2024

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

struct CheckLoopVisitor<'a, 'tcx> {
sess: &'a Session,
struct CheckLoopVisitor<'tcx> {
sess: &'tcx Session,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can get the session from the tcx, the field can be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a commit doing that.

@oli-obk oli-obk mentioned this pull request Sep 13, 2024
12 tasks
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me after oli's review.

I feel like this change is right at the edge of whether it's worth it given the potential merge conflicts, reviewer time and what not. So while I think it's an improvement and we should merge this PR, I would not like to review many more PRs like this.

@nnethercote
Copy link
Contributor Author

more PRs like this

The good news is that this PR does most of the these sorts of possible changes :)

@lcnr
Copy link
Contributor

lcnr commented Sep 14, 2024

@bors r+ rollup=iffy

@bors
Copy link
Contributor

bors commented Sep 14, 2024

📌 Commit 359b658 has been approved by lcnr

It is now in the queue for this repository.

@bors 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 Sep 14, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 14, 2024
Rollup of 6 pull requests

Successful merges:

 - rust-lang#130017 (coverage: Extract `executor::block_on` from several async coverage tests)
 - rust-lang#130268 (simd_shuffle: require index argument to be a vector)
 - rust-lang#130290 (Stabilize entry_insert)
 - rust-lang#130294 (Lifetime cleanups)
 - rust-lang#130343 (docs: Enable required feature for 'closure_returning_async_block' lint)
 - rust-lang#130349 (Fix `Parser::break_up_float`'s right span)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 03e8b6b into rust-lang:master Sep 14, 2024
6 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Sep 14, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 14, 2024
Rollup merge of rust-lang#130294 - nnethercote:more-lifetimes, r=lcnr

Lifetime cleanups

The last commit is very opinionated, let's see how we go.

r? `@oli-obk`
@nnethercote nnethercote deleted the more-lifetimes branch September 15, 2024 22:42
@nnethercote
Copy link
Contributor Author

FWIW, how I got to this PR:

  • I noticed some convoluted memory management in rustc_mir_transform. I cleaned this up in Simplify DestProp memory management #129720. One part of this change reduced the number of lifetimes for FilterInformation from four to two.
  • This got me looking for other unnecessary lifetimes. In Dataflow/borrowck lifetime cleanups #130022 I removed unnecessary lifetimes from multiple widely-used structs in dataflow/borrowck code.
  • I also noticed that rustc_resolver was using 'a for all references into the ResolverArenas type. This is a situation that is perfect for a more descriptive name, so in Introduce 'ra lifetime name. #130208 I introduced the 'ra lifetime name.
  • This PR is the tail end of this work, where I found a few more unnecessary lifetimes and fixed up some lifetime ordering and naming inconsistencies. I didn't address every single possible inconsistency, just the more obvious ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants