Skip to content

Conversation

nnethercote
Copy link
Contributor

There are several places in rustc_middle that check for an empty lifetime name. These checks appear to be totally unnecessary, because empty lifetime names aren't produced here. (Empty lifetime names are possible in hir::Lifetime. Perhaps there was some confusion between it and the rustc_middle types?)

This commit removes the kw::Empty checks.

r? @lcnr

@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 Mar 25, 2025
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.

That does stem from a confusion. I don't know why hir lifetimes can be empty and ty lifetimes cannot be 😅

I geuss r=me, but is there are way we can assert that there are no lifetimes in the Ty layer? 🤔 maybe in Lifetime::new_[early|late]_param

@nnethercote
Copy link
Contributor Author

nnethercote commented Mar 25, 2025

I am in the process of disallowing empty lifetime names in HIR (in favour of kw::UnderscoreLifetimes). It's a bit more complicated than this PR 😄

Asserting non-emptiness is a slippery slope. There are quite a lot of places where you could insert assertions. In general I am removing kw::Empty occurrences which makes assertions less important -- the fewer kw::Empty occurrences there are in the code, the less you need to protect against empty symbols showing up unexpectedly...

There are several places in `rustc_middle` that check for an empty
lifetime name. These checks appear to be totally unnecessary, because
empty lifetime names aren't produced here. (Empty lifetime names *are*
possible in `hir::Lifetime`. Perhaps there was some confusion between
it and the `rustc_middle` types?)

This commit removes the `kw::Empty` checks.
@nnethercote
Copy link
Contributor Author

#138965 is the PR removing kw::Empty from HIR lifetimes.

Much like the ones in the previous commit.
@nnethercote nnethercote force-pushed the less-kw-Empty-rustc_middle branch from 01ec436 to 5a6ed74 Compare March 27, 2025 08:13
@lcnr
Copy link
Contributor

lcnr commented Mar 27, 2025

thank you for cleaning this up :3

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Mar 27, 2025

📌 Commit 5a6ed74 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 Mar 27, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 27, 2025
Rollup of 7 pull requests

Successful merges:

 - rust-lang#138844 (expand: Leave traces when expanding `cfg` attributes)
 - rust-lang#138926 (Remove `kw::Empty` uses from `rustc_middle`.)
 - rust-lang#138989 (Clean up a few things in rustc_hir_analysis::check::region)
 - rust-lang#138999 (Report compiletest pass mode if forced)
 - rust-lang#139014 (Improve suggest construct with literal syntax instead of calling)
 - rust-lang#139015 (Remove unneeded LLVM CI test assertions)
 - rust-lang#139016 (Add job duration changes to post-merge analysis report)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 10debec into rust-lang:master Mar 27, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 27, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 27, 2025
Rollup merge of rust-lang#138926 - nnethercote:less-kw-Empty-rustc_middle, r=lcnr

Remove `kw::Empty` uses from `rustc_middle`.

There are several places in `rustc_middle` that check for an empty lifetime name. These checks appear to be totally unnecessary, because empty lifetime names aren't produced here. (Empty lifetime names *are* possible in `hir::Lifetime`. Perhaps there was some confusion between it and the `rustc_middle` types?)

This commit removes the `kw::Empty` checks.

r? `@lcnr`
@nnethercote nnethercote deleted the less-kw-Empty-rustc_middle branch March 27, 2025 23:10
@nnethercote
Copy link
Contributor Author

BTW, this helped with #137978.

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.

4 participants