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

cmp on non-Ord type says problem is type doesn't implement Iterator #113550

Open
carols10cents opened this issue Jul 10, 2023 · 2 comments
Open
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@carols10cents
Copy link
Member

carols10cents commented Jul 10, 2023

Code

playground

#[derive(Debug, Eq, PartialEq)]
struct OopsNoOrd(i32);

#[derive(Debug)]
struct Container {
    inner: OopsNoOrd,
}

fn main() {
    let mut list = vec![
        Container { inner: OopsNoOrd(1) }, 
        Container { inner: OopsNoOrd(10) },
        Container { inner: OopsNoOrd(3) },
    ];
    
    list.sort_by(|a, b| a.inner.cmp(&b.inner));
    
    dbg!(&list);
}

Current output

error[E0599]: `OopsNoOrd` is not an iterator
  --> src/main.rs:16:33
   |
2  | struct OopsNoOrd(i32);
   | ----------------
   | |
   | method `cmp` not found for this struct
   | doesn't satisfy `OopsNoOrd: Iterator`
...
16 |     list.sort_by(|a, b| a.inner.cmp(&b.inner));
   |                                 ^^^ `OopsNoOrd` is not an iterator
   |
   = note: the following trait bounds were not satisfied:
           `OopsNoOrd: Iterator`
           which is required by `&mut OopsNoOrd: Iterator`
note: the trait `Iterator` must be implemented
  --> /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/iter/traits/iterator.rs:74:1
   = help: items from traits can only be used if the trait is implemented and in scope
   = note: the following trait defines an item `cmp`, perhaps you need to implement it:
           candidate #1: `Iterator`

Desired output

I would like the output to recommend implementing or deriving Ord to get cmp instead of saying "the trait Iterator must be implemented", as implementing Ord is much more likely to be what I'm trying to do (in my experience).

Rationale and extra context

I tried calling cmp on a custom type that I had forgotten to derive Ord on, and I was surprised to see the compiler tell me the problem was that I didn't implement Iterator (which also has a cmp method, but isn't the best/most likely trait to provide cmp in this case).

Other cases

Interestingly, if I call sort_by a vec of OopsNoOrds such that I'm calling cmp on &OopsNoOrd instead of OopsNoOrd, then the compiler says I need both Ord and Iterator. This code:

#[derive(Debug, Eq, PartialEq)]
struct OopsNoOrd(i32);

fn main() {
    let mut list2 = vec![OopsNoOrd(1), OopsNoOrd(10), OopsNoOrd(3)];
    list2.sort_by(|a, b| a.cmp(&b));
    dbg!(list2);
}

gives me:

error[[E0599]](https://doc.rust-lang.org/stable/error_codes/E0599.html): the method `cmp` exists for reference `&OopsNoOrd`, but its trait bounds were not satisfied
 --> src/main.rs:6:28
  |
2 | struct OopsNoOrd(i32);
  | ----------------
  | |
  | doesn't satisfy `OopsNoOrd: Iterator`
  | doesn't satisfy `OopsNoOrd: Ord`
...
6 |     list2.sort_by(|a, b| a.cmp(&b));
  |                            ^^^ method cannot be called on `&OopsNoOrd` due to unsatisfied trait bounds
  |
  = note: the following trait bounds were not satisfied:
          `OopsNoOrd: Ord`
          which is required by `&OopsNoOrd: Ord`
          `&OopsNoOrd: Iterator`
          which is required by `&mut &OopsNoOrd: Iterator`
          `OopsNoOrd: Iterator`
          which is required by `&mut OopsNoOrd: Iterator`
note: the trait `Iterator` must be implemented
 --> /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/iter/traits/iterator.rs:74:1
help: consider annotating `OopsNoOrd` with `#[derive(Eq, Ord, PartialEq, PartialOrd)]`
  |
2 + #[derive(Eq, Ord, PartialEq, PartialOrd)]
3 | struct OopsNoOrd(i32);
  |

Anything else?

I'm using rustc 1.70.0, and I'm seeing the same compiler output with beta 1.71.0-beta.6 and nightly 2023-07-09 1065d87.

@carols10cents carols10cents added 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 Jul 10, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 10, 2023
@compiler-errors
Copy link
Member

compiler-errors commented Jul 10, 2023

How strange. Seems like method selection prefers Iterator::cmp over Ord::cmp.

Someone needs to look into method selection to understand why we're not just bailing with ambiguity here, since we have two methods to choose from in scope. Not sure if this is possible to fix given the impls provided by libstd and not wanting to break existing code that passes...

@compiler-errors compiler-errors added D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jul 10, 2023
@VorpalBlade
Copy link

I ran into this today, with an inner type that I had by mistake only derived PartialOrd, not Ord for. The outer type needed a manual implementation of PartialOrd and Ord and thus I got this error there.

I guess no one took a look at this over the last year?

hodgesds added a commit to hodgesds/scx that referenced this issue Sep 20, 2024
Add extra ordering macros for Core/CPU structs for ease of use with
Rust standard library features. This issue was hit when trying to sort
cores based on the CoreType. See this similar issue for details:
rust-lang/rust#113550

Signed-off-by: Daniel Hodges <[email protected]>
likewhatevs pushed a commit to sched-ext/scx-backports that referenced this issue Oct 4, 2024
Add extra ordering macros for Core/CPU structs for ease of use with
Rust standard library features. This issue was hit when trying to sort
cores based on the CoreType. See this similar issue for details:
rust-lang/rust#113550

Signed-off-by: Daniel Hodges <[email protected]>
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 D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. 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

4 participants