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

"Trait bounds were not satisfied" errors when inference is bailing #89418

Open
Manishearth opened this issue Oct 1, 2021 · 15 comments
Open

"Trait bounds were not satisfied" errors when inference is bailing #89418

Manishearth opened this issue Oct 1, 2021 · 15 comments
Labels
A-inference Area: Type inference T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Manishearth
Copy link
Member

Spun off of #89196

Code example
trait MiniYokeable<'a> {
    type Output;
}

struct MiniYoke<Y: for<'a> MiniYokeable<'a>> {
    pub yokeable: Y,
}

impl<Y> Clone for MiniYoke<Y>
where
    Y: for<'a> MiniYokeable<'a>,
    for<'a> <Y as MiniYokeable<'a>>::Output: Clone
{
    fn clone(&self) -> Self {
        todo!()
    }
}

trait MiniDataMarker {
    type Yokeable: for<'a> MiniYokeable<'a>;
}

trait MiniDataProvider<M>
where
    M: MiniDataMarker
{
    fn mini_load_payload(&self) -> MiniYoke<M::Yokeable>;
}

struct MiniStructProvider<M>
where
    M: MiniDataMarker,
{
    pub yoke: MiniYoke<M::Yokeable>,
}

impl<M> MiniDataProvider<M> for MiniStructProvider<M>
where
    M: MiniDataMarker,
    for<'a> <M::Yokeable as MiniYokeable<'a>>::Output: Clone,
{
    fn mini_load_payload(&self) -> MiniYoke<M::Yokeable> {
        self.yoke.clone()
    }
}

#[derive(Clone)]
struct SimpleStruct(pub u32);

impl<'a> MiniYokeable<'a> for SimpleStruct {
    type Output = SimpleStruct;
}

impl MiniDataMarker for SimpleStruct {
    type Yokeable = SimpleStruct;
}

fn main() {
    let provider = MiniStructProvider {
        yoke: MiniYoke {
            yokeable: SimpleStruct(42)
        }
    };

    let yoke: MiniYoke<SimpleStruct> = provider.mini_load_payload();
    assert_eq!(yoke.yokeable.0, 42);
}

(playpen)

This fails to compile (needs beta or nightly to avoid #85636 , fixed by #85499 ) with the following error:

error[E0599]: the method `mini_load_payload` exists for struct `MiniStructProvider<_>`, but its trait bounds were not satisfied
  --> src/main.rs:65:49
   |
30 | / struct MiniStructProvider<M>
31 | | where
32 | |     M: MiniDataMarker,
33 | | {
34 | |     pub yoke: MiniYoke<M::Yokeable>,
35 | | }
   | | -
   | | |
   | |_method `mini_load_payload` not found for this
   |   doesn't satisfy `MiniStructProvider<_>: MiniDataProvider<_>`
...
65 |       let yoke: MiniYoke<SimpleStruct> = provider.mini_load_payload();
   |                                                   ^^^^^^^^^^^^^^^^^ method cannot be called on `MiniStructProvider<_>` due to unsatisfied trait bounds
   |
   = note: the following trait bounds were not satisfied:
           `<_ as MiniYokeable<'a>>::Output: Clone`
           which is required by `MiniStructProvider<_>: MiniDataProvider<_>`

This error is incorrect, the trait bounds were satisfied. What's going on here is that the explicit type of provider is not known, and inference is unable to figure it out since there can be multiple M: MiniDataMarker types for the same M::Yokeable value. The fix is to be explicit on line 59 and use let provider = MiniStructProvider::<SimpleStruct> { instead, which avoids this issue.

Another version of this error can be seen in the code in this comment (playpen), which has a similar error due to inference bailing early (there are "Broken" and "Fixed" versions in the example so that you can see the specific thing inference needs to avoid the error)

cc @estebank

@Manishearth Manishearth 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 Oct 1, 2021
@eddyb
Copy link
Member

eddyb commented Oct 1, 2021

This error is incorrect, the trait bounds were satisfied

They're not, see #89196 (comment).

I think there's a diagnostic issue here but I'm not sure I can easily describe it.

Actually, looking again at the output, I missed this part:

   = note: the following trait bounds were not satisfied:
           `<_ as MiniYokeable<'a>>::Output: Clone`
           which is required by `MiniStructProvider<_>: MiniDataProvider<_>`

So IMO the only bug here is that this isn't pointing at the bound in the code, with a span.

For that, we can compare calling a method via path vs method call syntax (try in playground):

struct Foo;
impl Foo {
    fn method<T>(self)
        // bound that never holds, `Vec<T>` instead of `T` to bypass `_: Trait`.
        where
            Vec<T>: Copy // this line doesn't show up in lacks_span error output
    {}
}

fn has_span() { Foo::method(Foo) }

fn lacks_span() { Foo.method() }
error[E0277]: the trait bound `Vec<_>: Copy` is not satisfied
  --> src/lib.rs:10:17
   |
3  |     fn method<T>(self)
   |        ------ required by a bound in this
...
6  |             Vec<T>: Copy // this line doesn't show up in lacks_span error output
   |                     ---- required by this bound in `Foo::method`
...
10 | fn has_span() { Foo::method(Foo) }
   |                 ^^^^^^^^^^^ the trait `Copy` is not implemented for `Vec<_>`

error[E0277]: the trait bound `Vec<_>: Copy` is not satisfied
  --> src/lib.rs:12:23
   |
12 | fn lacks_span() { Foo.method() }
   |                       ^^^^^^ the trait `Copy` is not implemented for `Vec<_>`

I'm not sure if this issue should be closed or changed, but I feel like the change is too big to keep it.

@estebank estebank added D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. labels Oct 1, 2021
@Manishearth
Copy link
Member Author

@eddyb I think you're right that the core diagnostic (unable to determine _: Trait) is correct, but when in context _ does implement Trait (it's just that inference doesn't know enough to say so) I think we could be more clear on that, and suggest disambiguating exactly which type _ is or exactly which trait Trait is

@Manishearth
Copy link
Member Author

At the moment, a very reasonable reaction to this error is "This error is incorrect and a compiler bug", and I think that's a pretty good indicator of a bad diagnostic, regardless of the specifics of it :)

@Manishearth
Copy link
Member Author

Note: @eddyb has a more reduced version of the error here, which is pointing to a normalization issue:

#89196 (comment)

But it may be possible to fix the diagnostics without fixing that.

@estebank
Copy link
Contributor

estebank commented Oct 5, 2021

The E0277 without context is a MiscObligation. There are about 100~ of those introduced in different places. The culprit should be able to be found by hand and make it propagate the original BindingObligation instead.


Edit: it's in the call to self.add_obligations in rustc_typeck::check::method::confirm::confirm using a MiscObligation for all of them. I managed to point at the method easily, but that's not exactly correct:

error[E0277]: the trait bound `Vec<_>: Copy` is not satisfied
  --> f16.rs:12:23
   |
12 | fn lacks_span() { Foo.method() }
   |                       ^^^^^^ the trait `Copy` is not implemented for `Vec<_>`
   |
note: required by `Foo::method`
  --> f16.rs:3:5
   |
3  | /     fn method<T>(self)
4  | |         // bound that never holds, `Vec<T>` instead of `T` to bypass `_: Trait`.
5  | |         where
6  | |             Vec<T>: Copy // this line doesn't show up in lacks_span error output
   | |________________________^

Edit 2: damn you two and your nerdsniping reasonable requests

Screen Shot 2021-10-05 at 6 44 33 AM

There's no change for the original case, though:

Screen Shot 2021-10-05 at 6 45 38 AM

@estebank
Copy link
Contributor

estebank commented Oct 13, 2021

I managed to get the following output for the original report, @Manishearth are there things you would change on it?

error[E0599]: the method `mini_load_payload` exists for struct `MiniStructProvider<_>`, but its trait bounds were not satisfied
  --> f16.rs:85:59
   |
47 | / struct MiniStructProvider<M>
48 | | where
49 | |     M: MiniDataMarker,
50 | | {
51 | |     pub payload: MiniDataPayload<M>,
52 | | }
   | | -
   | | |
   | |_method `mini_load_payload` not found for this
   |   doesn't satisfy `MiniStructProvider<_>: MiniDataProvider<_>`
...
85 |       let payload: MiniDataPayload<SimpleStruct> = provider.mini_load_payload();
   |                                                             ^^^^^^^^^^^^^^^^^ method cannot be called on `MiniStructProvider<_>` due to unsatisfied trait bounds
   |
note: the following trait bounds were not satisfied because of the requirements of the implementation of `MiniDataProvider<_>` for `_`:
      `<_ as MiniYokeable<'a>>::Output: Clone`
  --> f16.rs:54:9
   |
54 | impl<M> MiniDataProvider<M> for MiniStructProvider<M>
   |         ^^^^^^^^^^^^^^^^^^^     ^^^^^^^^^^^^^^^^^^^^^

Screen Shot 2021-10-13 at 7 28 54 AM

Ideally we could point at the specific bound for Clone, but I think it is an improvement.

@Manishearth
Copy link
Member Author

@estebank Seems a bit better and we should land that, but I think the core problem isn't quite solved: The trait is implemented, the compiler just doesn't realize it because it hasn't figured out _, and perhaps a nudge from the compiler about "perhaps help me understand what _ is?" would be useful? As an experienced Rust programmer I know what to expect when I see _ but others don't and I think that's a common thing that crops up in rust errors, that is often obvious from context but less obvious in gnarly cases like this.

Ultimately, the solution here was to tell the compiler what _ was, and that sounds like something it should be able to help us do.

Also, why does it say "the implementation of MiniDataProvider<_> for _"? Shouldn't it say " the implementation of "MiniDataProvider<_> for MiniStructProvider<_>"?

@estebank
Copy link
Contributor

In order to turn _ into MiniDataProvider<_> I need to call resolve_vars_if_possible, which is giving me an ICE due to trim_paths being called somewhere it shouldn't, so I'm skipping it at the moment. I think I can clean it up, but at the moment I'm at the limit of what I can do on this session.

@Manishearth
Copy link
Member Author

Totally fair

estebank added a commit to estebank/rust that referenced this issue Nov 18, 2021
Be more thorough in using `ItemObligation` and `BindingObligation` when
evaluating obligations so that we can point at trait bounds that
introduced unfulfilled obligations. We no longer incorrectly point at
unrelated trait bounds (`substs-ppaux.verbose.stderr`).

In particular, we now point at trait bounds on method calls.

We no longer point at "obvious" obligation sources (we no longer have a
note pointing at `Trait` saying "required by a bound in `Trait`", like
in `associated-types-no-suitable-supertrait*`).

Address part of rust-lang#89418.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 18, 2021
…=nagisa

Point at source of trait bound obligations in more places

Be more thorough in using `ItemObligation` and `BindingObligation` when
evaluating obligations so that we can point at trait bounds that
introduced unfulfilled obligations. We no longer incorrectly point at
unrelated trait bounds (`substs-ppaux.verbose.stderr`).

In particular, we now point at trait bounds on method calls.

We no longer point at "obvious" obligation sources (we no longer have a
note pointing at `Trait` saying "required by a bound in `Trait`", like
in `associated-types-no-suitable-supertrait*`).

We no longer point at associated items (`ImplObligation`), as they didn't
add any user actionable information, they just added noise.

Address part of rust-lang#89418.
@estebank
Copy link
Contributor

Update: looked a little bit deeper into this and this is what I got to:

Screen Shot 2021-11-18 at 2 23 15 PM

error[E0599]: the method `mini_load_payload` exists for struct `MiniStructProvider<_>`, but its trait bounds were not satisfied
  --> f22.rs:65:49
   |
30 | / struct MiniStructProvider<M>
31 | | where
32 | |     M: MiniDataMarker,
33 | | {
34 | |     pub yoke: MiniYoke<M::Yokeable>,
35 | | }
   | | -
   | | |
   | |_method `mini_load_payload` not found for this
   |   doesn't satisfy `MiniStructProvider<_>: MiniDataProvider<_>`
...
65 |       let yoke: MiniYoke<SimpleStruct> = provider.mini_load_payload();
   |                                                   ^^^^^^^^^^^^^^^^^ method cannot be called on `MiniStructProvider<_>` due to unsatisfied trait bounds
   |
note: the following trait bounds were not satisfied because of the requirements of the implementation of `MiniDataProvider<_>` for `_`:
      `<_ as MiniYokeable<'a>>::Output: Clone`
  --> f22.rs:40:56
   |
37 | impl<M> MiniDataProvider<M> for MiniStructProvider<M>
   |         -------------------     --------------------- ...for this type
   |         |
   |         in the implementation of this trait...
...
40 |     for<'a> <M::Yokeable as MiniYokeable<'a>>::Output: Clone,
   |                                                        ^^^^^ obligation introduced here

Expect a follow up PR for that work, as the current changes improve things here but break others.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 18, 2021
…=nagisa

Point at source of trait bound obligations in more places

Be more thorough in using `ItemObligation` and `BindingObligation` when
evaluating obligations so that we can point at trait bounds that
introduced unfulfilled obligations. We no longer incorrectly point at
unrelated trait bounds (`substs-ppaux.verbose.stderr`).

In particular, we now point at trait bounds on method calls.

We no longer point at "obvious" obligation sources (we no longer have a
note pointing at `Trait` saying "required by a bound in `Trait`", like
in `associated-types-no-suitable-supertrait*`).

We no longer point at associated items (`ImplObligation`), as they didn't
add any user actionable information, they just added noise.

Address part of rust-lang#89418.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 19, 2021
…=nagisa

Point at source of trait bound obligations in more places

Be more thorough in using `ItemObligation` and `BindingObligation` when
evaluating obligations so that we can point at trait bounds that
introduced unfulfilled obligations. We no longer incorrectly point at
unrelated trait bounds (`substs-ppaux.verbose.stderr`).

In particular, we now point at trait bounds on method calls.

We no longer point at "obvious" obligation sources (we no longer have a
note pointing at `Trait` saying "required by a bound in `Trait`", like
in `associated-types-no-suitable-supertrait*`).

We no longer point at associated items (`ImplObligation`), as they didn't
add any user actionable information, they just added noise.

Address part of rust-lang#89418.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 19, 2021
…=nagisa

Point at source of trait bound obligations in more places

Be more thorough in using `ItemObligation` and `BindingObligation` when
evaluating obligations so that we can point at trait bounds that
introduced unfulfilled obligations. We no longer incorrectly point at
unrelated trait bounds (`substs-ppaux.verbose.stderr`).

In particular, we now point at trait bounds on method calls.

We no longer point at "obvious" obligation sources (we no longer have a
note pointing at `Trait` saying "required by a bound in `Trait`", like
in `associated-types-no-suitable-supertrait*`).

We no longer point at associated items (`ImplObligation`), as they didn't
add any user actionable information, they just added noise.

Address part of rust-lang#89418.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 19, 2021
…=nagisa

Point at source of trait bound obligations in more places

Be more thorough in using `ItemObligation` and `BindingObligation` when
evaluating obligations so that we can point at trait bounds that
introduced unfulfilled obligations. We no longer incorrectly point at
unrelated trait bounds (`substs-ppaux.verbose.stderr`).

In particular, we now point at trait bounds on method calls.

We no longer point at "obvious" obligation sources (we no longer have a
note pointing at `Trait` saying "required by a bound in `Trait`", like
in `associated-types-no-suitable-supertrait*`).

We no longer point at associated items (`ImplObligation`), as they didn't
add any user actionable information, they just added noise.

Address part of rust-lang#89418.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 19, 2021
…=nagisa

Point at source of trait bound obligations in more places

Be more thorough in using `ItemObligation` and `BindingObligation` when
evaluating obligations so that we can point at trait bounds that
introduced unfulfilled obligations. We no longer incorrectly point at
unrelated trait bounds (`substs-ppaux.verbose.stderr`).

In particular, we now point at trait bounds on method calls.

We no longer point at "obvious" obligation sources (we no longer have a
note pointing at `Trait` saying "required by a bound in `Trait`", like
in `associated-types-no-suitable-supertrait*`).

We no longer point at associated items (`ImplObligation`), as they didn't
add any user actionable information, they just added noise.

Address part of rust-lang#89418.
estebank added a commit to estebank/rust that referenced this issue Nov 20, 2021
Be more thorough in using `ItemObligation` and `BindingObligation` when
evaluating obligations so that we can point at trait bounds that
introduced unfulfilled obligations. We no longer incorrectly point at
unrelated trait bounds (`substs-ppaux.verbose.stderr`).

In particular, we now point at trait bounds on method calls.

We no longer point at "obvious" obligation sources (we no longer have a
note pointing at `Trait` saying "required by a bound in `Trait`", like
in `associated-types-no-suitable-supertrait*`).

Address part of rust-lang#89418.
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 21, 2021
…agisa

Point at source of trait bound obligations in more places

Be more thorough in using `ItemObligation` and `BindingObligation` when
evaluating obligations so that we can point at trait bounds that
introduced unfulfilled obligations. We no longer incorrectly point at
unrelated trait bounds (`substs-ppaux.verbose.stderr`).

In particular, we now point at trait bounds on method calls.

We no longer point at "obvious" obligation sources (we no longer have a
note pointing at `Trait` saying "required by a bound in `Trait`", like
in `associated-types-no-suitable-supertrait*`).

We no longer point at associated items (`ImplObligation`), as they didn't
add any user actionable information, they just added noise.

Address part of rust-lang#89418.
estebank added a commit to estebank/rust that referenced this issue Mar 24, 2022
Instead of probing for all possible impls that could have caused an
`ImplObligation`, keep track of its `DefId` and obligation spans for
accurate error reporting.

Follow up to rust-lang#89580. Addresses rust-lang#89418.

Remove some unnecessary clones.

Tweak output for auto trait impl obligations.
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 24, 2022
…=oli-obk

Properly track `ImplObligations`

Instead of probing for all possible `impl`s that could have caused an
`ImplObligation`, keep track of its `DefId` and obligation spans for
accurate error reporting.

Follow to rust-lang#89580. Addresses rust-lang#89418.
@estebank
Copy link
Contributor

estebank commented Mar 25, 2022

As of tomorrow's nightly, the output for the original report will be almost the same as #89418 (comment).

eddyb's case is already handled in stable.

@estebank estebank removed D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. labels Jun 2, 2022
@estebank
Copy link
Contributor

estebank commented Jul 7, 2022

Taking a quick peek at this again, it feels like we should probably identify bounds like Vec<T>: Copy that trivially/always unsatisfiable and be clear about it with something along the lines of "this bound cannot be satisfied (because Vec<T> is never Copy)". What do you think @eddyb?

@estebank estebank added the A-inference Area: Type inference label Jul 7, 2022
@eddyb
Copy link
Member

eddyb commented Jul 7, 2022

@estebank Sounds plausible, but you might want to check with @rust-lang/types with regards to bounds that can be solved independent of generics - I vaguely remember that many years ago there were some plans around them, or at least certain subsets (including at least one feature gate - maybe it involved the word "trivial"?).

@estebank
Copy link
Contributor

estebank commented Nov 16, 2023

Current output:

error[E0599]: the method `mini_load_payload` exists for struct `MiniStructProvider<_>`, but its trait bounds were not satisfied
  --> src/main.rs:65:49
   |
30 | struct MiniStructProvider<M>
   | ----------------------------
   | |
   | method `mini_load_payload` not found for this struct
   | doesn't satisfy `MiniStructProvider<_>: MiniDataProvider<_>`
...
65 |     let yoke: MiniYoke<SimpleStruct> = provider.mini_load_payload();
   |                                                 ^^^^^^^^^^^^^^^^^ method cannot be called on `MiniStructProvider<_>` due to unsatisfied trait bounds
   |
note: trait bound `<_ as MiniYokeable<'a>>::Output: Clone` was not satisfied
  --> src/main.rs:40:56
   |
37 | impl<M> MiniDataProvider<M> for MiniStructProvider<M>
   |         -------------------     ---------------------
...
40 |     for<'a> <M::Yokeable as MiniYokeable<'a>>::Output: Clone,
   |                                                        ^^^^^ unsatisfied trait bound introduced here
error[E0277]: the trait bound `Vec<_>: Copy` is not satisfied
  --> src/lib.rs:10:17
   |
10 | fn has_span() { Foo::method(Foo) }
   |                 ^^^^^^^^^^^ the trait `Copy` is not implemented for `Vec<_>`
   |
note: required by a bound in `Foo::method`
  --> src/lib.rs:6:21
   |
3  |     fn method<T>(self)
   |        ------ required by a bound in this associated function
...
6  |             Vec<T>: Copy // this line doesn't show up in lacks_span error output
   |                     ^^^^ required by this bound in `Foo::method`

error[E0277]: the trait bound `Vec<_>: Copy` is not satisfied
  --> src/lib.rs:12:23
   |
12 | fn lacks_span() { Foo.method() }
   |                       ^^^^^^ the trait `Copy` is not implemented for `Vec<_>`
   |
note: required by a bound in `Foo::method`
  --> src/lib.rs:6:21
   |
3  |     fn method<T>(self)
   |        ------ required by a bound in this associated function
...
6  |             Vec<T>: Copy // this line doesn't show up in lacks_span error output
   |                     ^^^^ required by this bound in `Foo::method`

@lcnr
Copy link
Contributor

lcnr commented Nov 17, 2023

This is a bug in the old solver with its handling of ambiguous projections containing bound vars. It often treats them as if they were unnormalizable as it cannot eagerly replace them with inference variables. THe underlying issue will be fixed with the new trait solver: https://rust.godbolt.org/z/xGn8sbEE1

the higher ranked obligation for<'a> Trait(<_ as MiniYokeable<'a>>::Output: Clone) results in an error but should be ambiguous.

@estebank estebank removed A-diagnostics Area: Messages for errors, warnings, and lints D-confusing Diagnostics: Confusing error or lint that should be reworked. labels Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inference Area: Type inference 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