rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.45k stars 12.73k forks source link

Unhelpful type error with PartialOrd ambiguity #71538

Closed cuviper closed 1 year ago

cuviper commented 4 years ago

In num-bigint master, we've implemented PartialEq and PartialOrd between the bigint types and the primitive integers. I'm reconsidering this, because I've experienced a lot of type inference failures from afar, in modules that aren't using num-bigint at all. I'm not too surprised by that, because it really does add ambiguity, but some of the errors are not helpful.

Here's a reduced example: playground

use ndarray::{Array2, ShapeError};

fn main() {
    let iter = std::iter::once(42);
    let array = square_from_iter(iter).unwrap();
    assert!(array.nrows() < u8::MAX.into());
}

fn square_from_iter<A>(iter: impl Iterator<Item = A>) -> Result<Array2<A>, ShapeError> {
    let v: Vec<A> = iter.collect();
    let n = (v.len() as f64).sqrt() as usize;
    Array2::from_shape_vec((n, n), v)
}

#[allow(unused)]
mod break_inference {
    use std::cmp::Ordering;

    struct Foo;

    impl PartialEq<Foo> for usize {
        fn eq(&self, _: &Foo) -> bool {
            false
        }
    }

    impl PartialOrd<Foo> for usize {
        fn partial_cmp(&self, _: &Foo) -> Option<Ordering> {
            None
        }
    }
}
error[E0283]: type annotations needed for `ndarray::ArrayBase<ndarray::OwnedRepr<i32>, ndarray::dimension::dim::Dim<[usize; 2]>>`
 --> src/main.rs:6:27
  |
5 |     let array = square_from_iter(iter).unwrap();
  |         ----- consider giving `array` the explicit type `ndarray::ArrayBase<ndarray::OwnedRepr<i32>, ndarray::dimension::dim::Dim<[usize; 2]>>`, where the type parameter `usize` is specified
6 |     assert!(array.nrows() < u8::MAX.into());
  |                           ^ cannot infer type for type `usize`
  |
  = note: cannot satisfy `usize: std::cmp::PartialOrd<_>`

AFAICS the "needed for _" and the suggested "explicit type" are identical, and it doesn't change anything if I add that type on array anyway (using ndarray::Dim instead of that private path). Besides, nrows() always returns usize, and "cannot infer type for type usize" is nonsense.

The last note about PartialOrd<_> is the only part that really seems relevant, although I think it could also mention that u8::MAX.into() is ambiguous. In this example, usize implements PartialOrd<usize> and PartialOrd<Foo>, and u8 implements lots of Into<{integer}>, though not Into<Foo>. The compiler could match up the only answer, PartialOrd<usize> and Into<usize>, but I guess it's not that aggressive.

In the full num-bigint case, we would have u8: Into<BigInt> + Into<BigUint> too, so then matching PartialOrd and Into really is ambiguous even if the compiler tried harder.

Meta

rustc --version --verbose:

$ rustc +nightly -Vv
rustc 1.44.0-nightly (14b15521c 2020-04-23)
binary: rustc
commit-hash: 14b15521c52549ebbb113173b4abecd124b5a823
commit-date: 2020-04-23
host: x86_64-unknown-linux-gnu
release: 1.44.0-nightly
LLVM version: 9.0
wabain commented 3 years ago

This seems to have improved between 1.48.0 and 1.49.0-beta.5. Now we get:

error[E0283]: type annotations needed
 --> src/main.rs:6:27
  |
6 |     assert!(array.nrows() < u8::MAX.into());
  |                           ^ -------------- this method call resolves to `T`
  |                           |
  |                           cannot infer type
  |
  = note: cannot satisfy `usize: PartialOrd<_>`

T comes a bit out of nowhere (the Into trait declaration) and there could probably be some suggestions or help around how comparisons desugar, but the message is substantially correct.

estebank commented 1 year ago

Current output:

error[E0283]: type annotations needed
  --> src/main.rs:6:37
   |
6  |     assert!(array.nrows() < u8::MAX.into());
   |                           -         ^^^^
   |                           |
   |                           type must be known at this point
   |
note: multiple `impl`s satisfying `usize: PartialOrd<_>` found
  --> src/main.rs:27:5
   |
27 |     impl PartialOrd<Foo> for usize {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = note: and another `impl` found in the `core` crate: `impl PartialOrd for usize;`
help: try using a fully qualified path to specify the expected types
   |
6  |     assert!(array.nrows() < <u8 as Into<T>>::into(u8::MAX));
   |                             ++++++++++++++++++++++       ~