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

op_ref false positive(?) when comparing Box<dyn Tr> where dyn Tr: PartialEq #12113

Open
kawadakk opened this issue Jan 9, 2024 · 0 comments
Open
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@kawadakk
Copy link

kawadakk commented Jan 9, 2024

Summary

op_ref triggers when equating values of type {Box,Rc,Arc}<dyn Tr> (where dyn Tr: PartialEq) with extra &s (as in &lhs == &rhs). The extra &s are necessary to avoid a borrowck error, though this is a bug on the rustc side (rust-lang/rust#31740).

Lint Name

op_ref

Reproducer

I tried this code (Playground, minimized from an in-the-wild usage of arrow::array::ArrayRef):

pub trait Tr {}

impl PartialEq for dyn Tr {
    fn eq(&self, _: &Self) -> bool { todo!() }
}

pub fn foo(x: Box<dyn Tr>, y: Box<dyn Tr>) -> Box<dyn Tr> {
    assert!(&x == &y);
    y
}

I saw this happen:

warning: needlessly taken reference of both operands
 --> src/lib.rs:8:13
  |
8 |     assert!(&x == &y);
  |             ^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#op_ref
  = note: `#[warn(clippy::op_ref)]` on by default
help: use the values directly
  |
8 |     assert!(x == y);
  |             ~    ~

I expected to see this happen:
No warnings, because removing & as per suggestion would cause a compile error:

error[E0382]: use of moved value: `y`
 --> src/lib.rs:9:5
  |
7 | pub fn foo(x: Box<dyn Tr>, y: Box<dyn Tr>) -> Box<dyn Tr> {
  |                            - move occurs because `y` has type `std::boxed::Box<dyn Tr>`, which does not implement the `Copy` trait
8 |     assert!(x == y);
  |                  - value moved here
9 |     y
  |     ^ value used here after move

For more information about this error, try `rustc --explain E0382`.

Version

rustc 1.77.0-nightly (ca663b06c 2024-01-08)
binary: rustc
commit-hash: ca663b06c5492ac2dde5e53cd11579fa8e4d68bd
commit-date: 2024-01-08
host: x86_64-unknown-linux-gnu
release: 1.77.0-nightly
LLVM version: 17.0.6

Additional Labels

@rustbot label +I-suggestion-causes-error

@kawadakk kawadakk added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Jan 9, 2024
@rustbot rustbot added the I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied label Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
Development

No branches or pull requests

2 participants