Skip to content

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

Development

Successfully merging this pull request may close these issues.

needless_borrow false positive, with broken suggestion needless_borrow suggestion doesn't compile lint unnecessary derefs

5 participants