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

Add lint explicit_auto_deref take 2 #8355

Merged
merged 14 commits into from
Jun 28, 2022
Merged

Conversation

Jarcho
Copy link
Contributor

@Jarcho Jarcho commented Jan 27, 2022

fixes: #234
fixes: #8367
fixes: #8380

Still things to do:

  • This currently only lints &*<expr> when it doesn't trigger needless_borrow.
  • This requires a borrow after a deref to trigger. So *<expr> changing &&T to &T won't be caught.
  • The deref and deref_mut trait methods aren't linted.
  • Neither field accesses, nor method receivers are linted.
  • This probably shouldn't lint reborrowing.
  • Full slicing to deref should probably be handled here as well. e.g. &vec[..] when just &vec would do

changelog: new lint explicit_auto_deref

@rust-highfive
Copy link

r? @flip1995

(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 Jan 27, 2022
@bors
Copy link
Contributor

bors commented Jan 27, 2022

☔ The latest upstream changes (presumably #8359) made this pull request unmergeable. Please resolve the merge conflicts.

@Jarcho Jarcho force-pushed the explicit_auto_deref_2 branch 14 times, most recently from ba3112f to a31eaad Compare January 28, 2022 23:22
@bors
Copy link
Contributor

bors commented Jan 29, 2022

☔ The latest upstream changes (presumably #8289) made this pull request unmergeable. Please resolve the merge conflicts.

Herschel added a commit to Herschel/ruffle that referenced this pull request Jan 29, 2022
Currently it is not directly possible to configure lints for the
entire workspace via TOML, which forced us to repeat `#![allow]`
blocks in each crate.

embark pointed out this workaround to configure lints at the
workspace level via RUSTFLAGS:

EmbarkStudios/rust-ecosystem#22 (comment)

Remove the common `#![allow]` blocks and switch to this method for
global lint config.

Temporarily allow `needless_borrow` lint, buggy pending this fix:
rust-lang/rust-clippy#8355
Herschel added a commit to ruffle-rs/ruffle that referenced this pull request Jan 29, 2022
Currently it is not directly possible to configure lints for the
entire workspace via TOML, which forced us to repeat `#![allow]`
blocks in each crate.

embark pointed out this workaround to configure lints at the
workspace level via RUSTFLAGS:

EmbarkStudios/rust-ecosystem#22 (comment)

Remove the common `#![allow]` blocks and switch to this method for
global lint config.

Temporarily allow `needless_borrow` lint, buggy pending this fix:
rust-lang/rust-clippy#8355
@Jarcho Jarcho force-pushed the explicit_auto_deref_2 branch 3 times, most recently from 3d37661 to 5a6a6d6 Compare January 30, 2022 19:14
@djc
Copy link
Contributor

djc commented Feb 7, 2022

Does this also fix this case?

pub fn put_u16(v: u16, out: &mut [u8]) {
    let out: &mut [u8; 2] = (&mut out[..2]).try_into().unwrap();
    *out = u16::to_be_bytes(v);
}
Warning:   --> rustls/src/msgs/codec.rs:91:29
   |
91 |     let out: &mut [u8; 2] = (&mut out[..2]).try_into().unwrap();
   |                             ^^^^^^^^^^^^^^^ help: change this to: `out[..2]`
   |
   = note: `#[warn(clippy::needless_borrow)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow

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.

LGTM. r=me after rebase

@bors
Copy link
Contributor

bors commented Jun 16, 2022

☔ The latest upstream changes (presumably #9007) made this pull request unmergeable. Please resolve the merge conflicts.

@Jarcho Jarcho force-pushed the explicit_auto_deref_2 branch from 1d935dd to dd3d747 Compare June 25, 2022 11:47
@bors
Copy link
Contributor

bors commented Jun 28, 2022

☔ The latest upstream changes (presumably #8639) made this pull request unmergeable. Please resolve the merge conflicts.

@flip1995
Copy link
Member

Oh no, another instance, where GitHub didn't show me a notification for a force push. @Jarcho please approve this PR yourself with bors r=flip1995 after you rebased (as long as there aren't any surprising changes).

@Jarcho Jarcho force-pushed the explicit_auto_deref_2 branch from dd3d747 to 68596aa Compare June 28, 2022 16:49
@Jarcho Jarcho force-pushed the explicit_auto_deref_2 branch from 68596aa to 5e2a2d3 Compare June 28, 2022 17:02
@Jarcho
Copy link
Contributor Author

Jarcho commented Jun 28, 2022

@bors r=flip1995

@bors
Copy link
Contributor

bors commented Jun 28, 2022

📌 Commit 5e2a2d3 has been approved by flip1995

@bors
Copy link
Contributor

bors commented Jun 28, 2022

⌛ Testing commit 5e2a2d3 with merge a4130e1...

@bors
Copy link
Contributor

bors commented Jun 28, 2022

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

@bors bors merged commit a4130e1 into rust-lang:master Jun 28, 2022
relrelb added a commit to relrelb/ruffle that referenced this pull request Jul 8, 2022
Though rust-lang/rust-clippy#8355 has been
merged, it seems to still report false-positives on nightly channel.

For now just fix the instances reported by stable clippy, and keep
`needless_borrow` allowed.
relrelb added a commit to ruffle-rs/ruffle that referenced this pull request Jul 8, 2022
Though rust-lang/rust-clippy#8355 has been
merged, it seems to still report false-positives on nightly channel.

For now just fix the instances reported by stable clippy, and keep
`needless_borrow` allowed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
5 participants