RFC: implement Rem for all equally signed integers where RHS < LHS

[lcnr]

rendered

Unresolved questions

• For which types should Rem<usize/isize> be implemented for.

[nagisa]

This will break/change type inference in some instances, I think. While historically that isn’t considered a breaking change that must never be done, this would have scope larger than usual, I think.

[lcnr]

@nagisa I do not think that it is going to be that impactful, as the return type and the second argument have the same type. So we only need one of them to be used in a different operation.
I will try it out.

[lcnr]

I've tested the rust compiler with ./x.py test after implementing this rfc. No regression there, at least 😄 . We might still need a crater run though.

[petrochenkov]

This has same issue as previous proposals to implement comparisons for heterogeneous operands (#2021).
Type mismatch between operands usually means that you have a mistake and should change types somewhere else, rather than an intended heterogeneous operation.

Performing heterogeneous operations on integers correctly, if they are actually intended, is a real issue though.
Since they are not common, I'd rather see separate library functions for them, rather than reusing operators and losing the error detection aspect.

[scottmcm]

To demonstrate the breakage that nagisa mentioned:

use std::ops::Rem;
struct Local<T>(T);

// What we have today
impl Rem for Local<i16> {
    type Output = Self;
    fn rem(self, x: Self) -> Self { Local(self.0 % x.0) }
}

// Adding this new one breaks the `main` below
impl Rem<Local<i8>> for Local<i16> {
    type Output = Local<i8>;
    fn rem(self, x: Local<i8>) -> Local<i8> { Local((self.0 % x.0 as i16) as i8) }
}

fn main() {
    let _ = Local(1_i16) % Local(4);
    // ERROR:            ^ no implementation for `Local<i16> % Local<i32>`
}

https://play.rust-lang.org/?edition=2018&gist=e7d759acb7e30d0b762a920520366f91

[leonardo-m]

I'd like to face such problem in a more principled and more general solution compared to this, that's based on a value range analysis instead.

[lcnr]

Performing heterogeneous operations on integers correctly, if they are actually intended, is a real issue though.
Since they are not common, I'd rather see separate library functions for them, rather than reusing operators and losing the error detection aspect.

In my opinion separate library functions could actually be enough, as long as they are part of std/core as I highly doubt that people would include a crate + trait for something which can be manually implemented (even if it is more prone to bugs). After looking at past RFCs / issues concerning heterogeneous operations. I realized that it will be far more inconsistent than I thought to only implement these operations for Rem, as the similar bugs exist when working with comparisons etc.