Remove kw::Empty uses from rustc_middle. by nnethercote · Pull Request #138926 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation6 Commits2 Checks6 Files changed

Conversation

This file contains hidden or 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

[ Show hidden characters]({{ revealButtonHref }})

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 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

Contributor

lcnr left a comment • Loading

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

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.

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

Much like the ones in the previous commit.

thank you for cleaning this up :3

@bors r+ rollup

📌 Commit 5a6ed74 has been approved by lcnr

It is now in the queue for this repository.

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

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

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.