| Message ID | 20241012075312.16342-6-witcher@wiredspace.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/7] drm/panic: avoid reimplementing Iterator::find | expand |
On Sat, Oct 12, 2024 at 9:53 AM Thomas Böhler <witcher@wiredspace.de> wrote: > > Clippy complains about a non-minimal boolean expression with > `nonminimal_bool`: > > error: this boolean expression can be simplified > --> drivers/gpu/drm/drm_panic_qr.rs:722:9 > | > 722 | (x < 8 && y < 8) || (x < 8 && y >= end) || (x >= end && y < 8) > | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > | > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool > = note: `-D clippy::nonminimal-bool` implied by `-D warnings` > = help: to override `-D warnings` add `#[allow(clippy::nonminimal_bool)]` > help: try > | > 722 | !(x >= 8 || y >= 8 && y < end) || (x >= end && y < 8) > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 722 | (y >= end || y < 8) && x < 8 || (x >= end && y < 8) > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > While this can be useful in a lot of cases, it isn't here because the > line expresses clearly what the intention is. Simplifying the expression > means losing clarity, so opt-out of this lint for the offending line. > > Reported-by: Miguel Ojeda <ojeda@kernel.org> > Closes: https://github.com/Rust-for-Linux/linux/issues/1123 > Signed-off-by: Thomas Böhler <witcher@wiredspace.de> > --- > drivers/gpu/drm/drm_panic_qr.rs | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_panic_qr.rs b/drivers/gpu/drm/drm_panic_qr.rs > index 58c46f366f76..226107c02679 100644 > --- a/drivers/gpu/drm/drm_panic_qr.rs > +++ b/drivers/gpu/drm/drm_panic_qr.rs > @@ -719,7 +719,8 @@ fn draw_finders(&mut self) { > > fn is_finder(&self, x: u8, y: u8) -> bool { > let end = self.width - 8; > - (x < 8 && y < 8) || (x < 8 && y >= end) || (x >= end && y < 8) > + #[allow(clippy::nonminimal_bool)] > + return (x < 8 && y < 8) || (x < 8 && y >= end) || (x >= end && y < 8); Surely introducing a return statement causes another clippy error? You can do this: #[allow(clippy::nonminimal_bool)] { (x < 8 && y < 8) || (x < 8 && y >= end) || (x >= end && y < 8) } or just put the allow on the function. Alice
On Sat, Oct 12, 2024 at 9:53 AM Thomas Böhler <witcher@wiredspace.de> wrote: > > Clippy complains about a non-minimal boolean expression with > `nonminimal_bool`: > > error: this boolean expression can be simplified > --> drivers/gpu/drm/drm_panic_qr.rs:722:9 > | > 722 | (x < 8 && y < 8) || (x < 8 && y >= end) || (x >= end && y < 8) > | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > | > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool > = note: `-D clippy::nonminimal-bool` implied by `-D warnings` > = help: to override `-D warnings` add `#[allow(clippy::nonminimal_bool)]` > help: try > | > 722 | !(x >= 8 || y >= 8 && y < end) || (x >= end && y < 8) > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 722 | (y >= end || y < 8) && x < 8 || (x >= end && y < 8) > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > While this can be useful in a lot of cases, it isn't here because the > line expresses clearly what the intention is. Simplifying the expression > means losing clarity, so opt-out of this lint for the offending line. > > Reported-by: Miguel Ojeda <ojeda@kernel.org> > Closes: https://github.com/Rust-for-Linux/linux/issues/1123 > Signed-off-by: Thomas Böhler <witcher@wiredspace.de> > --- > drivers/gpu/drm/drm_panic_qr.rs | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_panic_qr.rs b/drivers/gpu/drm/drm_panic_qr.rs > index 58c46f366f76..226107c02679 100644 > --- a/drivers/gpu/drm/drm_panic_qr.rs > +++ b/drivers/gpu/drm/drm_panic_qr.rs > @@ -719,7 +719,8 @@ fn draw_finders(&mut self) { > > fn is_finder(&self, x: u8, y: u8) -> bool { > let end = self.width - 8; > - (x < 8 && y < 8) || (x < 8 && y >= end) || (x >= end && y < 8) > + #[allow(clippy::nonminimal_bool)] > + return (x < 8 && y < 8) || (x < 8 && y >= end) || (x >= end && y < 8); This should be #[expect(...)] instead of #[allow(...)]. Alice
On 12/10/2024 09:52, Thomas Böhler wrote: > Clippy complains about a non-minimal boolean expression with > `nonminimal_bool`: > > error: this boolean expression can be simplified > --> drivers/gpu/drm/drm_panic_qr.rs:722:9 > | > 722 | (x < 8 && y < 8) || (x < 8 && y >= end) || (x >= end && y < 8) > | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > | > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool > = note: `-D clippy::nonminimal-bool` implied by `-D warnings` > = help: to override `-D warnings` add `#[allow(clippy::nonminimal_bool)]` > help: try > | > 722 | !(x >= 8 || y >= 8 && y < end) || (x >= end && y < 8) > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 722 | (y >= end || y < 8) && x < 8 || (x >= end && y < 8) > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > While this can be useful in a lot of cases, it isn't here because the > line expresses clearly what the intention is. Simplifying the expression > means losing clarity, so opt-out of this lint for the offending line. Thanks, I also prefer to keep the non-minimal boolean. With the suggestions from Alice Ryhl to not introduce a return, and use expect: Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com> > > Reported-by: Miguel Ojeda <ojeda@kernel.org> > Closes: https://github.com/Rust-for-Linux/linux/issues/1123 > Signed-off-by: Thomas Böhler <witcher@wiredspace.de> > --- > drivers/gpu/drm/drm_panic_qr.rs | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_panic_qr.rs b/drivers/gpu/drm/drm_panic_qr.rs > index 58c46f366f76..226107c02679 100644 > --- a/drivers/gpu/drm/drm_panic_qr.rs > +++ b/drivers/gpu/drm/drm_panic_qr.rs > @@ -719,7 +719,8 @@ fn draw_finders(&mut self) { > > fn is_finder(&self, x: u8, y: u8) -> bool { > let end = self.width - 8; > - (x < 8 && y < 8) || (x < 8 && y >= end) || (x >= end && y < 8) > + #[allow(clippy::nonminimal_bool)] > + return (x < 8 && y < 8) || (x < 8 && y >= end) || (x >= end && y < 8); > } > > // Alignment pattern: 5x5 squares in a grid.
On Mon, Oct 14, 2024 at 10:54 AM Jocelyn Falempe <jfalempe@redhat.com> wrote: > > With the suggestions from Alice Ryhl to not introduce a return, and use > expect: +1 to both. `expect` (here and the other ones I suggested) require `rust-next`, so if this goes through DRM, then we can clean this up later. Otherwise, if you prefer `rust-next`, we can change them to `expect` already. Thanks! Cheers, Miguel
On 14/10/2024 18:59, Miguel Ojeda wrote: > On Mon, Oct 14, 2024 at 10:54 AM Jocelyn Falempe <jfalempe@redhat.com> wrote: >> >> With the suggestions from Alice Ryhl to not introduce a return, and use >> expect: > > +1 to both. > > `expect` (here and the other ones I suggested) require `rust-next`, so > if this goes through DRM, then we can clean this up later. Otherwise, > if you prefer `rust-next`, we can change them to `expect` already. I don't plan to touch drm_panic_qr.rs, so I think it's better if this series goes through rust-next, to avoid an extra cleanup step later.
diff --git a/drivers/gpu/drm/drm_panic_qr.rs b/drivers/gpu/drm/drm_panic_qr.rs index 58c46f366f76..226107c02679 100644 --- a/drivers/gpu/drm/drm_panic_qr.rs +++ b/drivers/gpu/drm/drm_panic_qr.rs @@ -719,7 +719,8 @@ fn draw_finders(&mut self) { fn is_finder(&self, x: u8, y: u8) -> bool { let end = self.width - 8; - (x < 8 && y < 8) || (x < 8 && y >= end) || (x >= end && y < 8) + #[allow(clippy::nonminimal_bool)] + return (x < 8 && y < 8) || (x < 8 && y >= end) || (x >= end && y < 8); } // Alignment pattern: 5x5 squares in a grid.
Clippy complains about a non-minimal boolean expression with `nonminimal_bool`: error: this boolean expression can be simplified --> drivers/gpu/drm/drm_panic_qr.rs:722:9 | 722 | (x < 8 && y < 8) || (x < 8 && y >= end) || (x >= end && y < 8) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool = note: `-D clippy::nonminimal-bool` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::nonminimal_bool)]` help: try | 722 | !(x >= 8 || y >= 8 && y < end) || (x >= end && y < 8) | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 722 | (y >= end || y < 8) && x < 8 || (x >= end && y < 8) | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ While this can be useful in a lot of cases, it isn't here because the line expresses clearly what the intention is. Simplifying the expression means losing clarity, so opt-out of this lint for the offending line. Reported-by: Miguel Ojeda <ojeda@kernel.org> Closes: https://github.com/Rust-for-Linux/linux/issues/1123 Signed-off-by: Thomas Böhler <witcher@wiredspace.de> --- drivers/gpu/drm/drm_panic_qr.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)