Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unhelpful type error with PartialOrd ambiguity #71538

Closed
cuviper opened this issue Apr 24, 2020 · 2 comments
Closed

Unhelpful type error with PartialOrd ambiguity #71538

cuviper opened this issue Apr 24, 2020 · 2 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@cuviper
Copy link
Member

cuviper commented Apr 24, 2020

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
@cuviper cuviper added C-bug Category: This is a bug. A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 24, 2020
@estebank estebank added D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. labels Jun 13, 2020
@wabain
Copy link
Contributor

wabain commented Dec 24, 2020

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
Copy link
Contributor

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));
   |                             ++++++++++++++++++++++       ~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants