Rust tries to be too helpful with impl suggestions, sometimes being unhelpful #28894

sgrif · 2015-10-07T21:15:23Z

This code example is a simplified extraction of part of the hierarchy that I'm working on for https://github.com/sgrif/yaqb. It's still a bit contrived, but I think it really gets the point across about how unhelpful/nonsensical the resulting error message is.

The type structure goes like this: AsExpression represents a type which can be converted to an Expression which would return a given type. Expression represents... well, an expression. AsExpression gets a blanket impl for any type which implements Expression. Column represents a concrete column (which in the real code has other things associated to it). Ultimately the column has a type, and can be used as an expression of that type. Any type that implements Column gets a blanket impl for Expression.

In main, we're trying to compare a column representing an i32, to a str. This is incorrect, and should fail to compile. However, the error message we get out of this is:

error: the trait Column is not implemented for the type &str [E0277]

In the context of a SQL database, and what Column means, this is a nonsense error, and would not be helpful to the user in seeing why their code is failing to compile. Rust is just being super clever and realizing that the blanket impls would apply if we just implement that instead. I'd like to propose changing the error message to something with a bit more information, such as:

error: the trait AsExpression<i32> is not implemented for the type &str [E0277]

A blanket implementation of AsExpression, located at <file_path>:<line_number> could apply if Column were implemented for the type &str

The text was updated successfully, but these errors were encountered:

arielb1 · 2015-10-09T10:26:05Z

That was supposed to work.

I'm tired of seeing rustc recommend that I implement `Column` whenever I'm having issues resolving `SelectableExpression`. It's making things hard to debug, and I don't want to wait for rust-lang/rust#28894 to be fixed. The error behavior is awfully peculiar. I have no idea why this changed causes Rust to not even try and compile the second version of the same expression.

sgrif · 2016-01-08T21:29:52Z

Anyone have any opinions on my recommendation on how to improve this, or other possibilities?

sgrif · 2016-01-30T23:46:52Z

Is there anything I can do to get some attention on this? We're at the point where we're considering some sweeping/painful changes to our trait hierarchy to work around this, as there are cases that have been consistent sources of confusion in Diesel. (Our particular problem case, among others, is that rustc notices that we have impl<ST, T> AsExpression<ST> for T where T: Expression<SqlType=ST>, and never mentions AsExpression in the error message.)

I can't imagine that we have the only case where recommending the blanket impl without mentioning the actual trait it's trying to satisfy is problematic.

jonas-schievink · 2016-01-31T00:25:15Z

I think the obligation forest provides the functionality needed to fix this properly: #30976

Minified example:

trait Expression {}

trait Column {}

impl<C: Column> Expression for C {}


fn assert_expression<T: Expression>() {}

fn main() {
    assert_expression::<()>();
}

<anon>:11:5: 11:28 error: the trait `Column` is not implemented for the type `()` [E0277]
<anon>:11     assert_expression::<()>();
              ^~~~~~~~~~~~~~~~~~~~~~~
<anon>:11:5: 11:28 help: see the detailed explanation for E0277
<anon>:11:5: 11:28 note: required by `assert_expression`
error: aborting due to previous error

sgrif · 2016-01-31T01:16:14Z

It seems like your example demonstrates the problem, not that it's fixed? Expression isn't mentioned anywhere in that error message.

jonas-schievink · 2016-01-31T01:19:56Z

Yes, it's just a minified version of your code above. Sorry for not being clear about that.

sgrif · 2016-05-11T19:46:41Z

Had another user get bit by this today: diesel-rs/diesel#323

arielb1 · 2016-05-11T19:55:20Z

This is fixed on 1.10/nightly:

error: the trait bound `(): Column` is not satisfied [--explain E0277]
  --> <anon>:11:5
11 |>     assert_expression::<()>();
   |>     ^^^^^^^^^^^^^^^^^^^^^^^
note: required because of the requirements on the impl of `Expression` for `()`
note: required by `assert_expression`

error: aborting due to previous error
playpen: application terminated with error code 101

sgrif · 2016-05-11T20:11:07Z

I don't think this issue is fixed. This at least mentions the trait name, which is an improvement, but the error message is still confusing to me. In the example that you gave, it makes it sound like Column is a supertrait of Expression to me.

The case in diesel-rs/diesel#323 is also giving incorrect information.

src/macros/insertable.rs:206:13: 207:32 error: the trait bound `i32: expression::Expression` is not satisfied [E0277]
src/macros/insertable.rs:206             AsExpression::<<$column as Expression>::SqlType>
                                         ^
src/macros/insertable.rs:206:13: 207:32 help: run `rustc --explain E0277` to see a detailed explanation
src/macros/insertable.rs:206:13: 207:32 note: required because of the requirements on the impl of `expression::Expression` for `&i32`
src/macros/insertable.rs:206:13: 207:32 note: required by `expression::AsExpression::as_expression`
src/macros/insertable.rs:206:13: 207:47 error: mismatched types [E0308]

&i32 does not implement Expression, and the type parameter to AsExpression isn't even mentioned.

sgrif · 2016-05-11T20:15:10Z

It's still fairly trivial to get a case where the trait that's failing to apply isn't even mentioned. https://is.gd/DjS6Uk

@arielb1 I think there's still some work to do on this one, can we reopen this issue?

arielb1 · 2016-05-11T20:17:54Z

That looks like a different bug. Are investigating

pub struct Text;

pub trait Expression {
    type SqlType;
}

pub trait AsExpression<ST> {
    type Expression: Expression<SqlType=ST>;

    fn as_expression(self) -> Self::Expression;
}

impl<T, ST> AsExpression<ST> for T where
    T: Expression<SqlType=ST>,
{
    type Expression = Self;

    fn as_expression(self) -> Self::Expression {
        self
    }
}

fn main() {
    AsExpression::as_expression(Text);
}

arielb1 · 2016-05-14T23:35:43Z

@sgrif

In your case, the reported-upon Text: Expression predicate is coming from your trait declaration, rather than from any impl. We should not register these bounds directly, but there is some room for improvement.

arielb1 · 2016-05-14T23:38:21Z

I would like to refactor that code before we make any changes to it. cc @eddyb. (https://github.com/rust-lang/rust/blob/master/src/librustc_typeck/check/mod.rs#L4364)

eddyb · 2016-05-15T12:33:38Z

@arielb1 I will... try not to touch any of that. Except if I do anything to Substs, I suppose.

Do you want to track the reason for those obligations more accurately?
Although the code looks like it should do the right thing in terms of ObligationCause, but maybe that's not enough. I don't have any strong opinions about any of that function, really, as long as it works.

arielb1 · 2016-05-15T19:48:48Z

The problem is that we add obligations for the trait whose def-id refers to the method. If we would just add the FnSpace obligations + the Self: Trait predicate, we would get a cause that refers to the impl.

eddyb · 2016-05-15T22:53:41Z

@arielb1 Ah, I agree then that a single predicate would be better for cause tracking (maybe even more efficient?).

arielb1 · 2016-05-16T03:42:26Z

Do you have any plans for refactoring the above code, or can I try to hack on it myself?

eddyb · 2016-05-16T04:01:39Z

@arielb1 Go ahead, don't mind me.

behnam · 2017-08-09T22:48:11Z

I have faced a similar problem when trait A is required, and there's blanket impl B for A available, resulting in the error message to say trait B is required, which is not accurate.

More details: https://users.rust-lang.org/t/unexpected-behaviors-of-trait-bounds/12286

steveklabnik · 2019-03-14T16:07:59Z

Triage: other than the new format, the error message remains the same:

error[E0277]: the trait bound `&str: Column` is not satisfied
  --> src/main.rs:39:26
   |
39 |     let predicate = name.eq("Sean");
   |                          ^^ the trait `Column` is not implemented for `&str`
   |
   = note: required because of the requirements on the impl of `Expression` for `&str`
   = note: required because of the requirements on the impl of `AsExpression<std::string::String>` for `&str`

estebank · 2019-05-21T17:35:23Z

For nightly users, libraries can leverage rustc_on_unimplemented to provide better errors in these cases.

sgrif · 2019-05-21T17:38:56Z

#[rustc_on_unimplemented] doesn't really apply here, since Rust is showing a recommendation for the wrong trait, and won't show the custom message. https://play.rust-lang.org/?version=nightly&mode=debug&edition=2015&gist=963ede706e74095f9b99192b2ac0e5d7

estebank · 2019-05-21T18:05:19Z

@sgrif that's because you need to give the on_unimplemented to trait Column:

#[rustc_on_unimplemented(
    on(
        _Self="&str",
        message="found &str but we want a `Column`",
        label="not on a `Column`",
        note="you should be fruxolizing your thingamathings"
    ),
    message="default message"
)]
trait Column {
    type SqlType;
}

error[E0277]: found &str but we want a `Column`
  --> src/main.rs:50:26
   |
50 |     let predicate = name.eq("Sean");
   |                          ^^ not on a `Column`
   |
   = help: the trait `Column` is not implemented for `&str`
   = note: you should be fruxolizing your thingamathings
   = note: required because of the requirements on the impl of `Expression` for `&str`
   = note: required because of the requirements on the impl of `AsExpression<std::string::String>` for `&str`

sgrif · 2019-05-21T18:14:53Z

This issue is about the fact that `Column` is irrelevant, not useful to users when the missing trait is `AsExpression`, and there's no way to control this behavior as a library author.

…

On Tue, May 21, 2019 at 12:06 PM Esteban Kuber ***@***.***> wrote: @sgrif <https://github.com/sgrif> that's because you need to give the on_unimplemented to trait Column <https://play.rust-lang.org/?version=nightly&mode=debug&edition=2015&gist=f12573f27165332f884dfd1abafc11a2> : #[rustc_on_unimplemented( on( _Self="&str", message="found &str but we want a `Column`", label="not on a `Column`", note="you should be fruxolizing your thingamathings" ), message="default message" )] trait Column { type SqlType; } error[E0277]: found &str but we want a `Column` --> src/main.rs:50:26 | 50 | let predicate = name.eq("Sean"); | ^^ not on a `Column` | = help: the trait `Column` is not implemented for `&str` = note: you should be fruxolizing your thingamathings = note: required because of the requirements on the impl of `Expression` for `&str` = note: required because of the requirements on the impl of `AsExpression<std::string::String>` for `&str` — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#28894?email_source=notifications&email_token=AALVMKYC3WA66OPJP7SENATPWQ22DA5CNFSM4BRLSFGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODV4W4EA#issuecomment-494497296>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AALVMK5PICBHX6LQGOU3EDDPWQ22DANCNFSM4BRLSFGA> .

-- Thanks, Sean Griffin

estebank · 2019-05-21T18:54:48Z

Could you show us the "fixed" code for the example provided?

I believe that I understand where you're coming from, as this is a pretty bad limitation rustc has, but I also think that the subset of problems that are expected to happen (like this one) can receive specific error messages that point people in the right direction, even though they need to be added to the "wrong" trait. I'm not saying that this ticket should be closed, as rustc should behave in a more reasonable manner, but rather that diesel can improve its user friendliness today by judicious use of on_unimplemented for common problems.

sgrif · 2019-05-21T19:08:50Z

https://gist.github.com/sgrif/fb06eec01a38a8f7a8f0ff9c1f62be7d For what it's worth, #51992 is probably enough to address this

…

On Tue, May 21, 2019 at 12:55 PM Esteban Kuber ***@***.***> wrote: Could you show us the "fixed" code for the example provided? I believe that I understand where you're coming from, as this is a pretty bad limitation rustc has, but I also think that the subset of problems that are *expected* to happen (like this one) can receive specific error messages that point people in the right direction, even though they need to be added to the "wrong" trait. I'm not saying that this ticket should be closed, as rustc should behave in a more reasonable manner, but rather that diesel *can* improve its user friendliness *today* by judicious use of on_unimplemented for common problems. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#28894?email_source=notifications&email_token=AALVMK5BGJHDGW3DRJJNLITPWRATHA5CNFSM4BRLSFGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODV43CVQ#issuecomment-494514518>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AALVMK5INSIXFPW75NWHR5LPWRATHANCNFSM4BRLSFGA> .

-- Thanks, Sean Griffin

estebank · 2019-12-13T19:53:45Z

Current output, minor change to the span:

error[E0277]: the trait bound `&str: Column` is not satisfied
  --> file12.rs:39:29
   |
39 |     let predicate = name.eq("Sean");
   |                             ^^^^^^ the trait `Column` is not implemented for `&str`
   |
   = note: required because of the requirements on the impl of `Expression` for `&str`
   = note: required because of the requirements on the impl of `AsExpression<std::string::String>` for `&str`

estebank · 2021-04-21T01:48:45Z

Current output, more context for the requirements, but no other changes:

error[E0277]: the trait bound `&str: Column` is not satisfied
  --> src/main.rs:42:29
   |
42 |     let predicate = name.eq("Sean");
   |                             ^^^^^^ the trait `Column` is not implemented for `&str`
   |
note: required because of the requirements on the impl of `Expression` for `&str`
  --> src/main.rs:31:17
   |
31 | impl<C: Column> Expression for C {
   |                 ^^^^^^^^^^     ^
note: required because of the requirements on the impl of `AsExpression<String>` for `&str`
  --> src/main.rs:19:21
   |
19 | impl<E: Expression> AsExpression<E::SqlType> for E {
   |                     ^^^^^^^^^^^^^^^^^^^^^^^^     ^

Crazy idea: what if crates started adding comments next to places where spans are displayed, like 👀

error[E0277]: the trait bound `&str: Column` is not satisfied
  --> src/main.rs:43:29
   |
43 |     let predicate = name.eq("Sean");
   |                             ^^^^^^ the trait `Column` is not implemented for `&str`
   |
note: required because of the requirements on the impl of `Expression` for `&str`
  --> src/main.rs:29:1
   |
29 | Expression // Make sure that the type
   | ^^^^^^^^^^
30 | for        // of the value you're using
31 | C          // matches the table definition
   | ^
note: required because of the requirements on the impl of `AsExpression<String>` for `&str`
  --> src/main.rs:16:21
   |
16 | impl<E: Expression> AsExpression<E::SqlType> for E {
   |                     ^^^^^^^^^^^^^^^^^^^^^^^^     ^

estebank · 2023-03-15T03:07:46Z

Current output:

error[E0277]: the trait bound `&str: Column` is not satisfied
  --> src/main.rs:39:29
   |
39 |     let predicate = name.eq("Sean");
   |                          -- ^^^^^^ the trait `Column` is not implemented for `&str`
   |                          |
   |                          required by a bound introduced by this call
   |
   = help: the trait `Column` is implemented for `name`
note: required for `&str` to implement `Expression`
  --> src/main.rs:28:17
   |
28 | impl<C: Column> Expression for C {
   |         ------  ^^^^^^^^^^     ^
   |         |
   |         unsatisfied trait bound introduced here
note: required for `&str` to implement `AsExpression<String>`
  --> src/main.rs:16:21
   |
16 | impl<E: Expression> AsExpression<E::SqlType> for E {
   |         ----------  ^^^^^^^^^^^^^^^^^^^^^^^^     ^
   |         |
   |         unsatisfied trait bound introduced here
note: required by a bound in `Expression::eq`
  --> src/main.rs:4:14
   |
4  |     fn eq<T: AsExpression<Self::SqlType>>(self, other: T) {
   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Expression::eq`

estebank · 2024-11-18T05:10:14Z

Triage: no change.

jdm added the A-diagnostics Area: Messages for errors, warnings, and lints label Oct 7, 2015

sgrif mentioned this issue Nov 11, 2015

Tracking issue for on_unimplemented feature #29628

Closed

sgrif mentioned this issue Nov 29, 2015

Clarification on update diesel-rs/diesel#26

Closed

sgrif mentioned this issue Jan 8, 2016

Implement diesel Expression for more types diesel-rs/diesel#80

Closed

sgrif mentioned this issue Jan 30, 2016

Examine our compiler errors, and see if we can improve them diesel-rs/diesel#151

Open

sgrif mentioned this issue May 11, 2016

Error message is unclear when compiling diesel-rs/diesel#323

Closed

arielb1 closed this as completed May 11, 2016

arielb1 reopened this May 11, 2016

steveklabnik removed the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 9, 2017

Mark-Simulacrum mentioned this issue Jun 5, 2017

Misleading error message when there is a generic trait impl for all types implementing other trait #32704

Closed

Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 24, 2017

jturner314 mentioned this issue Nov 1, 2018

Misleading error message involving blanket implementations and associated types #55591

Closed

estebank added the F-on_unimplemented Error messages that can be tackled with `#[rustc_on_unimplemented]` label Aug 4, 2019

estebank added D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 13, 2019

estebank mentioned this issue Apr 21, 2021

Bad compiler error messages while using diesel #83383

Open

Jeremiah-Griffin mentioned this issue Jul 15, 2023

INTEGER PRIMARY KEY in SQLite diesel-rs/diesel#852

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rust tries to be too helpful with impl suggestions, sometimes being unhelpful #28894

Rust tries to be too helpful with impl suggestions, sometimes being unhelpful #28894

sgrif commented Oct 7, 2015

arielb1 commented Oct 9, 2015

sgrif commented Jan 8, 2016

sgrif commented Jan 30, 2016

jonas-schievink commented Jan 31, 2016

sgrif commented Jan 31, 2016

jonas-schievink commented Jan 31, 2016

sgrif commented May 11, 2016

arielb1 commented May 11, 2016

sgrif commented May 11, 2016

sgrif commented May 11, 2016

arielb1 commented May 11, 2016

arielb1 commented May 14, 2016

arielb1 commented May 14, 2016

eddyb commented May 15, 2016

arielb1 commented May 15, 2016

eddyb commented May 15, 2016

arielb1 commented May 16, 2016

eddyb commented May 16, 2016

behnam commented Aug 9, 2017

steveklabnik commented Mar 14, 2019

estebank commented May 21, 2019

sgrif commented May 21, 2019

estebank commented May 21, 2019

sgrif commented May 21, 2019 via email

estebank commented May 21, 2019

sgrif commented May 21, 2019 via email

estebank commented Dec 13, 2019

estebank commented Apr 21, 2021 •

edited

Loading

estebank commented Mar 15, 2023

estebank commented Nov 18, 2024

Rust tries to be too helpful with impl suggestions, sometimes being unhelpful #28894

Rust tries to be too helpful with impl suggestions, sometimes being unhelpful #28894

Comments

sgrif commented Oct 7, 2015

arielb1 commented Oct 9, 2015

sgrif commented Jan 8, 2016

sgrif commented Jan 30, 2016

jonas-schievink commented Jan 31, 2016

sgrif commented Jan 31, 2016

jonas-schievink commented Jan 31, 2016

sgrif commented May 11, 2016

arielb1 commented May 11, 2016

sgrif commented May 11, 2016

sgrif commented May 11, 2016

arielb1 commented May 11, 2016

arielb1 commented May 14, 2016

arielb1 commented May 14, 2016

eddyb commented May 15, 2016

arielb1 commented May 15, 2016

eddyb commented May 15, 2016

arielb1 commented May 16, 2016

eddyb commented May 16, 2016

behnam commented Aug 9, 2017

steveklabnik commented Mar 14, 2019

estebank commented May 21, 2019

sgrif commented May 21, 2019

estebank commented May 21, 2019

sgrif commented May 21, 2019 via email

estebank commented May 21, 2019

sgrif commented May 21, 2019 via email

estebank commented Dec 13, 2019

estebank commented Apr 21, 2021 • edited Loading

estebank commented Mar 15, 2023

estebank commented Nov 18, 2024

estebank commented Apr 21, 2021 •

edited

Loading