cranelift / PR #1324 Fix clippy warnings in EncodingBits · git-cranelift

Stream: git-cranelift

Topic: cranelift / PR #1324 Fix clippy warnings in EncodingBits

GitHub (Jan 08 2020 at 18:13):

abrown opened PR #1324 from fix-warnings to master:

GitHub (Jan 08 2020 at 18:13):

abrown requested sstangl for a review on PR #1324.

GitHub (Jan 08 2020 at 18:14):

abrown edited PR #1324 from fix-warnings to master:

Prior to this I would see the following on cargo build:

$ cargo build
warning: identical conversion
  --> cranelift-codegen/shared/src/isa/x86/encoding_bits.rs:63:26
   |
63 |         debug_assert_eq!(u8::from(self.rrr()), 0);
   |                          ^^^^^^^^^^^^^^^^^^^^ help: consider removing `u8::from()`: `self.rrr()`
   |
   = note: `#[warn(clippy::identity_conversion)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_conversion

warning: using `clone` on a `Copy` type
  --> cranelift-codegen/shared/src/isa/x86/encoding_bits.rs:64:23
   |
64 |         let mut enc = self.clone();
   |                       ^^^^^^^^^^^^ help: try removing the `clone` call: `self`
   |
   = note: `#[warn(clippy::clone_on_copy)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy

warning: using `clone` on a `Copy` type
  --> cranelift-codegen/shared/src/isa/x86/encoding_bits.rs:73:23
   |
73 |         let mut enc = self.clone();
   |                       ^^^^^^^^^^^^ help: try removing the `clone` call: `self`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy

    Finished dev [unoptimized + debuginfo] target(s) in 0.04s

GitHub (Jan 08 2020 at 18:44):

bjorn3 submitted PR Review.

GitHub (Jan 08 2020 at 18:44):

bjorn3 submitted PR Review.

GitHub (Jan 08 2020 at 18:44):

bjorn3 created PR Review Comment:

You can use mut self here.

GitHub (Jan 09 2020 at 19:22):

abrown updated PR #1324 from fix-warnings to master:

Prior to this I would see the following on cargo build:

$ cargo build
warning: identical conversion
  --> cranelift-codegen/shared/src/isa/x86/encoding_bits.rs:63:26
   |
63 |         debug_assert_eq!(u8::from(self.rrr()), 0);
   |                          ^^^^^^^^^^^^^^^^^^^^ help: consider removing `u8::from()`: `self.rrr()`
   |
   = note: `#[warn(clippy::identity_conversion)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_conversion

warning: using `clone` on a `Copy` type
  --> cranelift-codegen/shared/src/isa/x86/encoding_bits.rs:64:23
   |
64 |         let mut enc = self.clone();
   |                       ^^^^^^^^^^^^ help: try removing the `clone` call: `self`
   |
   = note: `#[warn(clippy::clone_on_copy)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy

warning: using `clone` on a `Copy` type
  --> cranelift-codegen/shared/src/isa/x86/encoding_bits.rs:73:23
   |
73 |         let mut enc = self.clone();
   |                       ^^^^^^^^^^^^ help: try removing the `clone` call: `self`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy

    Finished dev [unoptimized + debuginfo] target(s) in 0.04s

GitHub (Jan 09 2020 at 19:22):

abrown submitted PR Review.

GitHub (Jan 09 2020 at 19:22):

abrown created PR Review Comment:

Good call :+1:

GitHub (Jan 09 2020 at 19:46):

abrown updated PR #1324 from fix-warnings to master:

Prior to this I would see the following on cargo build:

$ cargo build
warning: identical conversion
  --> cranelift-codegen/shared/src/isa/x86/encoding_bits.rs:63:26
   |
63 |         debug_assert_eq!(u8::from(self.rrr()), 0);
   |                          ^^^^^^^^^^^^^^^^^^^^ help: consider removing `u8::from()`: `self.rrr()`
   |
   = note: `#[warn(clippy::identity_conversion)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_conversion

warning: using `clone` on a `Copy` type
  --> cranelift-codegen/shared/src/isa/x86/encoding_bits.rs:64:23
   |
64 |         let mut enc = self.clone();
   |                       ^^^^^^^^^^^^ help: try removing the `clone` call: `self`
   |
   = note: `#[warn(clippy::clone_on_copy)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy

warning: using `clone` on a `Copy` type
  --> cranelift-codegen/shared/src/isa/x86/encoding_bits.rs:73:23
   |
73 |         let mut enc = self.clone();
   |                       ^^^^^^^^^^^^ help: try removing the `clone` call: `self`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy

    Finished dev [unoptimized + debuginfo] target(s) in 0.04s

GitHub (Jan 10 2020 at 10:10):

bnjbvr submitted PR Review.

GitHub (Jan 10 2020 at 10:10):

bnjbvr submitted PR Review.

GitHub (Jan 10 2020 at 10:10):

bnjbvr created PR Review Comment:

Out of curiosity, why was it split into two lines? It seems dominates would be dead in a non-debug build.

GitHub (Jan 10 2020 at 16:09):

abrown created PR Review Comment:

clippy was complaining about using something mutable inside the debug_assert! (IIRC it was func.layout); moving the call out of the macro fixed the warning but honestly I didn't give it too much thought. Is there a better way?

GitHub (Jan 10 2020 at 16:09):

abrown submitted PR Review.

GitHub (Jan 10 2020 at 16:20):

abrown updated PR #1324 from fix-warnings to master:

Prior to this I would see the following on cargo build:

$ cargo build
warning: identical conversion
  --> cranelift-codegen/shared/src/isa/x86/encoding_bits.rs:63:26
   |
63 |         debug_assert_eq!(u8::from(self.rrr()), 0);
   |                          ^^^^^^^^^^^^^^^^^^^^ help: consider removing `u8::from()`: `self.rrr()`
   |
   = note: `#[warn(clippy::identity_conversion)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_conversion

warning: using `clone` on a `Copy` type
  --> cranelift-codegen/shared/src/isa/x86/encoding_bits.rs:64:23
   |
64 |         let mut enc = self.clone();
   |                       ^^^^^^^^^^^^ help: try removing the `clone` call: `self`
   |
   = note: `#[warn(clippy::clone_on_copy)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy

warning: using `clone` on a `Copy` type
  --> cranelift-codegen/shared/src/isa/x86/encoding_bits.rs:73:23
   |
73 |         let mut enc = self.clone();
   |                       ^^^^^^^^^^^^ help: try removing the `clone` call: `self`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy

    Finished dev [unoptimized + debuginfo] target(s) in 0.04s

GitHub (Jan 10 2020 at 16:38):

abrown merged PR #1324.

GitHub (Jan 11 2020 at 08:53):

philipc submitted PR Review.

GitHub (Jan 11 2020 at 08:53):

philipc created PR Review Comment:

Seems like a false positive to me, see this reduced case, and it's probably better to ignore clippy here if it's wrong.

Last updated: Dec 23 2024 at 13:07 UTC