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

Introduce 'ra lifetime name. #130208

Merged
merged 1 commit into from
Sep 12, 2024
Merged

Introduce 'ra lifetime name. #130208

merged 1 commit into from
Sep 12, 2024

Conversation

nnethercote
Copy link
Contributor

rustc_resolve allocates many things in ResolverArenas. The lifetime used for references into the arena is mostly 'a, and sometimes 'b.

This commit changes it to 'rslv, which is much more descriptive. The commit also changes the order of lifetimes on a couple of structs so that 'rslv is second last, before 'tcx, and does other minor renamings such as 'r to 'a.

r? @petrochenkov
cc @oli-obk

@rustbot 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 Sep 10, 2024
@Noratrieb
Copy link
Member

how about 'res, an already common abbreviation for resolution/resolving?

@nnethercote
Copy link
Contributor Author

I deliberately avoided res because it's also used a lot for "result". But I could live with it, it's certainly better than 'a'.

@petrochenkov
Copy link
Contributor

petrochenkov commented Sep 11, 2024

Technically it's not a lifetime of resolver (i.e. you can't do &'rslv Resolver), it's a lifetime of the arenas and it's larger.
On the other hand 'tcx is also technically not a lifetime of TyCtxt.

I don't like long lifetime names though, as long as 'tcx is probably the limit, the resolver also acts sort of like tcx (i.e. a global context) for a large part of compilation.
Maybe compress it to 3 letters - 'res, 'rcx, 'rsl, 'rsv, 'ra (resolver arenas), ...?

@petrochenkov
Copy link
Contributor

'r is currently used as an actual Resolver lifetime, why renaming it?

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 11, 2024
@nnethercote
Copy link
Contributor Author

Maybe compress it to 3 letters - 'res, 'rcx, 'rsl, 'rsv, 'ra (resolver arenas), ...?

I like 'ra, I will switch to that.

'r is currently used as an actual Resolver lifetime, why renaming it?

This is in reference to changes like this one:

-        struct FindReferenceVisitor<'r, 'a, 'tcx> {
-            r: &'r Resolver<'a, 'tcx>,
+        struct FindReferenceVisitor<'a, 'rslv, 'tcx> {
+            r: &'a Resolver<'rslv, 'tcx>,
             impl_self: Option<Res>,
             lifetime: Set1<LifetimeRes>, 
         }

There are a handful of structs where I changed &'r Resolver<...> to &'a Resolver<...>. I did this because I don't think these references are truly specific to the resolver. If another field containing a reference was added to this struct, it's quite likely that it would have the same lifetime. This issue came up in #130022 where several structs had multiple reference fields with the same lifetime, and @oli-obk suggested switching from 'mir (which was appropriate only for the &mir::Body field) to 'a to indicate that the lifetime is more just "a lifetime" rather than a very specific lifetime. E.g. we ended up with this:

pub struct EverInitializedPlaces<'a, 'tcx> {
    body: &'a mir::Body<'tcx>,
    move_data: &'a MoveData<'tcx>,
}

instead of using 'mir. (Lifetime naming is difficult!)

`rustc_resolve` allocates many things in `ResolverArenas`. The lifetime
used for references into the arena is mostly `'a`, and sometimes `'b`.

This commit changes it to `'ra`, which is much more descriptive. The
commit also changes the order of lifetimes on a couple of structs so
that '`ra` is second last, before `'tcx`, and does other minor
renamings such as `'r` to `'a`.
@nnethercote nnethercote changed the title Introduce 'rslv lifetime name. Introduce 'ra lifetime name. Sep 11, 2024
@nnethercote
Copy link
Contributor Author

I have updated 'rslv to 'ra throughout.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Sep 12, 2024

📌 Commit d4fc76c has been approved by petrochenkov

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 12, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 12, 2024
…chenkov

Introduce `'ra` lifetime name.

`rustc_resolve` allocates many things in `ResolverArenas`. The lifetime used for references into the arena is mostly `'a`, and sometimes `'b`.

This commit changes it to `'rslv`, which is much more descriptive. The commit also changes the order of lifetimes on a couple of structs so that '`rslv` is second last, before `'tcx`, and does other minor renamings such as `'r` to `'a`.

r? `@petrochenkov`
cc `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 12, 2024
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#130101 (some const cleanup: remove unnecessary attributes, add const-hack indications)
 - rust-lang#130208 (Introduce `'ra` lifetime name.)
 - rust-lang#130263 (coverage: Simplify creation of sum counters)
 - rust-lang#130273 (more eagerly discard constraints on overflow)
 - rust-lang#130276 (Add test for nalgebra hang in coherence)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b0ff0b7 into rust-lang:master Sep 12, 2024
6 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Sep 12, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 12, 2024
Rollup merge of rust-lang#130208 - nnethercote:rslv-lifetime, r=petrochenkov

Introduce `'ra` lifetime name.

`rustc_resolve` allocates many things in `ResolverArenas`. The lifetime used for references into the arena is mostly `'a`, and sometimes `'b`.

This commit changes it to `'rslv`, which is much more descriptive. The commit also changes the order of lifetimes on a couple of structs so that '`rslv` is second last, before `'tcx`, and does other minor renamings such as `'r` to `'a`.

r? ``@petrochenkov``
cc ``@oli-obk``
@nnethercote nnethercote deleted the rslv-lifetime branch September 12, 2024 23:31
@nnethercote nnethercote mentioned this pull request Sep 15, 2024
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants