Unexpected compiler error E0308 on arithmetic with borrowed values

[vdmit]

I've faced with compiler error on arithmetic operations with borrowed values. After some simplification of my code, it was reduced to following program:

fn main() {
    let a: i32 = 0;
    let b: i32 = 0;
    let aa = &a;
    // works
    if b - aa < 0 { }
    // unexpected e0308 error    
    if b < aa {  }
}

Here we have two if statements. First compiles OK, and second fails with compiler error E0308: It complains on "incompatible" types i32 and &i32. However, expression in first if also contains variables of different types, and does not irritate Rust compiler.
I suppose it's a bug, because this behaviour is counter-intuitive.

Note: I've tested it on stable 1.2.0 and nightly versions (via http://play.rust-lang.org) - results are the same.

[petrochenkov]

It's a standard library quirk - operator minus (std::ops::Sub) is implemented for all combinations of values and references: i32 + i32, i32 + &i32, &i32 + i32, &i32 + &i32 (say thanks to @sellibitze #21227). At the same time operator less (std::cmp::PartialOrd) doesn't have such extra implementations (i32 < &i32 and &i32 < i32) but they can probably be added for consistency.

[petrochenkov]

I've tried to add the needed impls of PartialEq and PartialOrd but type inference around operators ==/< is bad enough for it to be a breaking change (the branch: https://github.com/petrochenkov/rust/tree/cmpref).
IIUC, it will make sense to try this again after ungating default type inference fallback and removing special treatment of SIMD from comparison operators.

[steveklabnik]

/cc @rust-lang/libs , should these extra impls be added? /cc @rust-lang/lang , is this a breaking change that's worth making or not?

[sellibitze]

The motivation to provide i32: Sub<&i32> (and similar) is because the arithmetic operators Add, Sub, Mul, etc take their arguments by value and thus consume them in generic contexts (if you parameterize the arithmetic type) unless you require the type to be Copy as well. If you don't add the Copy bound but you want to reuse the arguments you would have to use Clone. But that would make the code less readable and may incur unnecessary costs for user-defined arithmetic types like extended precision floating point numbers, for example, which are presumably not Copy and more expensive to clone. So, for the sake of uniformity, arithmetic type providers should add non-consuming operator implementations so that you can rely on their presence in generic code to avoid unnecessary cloning. This includes the built-in types.

However, the comparison operators and PartialOrd don't consume their arguments. They are implicitly borrowed: b < *aa is equivalent to PartialOrd::lt(&b, &*aa). So, in my opinion, it's less important to have i32 implement PartialOrd<&i32> because you could just as well write b < *aa even in generic contexts without the Copy bound.

[Gankra]

I have mixed feelings on combinatoric impls so people don't have to think about indirections too much. @sellibitze's summary is apt.

[nikomatsakis]

I agree the summary is good. I personally don't feel a strong need to make
this change.

On Mon, Sep 28, 2015 at 10:13 AM, Alexis Beingessner <
[email protected]> wrote:

I have mixed feelings on combinatoric impls so people don't have to think
about indirections too much. @sellibitze https://github.com/sellibitze's
summary is apt.

—
Reply to this email directly or view it on GitHub
#28293 (comment).

[jfager]

Based on comments seems like this should be closed as intended behavior?

[Mark-Simulacrum]

I'm going to close this -- I don't think we want these impls at this point. Thanks for the issue, though!