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

bump clippy crates to edition 2021 #7664

Merged
merged 2 commits into from
Sep 27, 2021
Merged

Conversation

matthiaskrgr
Copy link
Member

Also helps with dogfooding edition 2021 a bit. :)
Tests passed locally.


changelog: bump edition from 2018 to 2021

@rust-highfive
Copy link

r? @xFrednet

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 10, 2021
Copy link
Member

@xFrednet xFrednet left a comment

Choose a reason for hiding this comment

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

This looks good to me. However, I would like to also get some feedback from @flip1995 before merging this 🙃.

r? @flip1995

@xFrednet
Copy link
Member

r? @flip1995

@rust-highfive rust-highfive assigned flip1995 and unassigned xFrednet Sep 11, 2021
@flip1995
Copy link
Member

flip1995 commented Sep 13, 2021

Is there already a rust_2021_idioms lint group we can enable? This diff looks suspiciously small.

If not, r=me.

@matthiaskrgr
Copy link
Member Author

There is rust_2021_compatibility I turned that on now and fix the only warning 🙃

warning: changes to closure capture in Rust 2021 will affect drop order
   --> clippy_utils/src/lib.rs:995:26
    |
995 |     v.allow_closure.then(|| v.captures)
    |                          ^^^----------
    |                             |
    |                             in Rust 2018, this closure captures all of `v`, but in Rust 2021, it will only capture `v.captures`
996 | }
    | - in Rust 2018, `v` is dropped here, but in Rust 2021, only `v.captures` will be dropped here as part of the closure
    |
note: the lint level is defined here
   --> clippy_utils/src/lib.rs:16:9
    |
16  | #![warn(rust_2021_compatibility)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^
    = note: `#[warn(rust_2021_incompatible_closure_captures)]` implied by `#[warn(rust_2021_compatibility)]`
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/disjoint-capture-in-closures.html>
help: add a dummy let to cause `v` to be fully captured
    |
995 |     v.allow_closure.then(|| { let _ = &v; v.captures })
    |                             +++++++++++++            +

Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

Nice, let's also keep the rust_2021_compatibility 👍

Comment on lines 995 to 998
v.allow_closure.then(|| {
let _ = &v;
v.captures
})
Copy link
Member

Choose a reason for hiding this comment

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

We don't really care about fully capturing v, do we?

clippy_utils/src/lib.rs Outdated Show resolved Hide resolved
@camsteffen
Copy link
Contributor

camsteffen commented Sep 13, 2021

I think rust_2021_compatibility is only meant to be enabled before transitioning, not after?

@flip1995
Copy link
Member

flip1995 commented Sep 14, 2021

From the rustc book:

Lints used to transition code from the 2018 edition to 2021

A rust-2021-idioms lint group does not exist yet. So I guess we don't want to enable the compatibility group. But good to know, that only one of our closures "fails" this.

@matthiaskrgr
Copy link
Member Author

removed the rust_2021_compatibility warnings again.

@flip1995
Copy link
Member

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Sep 27, 2021

📌 Commit d888b4b has been approved by flip1995

@bors
Copy link
Contributor

bors commented Sep 27, 2021

⌛ Testing commit d888b4b with merge 0c8799d...

@bors
Copy link
Contributor

bors commented Sep 27, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 0c8799d to master...

@bors bors merged commit 0c8799d into rust-lang:master Sep 27, 2021
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants