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

In presence of impl <T: Trait1> Trait2 for T, when X: Trait2 fails, Rust guides you towards satisfying X: Trait1 instead of X: Trait2 #124802

Open
GrigorenkoPV opened this issue May 6, 2024 · 4 comments
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@GrigorenkoPV
Copy link
Contributor

GrigorenkoPV commented May 6, 2024

Code

use rayon::iter::{IntoParallelIterator, ParallelIterator};

pub fn parallelize<T>(v: Vec<T>) -> impl ParallelIterator<Item = T>
where
    // T: Send,
{
    v.into_par_iter()
}

Current output

error[E0599]: the method `into_par_iter` exists for struct `Vec<T>`, but its trait bounds were not satisfied
 --> src/lib.rs:7:7
  |
7 |     v.into_par_iter()
  |       ^^^^^^^^^^^^^
  |
  = note: the following trait bounds were not satisfied:
          `[T]: Sized`
          which is required by `[T]: IntoParallelIterator`
          `[T]: ParallelIterator`
          which is required by `[T]: IntoParallelIterator`

warning: unused import: `IntoParallelIterator`
 --> src/lib.rs:1:19
  |
1 | use rayon::iter::{IntoParallelIterator, ParallelIterator};
  |                   ^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

Desired output

What's really missing is a T: Send bound.

Rationale and extra context

No response

Other cases

No response

Rust Version

rustc 1.79.0-beta.3 (f5d04caa7 2024-05-03)
binary: rustc
commit-hash: f5d04caa74a1dfa5ffc4082c2c8f621f25336bbc
commit-date: 2024-05-03
host: x86_64-unknown-linux-gnu
release: 1.79.0-beta.3
LLVM version: 18.1.4

Anything else?

I will try to provide a MCVE. In the meantime, @rustbot label +E-needs-mcve

@GrigorenkoPV GrigorenkoPV 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 May 6, 2024
@rustbot rustbot added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label May 6, 2024
@GrigorenkoPV
Copy link
Contributor Author

Here's a somewhat minimized example:
https://github.com/GrigorenkoPV/rust-issue-124802

error[E0599]: the method `into_pi` exists for enum `Option<T>`, but its trait bounds were not satisfied
 --> src/lib.rs:7:7
  |
7 |     v.into_pi()
  |       ^^^^^^^
  |
  = note: the following trait bounds were not satisfied:
          `&Option<T>: PI`
          which is required by `&Option<T>: IntoPI`
          `&mut Option<T>: PI`
          which is required by `&mut Option<T>: IntoPI`

warning: unused import: `IntoPI`
 --> src/lib.rs:1:10
  |
1 | use pi::{IntoPI, PI};
  |          ^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

If you inline pi intro the main crate, you get a longer error message

error[E0599]: the method `into_pi` exists for enum `Option<T>`, but its trait bounds were not satisfied
  --> src/lib.rs:39:7
   |
39 |     v.into_pi()
   |       ^^^^^^^
   |
note: the following trait bounds were not satisfied:
      `&Option<T>: PI`
      `&mut Option<T>: PI`
  --> src/lib.rs:13:9
   |
13 | impl<T: PI> IntoPI for T {
   |         ^^  ------     -
   |         |
   |         unsatisfied trait bound introduced here
   = help: items from traits can only be used if the trait is implemented and in scope
note: `IntoPI` defines an item `into_pi`, perhaps you need to implement it
  --> src/lib.rs:8:1
   |
8  | pub trait IntoPI {
   | ^^^^^^^^^^^^^^^^

indeed the culprit behind such weird diagnostic messages is this impl

impl<T: PI> IntoPI for T {
    type I = T;
    fn into_pi(self) -> Self::I {
        self
    }
}

without it, you get the correct suggestion to add the T: Send bound.

But if instead you remove the IntoPI for Option<T> impl, you'll get

   = note: the following trait bounds were not satisfied:
            `Option<T>: PI`
            which is required by `Option<T>: IntoPI`
            `&Option<T>: PI`
            which is required by `&Option<T>: IntoPI`
            `&mut Option<T>: PI`
            which is required by `&mut Option<T>: IntoPI`

which now has 3 items instead of two.

Similarly, adding IntoPI for &{mut,} Option<T> without removing any of the present impls, will affect the message. And with all 3 impls in place, you get

error[E0599]: no method named `into_pi` found for enum `Option` in the current scope
 --> src/lib.rs:7:7
  |
7 |     v.into_pi()
  |       ^^^^^^^
  |
help: there is a method `into` with a similar name
  |
7 |     v.into()
  |       ~~~~

warning: unused import: `IntoPI`
 --> src/lib.rs:1:10
  |
1 | use pi::{IntoPI, PI};
  |          ^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

TL;DR

So, to recap. You have two traits. I and IntoI. You also have a blanket <T: I> IntoI for T impl. You also have a number of IntoI for X impl. Then if some conditions for this IntoI for X impl do not get satisfied, Rust will completely ignore it. And instead of suggesting to add some extra clauses to satisfy this impl, it will instead guide you towards satisfying X: I/&X: I/&mut X: I for that blanket impl to kick in.

So I guess @rustbot label -E-needs-mcve

@rustbot rustbot removed the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label May 6, 2024
@GrigorenkoPV GrigorenkoPV changed the title Confusing diagnostics when missing a : Send bound with rayon's IntoParallelIterator In presence of impl <T: Trait1> Trait2 for T, when X: Trait2 fails, Rust guides you towards satisfying X: Trait1 instead of X: Trait2 May 6, 2024
@rustbot rustbot added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label May 6, 2024
@brownie-in-motion
Copy link

brownie-in-motion commented May 12, 2024

In this case, we get a suggestion to restrict T: S only when impl<T: S> A for T {} is commented out.

trait S {}
trait A {}

// impl<T: S> A for T {}
impl<T: S> A for Option<T> {}

fn f<T>(v: Option<T>) -> impl A { v }

Is that similar to the issue that's being brought up?

@GrigorenkoPV
Copy link
Contributor Author

Is that similar to the issue that's being brought up?

I think so

@dianne
Copy link
Contributor

dianne commented Nov 8, 2024

@rustbot claim

workingjubilee added a commit to workingjubilee/rustc that referenced this issue Nov 9, 2024
Don't suggest `.into_iter()` on iterators

This makes the the suggestion to call `.into_iter()` only consider unsatisfied `Iterator` bounds for the receiver type itself. That way, it ignores predicates generated by trying to auto-ref the receiver (the result of which usually won't implement `Iterator`).

Fixes rust-lang#127511

Unfortunately, the error in that case is still confusing: it labels `Iterator` as an unsatisfied bound because `&impl Iterator: Iterator` can't be satisfied, despite that not being required or helpful. I'd like to handle that in a separate PR. ~~I'm hoping fixing rust-lang#124802 will fix it too.~~ It doesn't look connected to that issue. Still, I think it'd be clearest to visually distinguish unsatisfied predicates from different attempts at `pick_method`; I'll make a PR for that soon.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Nov 9, 2024
Rollup merge of rust-lang#132760 - dianne:iter-into-iter, r=lcnr

Don't suggest `.into_iter()` on iterators

This makes the the suggestion to call `.into_iter()` only consider unsatisfied `Iterator` bounds for the receiver type itself. That way, it ignores predicates generated by trying to auto-ref the receiver (the result of which usually won't implement `Iterator`).

Fixes rust-lang#127511

Unfortunately, the error in that case is still confusing: it labels `Iterator` as an unsatisfied bound because `&impl Iterator: Iterator` can't be satisfied, despite that not being required or helpful. I'd like to handle that in a separate PR. ~~I'm hoping fixing rust-lang#124802 will fix it too.~~ It doesn't look connected to that issue. Still, I think it'd be clearest to visually distinguish unsatisfied predicates from different attempts at `pick_method`; I'll make a PR for that soon.
mati865 pushed a commit to mati865/rust that referenced this issue Nov 12, 2024
Don't suggest `.into_iter()` on iterators

This makes the the suggestion to call `.into_iter()` only consider unsatisfied `Iterator` bounds for the receiver type itself. That way, it ignores predicates generated by trying to auto-ref the receiver (the result of which usually won't implement `Iterator`).

Fixes rust-lang#127511

Unfortunately, the error in that case is still confusing: it labels `Iterator` as an unsatisfied bound because `&impl Iterator: Iterator` can't be satisfied, despite that not being required or helpful. I'd like to handle that in a separate PR. ~~I'm hoping fixing rust-lang#124802 will fix it too.~~ It doesn't look connected to that issue. Still, I think it'd be clearest to visually distinguish unsatisfied predicates from different attempts at `pick_method`; I'll make a PR for that soon.
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 E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example 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