Skip to content

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