Skip to content

build: migrate the codebase to the 2024 Language Edition #4191

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

Merged
merged 6 commits into from
Feb 23, 2025

Conversation

rami3l
Copy link
Member

@rami3l rami3l commented Feb 22, 2025

Continuation of #4190.

This is a semi-automatic migration due to the sheer amount of false positives in the rust-2024-compatibility lint group and the if_let_rescope lint in particular. Some steps have more details in the corresponding commit messages.

You can get a clearer version of the PR diff via semanticdiff.

Concerns

@rami3l rami3l force-pushed the build/edition-2024 branch 5 times, most recently from 6799481 to 665f283 Compare February 22, 2025 07:07
@rami3l rami3l requested review from djc and ChrisDenton February 22, 2025 07:17
@rami3l rami3l marked this pull request as ready for review February 22, 2025 07:19
@rami3l rami3l force-pushed the build/edition-2024 branch from 665f283 to 396623f Compare February 22, 2025 07:56
@djc
Copy link
Contributor

djc commented Feb 22, 2025

Nice work!

Following
<https://users.rust-lang.org/t/rust-2024-compatibility-lint-and-the-if-let-temporary-scope/125969/5>,
I have manually audited all `if_let_rescope` cases and found out that
certain existing cases can be rewritten to make it clearer semantically.
So these migrations have been made in advance so that the only cases of
`if-let` remaining are false positives of this lint.

The crux of determining false positives here is that, according to
<rust-lang/rust#133167>, `if let Some()`
and other similar cases where the `else` drop is not significant are
not affected by the Edition change.
@rami3l rami3l force-pushed the build/edition-2024 branch from 396623f to ff252c5 Compare February 22, 2025 14:57
@rami3l rami3l force-pushed the build/edition-2024 branch from ff252c5 to 85b90f6 Compare February 22, 2025 15:01
@@ -129,7 +129,7 @@ impl NotificationLevel {
/// A [`tracing::Subscriber`] [`Layer`][`tracing_subscriber::Layer`] that corresponds to Rustup's
/// optional `opentelemetry` (a.k.a. `otel`) feature.
#[cfg(feature = "otel")]
fn telemetry<S>(process: &Process) -> impl Layer<S>
fn telemetry<S>(process: &Process) -> impl Layer<S> + use<S>
Copy link
Member Author

@rami3l rami3l Feb 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@djc So basically impl Layer<S> + use<S> means "this impl Layer<S> has nothing to do with 'process" (this function is generic over both 'process and S, so we use<S> to exclude 'process), and the same thing goes for other fixes in this commit.

@rami3l
Copy link
Member Author

rami3l commented Feb 23, 2025

I just did one more check and have again found no missing significant drops. I think it should be safe to merge this one now.

@rami3l rami3l added this pull request to the merge queue Feb 23, 2025
Merged via the queue into rust-lang:master with commit 83d0901 Feb 23, 2025
29 checks passed
@rami3l rami3l deleted the build/edition-2024 branch February 23, 2025 01:42
@rami3l rami3l added this to the 1.28.0 milestone Mar 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants