cargo
cargo copied to clipboard
[DO NOT MERGE] Force edition 2024 lint into action on crater
This is to initiate a discussion on enabling crater to apply some lints as a trial while checking crates and make sure that lints marked as machine applicable are not rendering code invalid after the lint application.
Help is needed for this PR. rust-lang/rust#107251 is introducing a new machine applicable lint. The intention of this PR is to force application of if-let-rescope lint during the cargo fix --edition ... invocation. With this commit, it seems that the in the migration pass the lint is still suppressed but the second cargo check did activate the said lint. Here is the log.
Copying to /tmp/fixit
Running `cargo fix --edition`
Migrating Cargo.toml from 2021 edition to 2024
Dirty testfixer v0.1.0 (/tmp/fixit): the target configuration changed
Checking testfixer v0.1.0 (/tmp/fixit)
Running `/mnt/dev/rust/build/x86_64-unknown-linux-gnu/stage1-tools-bin/cargo /home/xxx/.rustup/toolchains/devrust-stage1/bin/rustc --crate-name testfixer --edition=2021 src/main.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --diagnostic-width=420 --emit=dep-info,metadata -C embed-bitcode=no -C debuginfo=2 --test --check-cfg 'cfg(docsrs)' --check-cfg 'cfg(feature, values())' -C metadata=225fa3b10800db73 -C extra-filename=-225fa3b10800db73 --out-dir /home/xxx/.cargo-target/debug/deps -C incremental=/home/xxx/.cargo-target/debug/incremental -L dependency=/home/xxx/.cargo-target/debug/deps`
Running `/mnt/dev/rust/build/x86_64-unknown-linux-gnu/stage1-tools-bin/cargo /home/xxx/.rustup/toolchains/devrust-stage1/bin/rustc --crate-name testfixer --edition=2021 src/main.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --diagnostic-width=420 --crate-type bin --emit=dep-info,metadata -C embed-bitcode=no -C debuginfo=2 --check-cfg 'cfg(docsrs)' --check-cfg 'cfg(feature, values())' -C metadata=72e3d0a692bea088 -C extra-filename=-72e3d0a692bea088 --out-dir /home/xxx/.cargo-target/debug/deps -C incremental=/home/xxx/.cargo-target/debug/incremental -L dependency=/home/xxx/.cargo-target/debug/deps`
Migrating src/main.rs from 2021 edition to 2024
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.06s
Running `cargo check` to verify 2024
Dirty testfixer v0.1.0 (/tmp/fixit): the target configuration changed
Checking testfixer v0.1.0 (/tmp/fixit)
Running `/home/xxx/.rustup/toolchains/devrust-stage1/bin/rustc --crate-name testfixer --edition=2024 -Z unstable-options src/main.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --diagnostic-width=420 --crate-type bin --emit=dep-info,metadata -C embed-bitcode=no -C debuginfo=2 --check-cfg 'cfg(docsrs)' --check-cfg 'cfg(feature, values())' -C metadata=72e3d0a692bea088 -C extra-filename=-72e3d0a692bea088 --out-dir /home/xxx/.cargo-target/debug/deps -C incremental=/home/xxx/.cargo-target/debug/incremental -L dependency=/home/xxx/.cargo-target/debug/deps`
warning: `if let` assigns a shorter lifetime since Edition 2024
--> src/main.rs:17:8
|
17 | if let Some(_value) = droppy().get() {
| ^^^^^^^^^^^^^^^^^^^--------^^^^^^
| |
| this value has a significant drop implementation which may observe a major change in drop order and requires your discretion
|
= warning: this changes meaning in Rust 2024
= note: for more information, see issue #124085 <https://github.com/rust-lang/rust/issues/124085>
help: the value is now dropped here in Edition 2024
--> src/main.rs:18:5
|
18 | } else {
| ^
= note: `#[warn(if_let_rescope)]` on by default
help: rewrite this `if let` into a `match` with a single arm to preserve the drop order up to Edition 2021
|
17 ~ match droppy().get() { Some(_value) => {
18 ~ } _ => {
19 ~ }}
|
warning: `testfixer` (bin "testfixer") generated 1 warning (run `cargo fix --bin "testfixer"` to apply 1 suggestion)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04
So it is really close to working, but preferably the lint should be applied in the Migrating phase already so that the check shows up successful.
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) some time within the next two weeks.
Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:
@rustbot author: the review is finished, PR author should check the comments and take action accordingly@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue
Thanks for the pull request. Just note that in Cargo a feature discussion usually happens in an issue. We leave pull requests for implementation specific discussions. See https://doc.crates.io/contrib/process/working-on-cargo.html#before-hacking-on-cargo
Just note that in Cargo a feature discussion usually happens in an issue. We leave pull requests for implementation specific discussions.
As a clarification, he's not necessarily asking for a Cargo feature here. He's asking for help on making this branch, which would never be merged, do what he needs it to do for an edition-related crater run for rust-lang/rust#107251.
Hacking up Cargo on a branch in this way is how these edition-related crater runs have been done in the past. See, e.g., rust-lang/rust#125384.
I asked him to file this as a draft "do not merge" PR so there would be a place to hang that discussion of how to implement this properly.
This branch would be for the crater run in:
- https://github.com/rust-lang/rust/pull/129466
Thanks for the clarification! Yeah I realised that immediately after posting that comment 😬
The discussion is now happening here https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Simulate.20Edition.202024.20lints.20in.20edition.20migration
:umbrella: The latest upstream changes (presumably 4c39aaff66862cc0da52fe529aa1990bb8bb9a22) made this pull request unmergeable. Please resolve the merge conflicts.
I'm going to close as I do not think this is needed anymore, thanks!