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

Use (😏) precise capture instead of Captures trait hack #135416

Closed
wants to merge 1 commit into from

Conversation

yotamofek
Copy link
Contributor

No description provided.

@rustbot
Copy link
Collaborator

rustbot commented Jan 12, 2025

r? @GuillaumeGomez

rustbot has assigned @GuillaumeGomez.
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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 12, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 12, 2025

Some changes occurred in coverage instrumentation.

cc @Zalathar

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

Some changes occurred in exhaustiveness checking

cc @Nadrieril

@yotamofek yotamofek force-pushed the use-precise-capture branch from 63d096d to 2b64ab5 Compare January 12, 2025 20:53
@Urgau
Copy link
Member

Urgau commented Jan 12, 2025

r? compiler

@rustbot rustbot assigned BoxyUwU and unassigned GuillaumeGomez Jan 12, 2025
@GuillaumeGomez
Copy link
Member

At least looks good to me. :)

@Zalathar
Copy link
Contributor

Many of the functions that currently use Captures would not need any use<> markers at all under the 2024 edition capture rules.

For that reason, even though we have use<> today, I think it’s better to delay some or all of this cleanup until after upgrading to Rust 2024. Until then, the hack serves as a de-facto marker of functions that need to be reexamined after the edition bump.

Whereas if we do this now, we either lose that easily-greppable information from the source, or we’re just replacing one perfectly-functional hack with a bunch of noisy TODOs to revisit later.

@lqd
Copy link
Member

lqd commented Jan 12, 2025

For the compiler side, I think the plan was to move to edition 2024 with its different capture rules instead of what this PR proposes.

@yotamofek
Copy link
Contributor Author

Many of the functions that currently use Captures would not need any use<> markers at all under the 2024 edition capture rules.

For that reason, even though we have use<> today, I think it’s better to delay some or all of this cleanup until after upgrading to Rust 2024. Until then, the hack serves as a de-facto marker of functions that need to be reexamined after the edition bump.

Whereas if we do this now, we either lose that easily-greppable information from the source, or we’re just replacing one perfectly-functional hack with a bunch of noisy TODOs to revisit later.

Waiting for the 2024 edition sounds like a better plan then, thank you for taking the time to explain. So should I close this?

BTW, are there any plans for a lint that'll point out removable use<>s in Rust 2024?

@compiler-errors
Copy link
Member

compiler-errors commented Jan 13, 2025

Yea, I'd prefer if we close this.

I've got a PR (#129636) open to upgrade the compiler to edition 2024, but I'm waiting for the edition to be stable so that we can upgrade all of the crates that currently rust-analyzer shares with rustc, rather than having to hold them back separately.

I've implemented a lint for redundant captures in the IMPL_TRAIT_REDUNDANT_CAPTURES lint, which will fire in edition 2024. I've noticed that I don't think I ever got signoff on making that lint warn, so I'll probably make sure T-lang is okay with that, lool.

@yotamofek yotamofek deleted the use-precise-capture branch January 13, 2025 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants