Float comparison lint doesn't seem to trigger on float array comparison

[Lokathor]

#[test]
fn m128_max() {
  let a: m128 = cast([5.0_f32, 6.0, 7.0, 8.5]);
  let b: m128 = cast([-8.0_f32, 4.0, 12.0, 0.5]);
  let out: [f32; 4] = cast(a.max(b));
  assert_eq!(out, [5.0_f32, 6.0, 12.0, 8.5]);
  assert_eq!(out[0], [5.0_f32, 6.0, 12.0, 8.5][0]);
}

the second assert triggers clippy, the first doesn't.

[briankabiro]

Hi, I would like to take a look at this.

[flip1995]

Great! If you have any questions, don't hesitate to ask.

[briankabiro]

Cool, I am going to go through it and make sure to reach out. :)

[briankabiro]

Hi, @flip1995. I took the time to go through the codebase and think that the code change should be somewhere in the misc.rs

I am not sure why it wouldn't trigger when comparing array values. Could you offer some pointers?

[flip1995]

You're on the right track. In the line you linked is a check for the left and right side of the Eq/Ne Binop if both sides are floats:

rust-clippy/clippy_lints/src/misc.rs

Lines 521 to 523 in e7d153a

	fn is_float(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
	matches!(walk_ptrs_ty(cx.tables.expr_ty(expr)).sty, ty::Float(_))
	}

This function takes the expression, "derefs" it, and checks if the type is float. (i.e is_float(&&float) = is_float(float) = true, but is_float([float; N]) = false). So you have to modify this function, so that it not only checks the expressions type to be ty::Floats, but also for ty::Array(ty::Float, _)s.

Hope this helps :)

[briankabiro]

Yes, this definitely helps. I think I can work with this. Thanks!

[briankabiro]

Hey @flip1995. I've been caught up with work and this slipped under my radar; apologies for not giving an update sooner. I'd be happy to let someone else finish this if there's someone that you might recommend.

If not, I'd still be interested in completing the feature. 😃

[flip1995]

No problem, you can get back to it, once you have more time 👍

[briankabiro]

Thanks for understanding :)

[marcin-serwin]

Hi, I'd like to take up this issue if possible.

[flip1995]

Yeah sure, you can take a look at #4343 for some inspiration.

[briankabiro]

Hey @Toxyxer. Go right ahead. I'm interested in seeing the solution that you come up with!

[marcin-serwin]

Hey @flip1995, I've opened a pull request with my proposed changes. Some comments:

I think this example should trigger the lint because the arrays are declared with let, so we don't know whether they are all zeros at compile-time (unless there is another reason why we know this). Similarly code

    let x = 0.0;
    let y = 0.0;
    x == y;

triggers the lint. I've changed the code so that when we change the code in the example to declare arrays with consts it will pass.

I've noticed that the assert_eq macro with floats triggers the lint regardless if they are equal to zeros as evidenced by:

error: strict comparison of `f32` or `f64`
  --> $DIR/float_cmp.rs:47:5
   |
LL |     assert_eq!(0.0, 0.0);
   |     ^^^^^^^^^^^^^^^^^^^^^
   |

so I didn't use it in tests. It's a separate issue so I didn't do anything about it. It's already reported here: #3804.

I'm new to rust so there is probably room for improvement in the submitted code. Thanks in advance for any suggestions on how to make it better.