fix typo in numeric_arithmetic logic

[klensy]

Looks like typo for me.

Looking at blame b76b0ae#diff-ac15e787d7d276b24f251f4f5bdedf1e6ac81aa1e2ea0db27219e9a7fa8b0b30L66 the same typo was before.

Is logic here actually correct? Tests passed locally, but i expected some changes.

changelog: none

[rustbot]

r? @y21

rustbot has assigned @y21.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[samueltardieu]

You can probably build a test case which would fail before and succeed after your patch. For example, what if you define your own type, which implements multiplication by a floating point type? Since only the right hand site seems to be checked for floating points in binary operators, it might not trigger before your PR. It would be a great addition to make sure this bug does not reappear.

Also, this would warrant a changelog entry since it appears that you are fixing a real bug.

[klensy]

Float ops allowed only if both ops is float, otherwise there will be compile error, so this check almost redundant, i think?

[samueltardieu]

Float ops allowed only if both ops is float, otherwise there will be compile error, so this check almost redundant, i think?

No, you can have different types for left side, right side and result. This would be valid:

struct S;

impl std::ops::Mul<f32> for S {
    type Output = f32;

    fn mul(self, rhs: f32) -> Self::Output {
        todo!()
    }
}

fn test() -> f32 {
    S * 3.0
}

[samueltardieu]

And by the way @klensy you can check the result of the lintcheck differences in the CI, run on some popular crates. For your latest push so far, it is here (look at "Diff results").

You'll see that your PR causes some new triggers, and that they involve a non-floating point as a left operand, as in my example.

[samueltardieu]

And apart from missing tests, it looks good to me!

[klensy]

Before it checked (erroneously) only right part of binop, now checks if both part is floats. Shouldn't it actually check if any part of binop is float instead?

[klensy]

Detectable by #13828

[samueltardieu]

Before it checked (erroneously) only right part of binop, now checks if both part is floats. Shouldn't it actually check if any part of binop is float instead?

Probably.

[y21]

Looks like this regressed all the way back in rust-lang/rust@e97602e , where before it was checking for the same thing so this looks like the intended behavior to me. Thank you!

[klensy]

Wait,

Shouldn't it actually check if any part of binop is float instead?

I'll add follow up then.