Skip to content

Conversation

Zalathar
Copy link
Contributor

This re-lookup of &hir::Pat by its ID appears to be an artifact of earlier complexity that has since been removed from the compiler.

Merely deleting the let/match results in borrow errors, but sprinkling 'tcx in the signature allows it to work again, so I suspect that this code's current function is simply to compensate for overly loose lifetimes in the signature. Perhaps it made more sense at a time when HIR lifetimes were not tied to 'tcx.

I spotted this while working on some more experimental changes, which is why I've extracted it into its own PR.

This re-lookup of `&hir::Pat` by its ID appears to be an artifact of earlier
complexity that has since been removed from the compiler.
@rustbot
Copy link
Collaborator

rustbot commented Aug 17, 2024

r? @Nadrieril

rustbot has assigned @Nadrieril.
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 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 Aug 17, 2024
@Zalathar
Copy link
Contributor Author

For reference, this code was originally introduced by 50076b0 (#44275).

@compiler-errors
Copy link
Member

Yeah, I agree that it's correct to use 'tcx in signatures for HIR data almost always.

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 17, 2024

📌 Commit 194ade1 has been approved by compiler-errors

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 Aug 17, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 17, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#128989 (Emit an error for invalid use of the linkage attribute)
 - rust-lang#129167 (mir/pretty: use `Option` instead of `Either<Once, Empty>`)
 - rust-lang#129168 (Return correct HirId when finding body owner in diagnostics)
 - rust-lang#129191 (rustdoc-json: Clean up serialization and printing.)
 - rust-lang#129192 (Remove useless attributes in merged doctest generated code)
 - rust-lang#129196 (Remove a useless ref/id/ref round-trip from `pattern_from_hir`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4e6147d into rust-lang:master Aug 17, 2024
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 17, 2024
Rollup merge of rust-lang#129196 - Zalathar:ref-id-ref, r=compiler-errors

Remove a useless ref/id/ref round-trip from `pattern_from_hir`

This re-lookup of `&hir::Pat` by its ID appears to be an artifact of earlier complexity that has since been removed from the compiler.

Merely deleting the let/match results in borrow errors, but sprinkling `'tcx` in the signature allows it to work again, so I suspect that this code's current function is simply to compensate for overly loose lifetimes in the signature. Perhaps it made more sense at a time when HIR lifetimes were not tied to `'tcx`.

I spotted this while working on some more experimental changes, which is why I've extracted it into its own PR.
@rustbot rustbot added this to the 1.82.0 milestone Aug 17, 2024
@Zalathar Zalathar deleted the ref-id-ref branch August 17, 2024 22:05
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