Add better error message when == operator is badly used by GuillaumeGomez · Pull Request #41559 · rust-lang/rust

GuillaumeGomez · 2017-04-26T16:42:53Z

Part of #40660.

With the following code:

fnfoo<T:PartialEq>(a:&T,b:T){
a == b;}fnmain(){foo(&1,1);}

It prints:

error[E0277]: the trait bound `&T: std::cmp::PartialEq<T>` is not satisfied
--> test.rs:2:5
|
2 | a == b;
| ^^^^^^ can't compare `&T` with `T`
|
= help: the trait `std::cmp::PartialEq<T>` is not implemented for `&T`
= help: consider adding a `where &T: std::cmp::PartialEq<T>` bound
error: aborting due to previous error

rust-highfive · 2017-04-26T16:43:10Z

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

estebank · 2017-04-26T19:33:03Z

@GuillaumeGomez you need to update the output of src/test/ui/mismatched_types/binops.stderr.

GuillaumeGomez · 2017-04-26T19:43:10Z

Ah thanks!

GuillaumeGomez · 2017-04-26T19:59:51Z

The output looks way nicer this way. :D (Updated.)

frewsxcv · 2017-04-30T20:10:53Z

This is just let trait_display = trait_ref.to_string();

GuillaumeGomez · 2017-05-01T09:00:06Z

Updated.

kennytm · 2017-05-01T09:34:39Z

I thought a better error message should suggest a == &b or *a == b, not adding a where clause? 😳

GuillaumeGomez · 2017-05-01T10:18:30Z

Well, I didn't add the where clause, it was already there.

kennytm · 2017-05-01T10:25:15Z

@GuillaumeGomez Ah I see.

estebank

I think some of @arielb1's comments in #40565 apply to this review too. I'm all for more explicit diagnostic output, but I came around to thinking that using #[rustc_on_unimplemented] for this is enough (at least for now).

estebank · 2017-05-02T01:18:28Z

This will not work with no_std (as pointed out before to me).

Oh good point!

estebank · 2017-05-02T01:19:06Z

The label and a note have the same text.

They don't?

Ah, didn't look more below. :-/

estebank · 2017-05-02T01:19:18Z

Same as above.

GuillaumeGomez · 2017-05-02T09:02:37Z

Updated.

Mark-Simulacrum · 2017-05-07T00:33:39Z

Label and note seem to have the same text still? @GuillaumeGomez Was that intentional?

GuillaumeGomez · 2017-05-07T16:23:43Z

No, I need to fix this.

GuillaumeGomez · 2017-05-07T18:04:25Z

Updated.

bors · 2017-05-08T14:47:31Z

☔ The latest upstream changes (presumably #41745) made this pull request unmergeable. Please resolve the merge conflicts.

arielb1 · 2017-05-09T09:11:52Z

@GuillaumeGomez

Any particular reason this isn't a rustc_on_unimplemented note? I think we should make rustc_on_unimplemented be the label (instead of the redundant "the trait X is not implemented for Y").

GuillaumeGomez · 2017-05-09T10:05:01Z

@arielb1: I thought about it. So as you prefer.

alexcrichton · 2017-05-11T22:30:35Z

I think @pnkfelix is on PTO right now, @estebank would you be willing to review?

estebank · 2017-05-12T00:53:06Z

@alexcrichton the difference in output at the moment is swapping the note and the label, but I have the same hangup as @arielb1 as I would rather avoid checking paths by string. That being said, @GuillaumeGomez I'd be down with setting rustc_on_unimplemented text as label always and the existing label as a note or, as I half jokingly proposed, expanding rustc_on_unimplemented to have a way to set wether the text should be rendered as a note or a label.

GuillaumeGomez · 2017-05-12T09:05:01Z

Ok so we definitely agree on this:

If rustc_on_unimplemented is available, it'll be the label.

GuillaumeGomez · 2017-05-15T21:51:40Z

Ok, updated. Let's see what the CI says.

estebank · 2017-05-15T22:10:05Z

Is &* necessary here?

Yep, I'm surprised about it but it seems that it fails otherwise because of the From<&str> not matching and auto-dereferencing a String.

you should be able to use the format!() directly since #41745

It's since my last rebase that I can't, so not sure...

estebank · 2017-05-16T00:34:05Z

@GuillaumeGomez the failing files are:

compile-fail/E0277-2.rs
compile-fail/E0277.rs
compile-fail/const-unsized.rs
compile-fail/impl-trait/auto-trait-leak.rs
compile-fail/on-unimplemented/multiple-impls.rs
compile-fail/on-unimplemented/on-impl.rs
compile-fail/on-unimplemented/on-trait.rs
compile-fail/on-unimplemented/slice-index.rs
compile-fail/trait-suggest-where-clause.rs

GuillaumeGomez · 2017-05-16T08:17:47Z

@estebank: Ah great, thanks for the list! :)

GuillaumeGomez · 2017-05-17T20:19:49Z

Updated.

estebank · 2017-05-17T20:32:46Z

+ | ^^^^^^^^^^^ no implementation for `{integer} + std::option::Option<{integer}>`
 |
- = note: no implementation for `{integer} + std::option::Option<{integer}>`
+ = help: the trait `std::ops::Add<std::option::Option<{integer}>>` is not implemented for `{integer}`


Do not prevent the merge of this PR, but reading this file I feel the full output of this error should be:
error[E0277]: the trait `std::ops::Add<std::option::Option<{integer}>>` is not implemented for `{integer}` --> $DIR/binops.rs:12:5 | 12 | 1 + Some(1); | ^^^^^^^^^^^ no implementation for `{integer} + std::option::Option<{integer}>`
instead of
error[E0277]: the trait bound `{integer}: std::ops::Add<std::option::Option<{integer}>>` is not satisfied --> $DIR/binops.rs:12:5 | 12 | 1 + Some(1); | ^^^^^^^^^^^ no implementation for `{integer} + std::option::Option<{integer}>` | = help: the trait `std::ops::Add<std::option::Option<{integer}>>` is not implemented for `{integer}`
Thoughts?

I appreciate having both in case the user isn't well aware of the Add and equivalent traits.

carols10cents · 2017-05-22T16:50:27Z

What's the current state of this PR please @GuillaumeGomez @estebank? I'm having trouble following :)

estebank · 2017-05-22T18:42:25Z

@carols10cents thanks for the ping

@GuillaumeGomez can you post the visual diff of one of the modified cfail tests, like this one against the current nightly? I want to make sure that we're not removing information. If the new output looks fine for that, I'll r+.

GuillaumeGomez · 2017-05-22T21:17:30Z

Sure. Here it is:

> ./build/x86_64-apple-darwin/stage1/bin/rustc src/test/compile-fail/on-unimplemented/slice-index.rs
error[E0277]: the trait bound `i32: std::slice::SliceIndex<[i32]>` is not satisfied
--> src/test/compile-fail/on-unimplemented/slice-index.rs:21:5
|
21 | x[1i32]; //~ ERROR E0277
| ^^^^^^^ slice indices are of type `usize` or ranges of `usize`
|
= help: the trait `std::slice::SliceIndex<[i32]>` is not implemented for `i32`
= note: required because of the requirements on the impl of `std::ops::Index<i32>` for `[i32]`
error[E0277]: the trait bound `std::ops::RangeTo<i32>: std::slice::SliceIndex<[i32]>` is not satisfied
--> src/test/compile-fail/on-unimplemented/slice-index.rs:24:5
|
24 | x[..1i32]; //~ ERROR E0277
| ^^^^^^^^^ slice indices are of type `usize` or ranges of `usize`
|
= help: the trait `std::slice::SliceIndex<[i32]>` is not implemented for `std::ops::RangeTo<i32>`
= note: required because of the requirements on the impl of `std::ops::Index<std::ops::RangeTo<i32>>` for `[i32]`
error: aborting due to 2 previous errors

And the old one:

> rustc --version
> rustc 1.19.0-nightly (14f30da61 2017-05-21)
imperio@/Users/imperio/rust/rust > rustc src/test/compile-fail/on-unimplemented/slice-index.rs
error[E0277]: the trait bound `i32: std::slice::SliceIndex<[i32]>` is not satisfied
--> src/test/compile-fail/on-unimplemented/slice-index.rs:21:5
|
21 | x[1i32]; //~ ERROR E0277
| ^^^^^^^ the trait `std::slice::SliceIndex<[i32]>` is not implemented for `i32`
|
= note: slice indices are of type `usize` or ranges of `usize`
= note: required because of the requirements on the impl of `std::ops::Index<i32>` for `[i32]`
error[E0277]: the trait bound `std::ops::RangeTo<i32>: std::slice::SliceIndex<[i32]>` is not satisfied
--> src/test/compile-fail/on-unimplemented/slice-index.rs:24:5
|
24 | x[..1i32]; //~ ERROR E0277
| ^^^^^^^^^ the trait `std::slice::SliceIndex<[i32]>` is not implemented for `std::ops::RangeTo<i32>`
|
= note: slice indices are of type `usize` or ranges of `usize`
= note: required because of the requirements on the impl of `std::ops::Index<std::ops::RangeTo<i32>>` for `[i32]`
error: aborting due to 2 previous errors

estebank · 2017-05-23T00:28:17Z

@bors r+

bors · 2017-05-23T00:28:18Z

📌 Commit 747287a has been approved by estebank

bors · 2017-05-23T00:37:03Z

⌛ Testing commit 747287a with merge 2e91391...

Add better error message when == operator is badly used Part of #40660. With the following code: ```rust fn foo<T: PartialEq>(a: &T, b: T) { a == b; } fn main() { foo(&1, 1); } ``` It prints: ``` error[E0277]: the trait bound `&T: std::cmp::PartialEq<T>` is not satisfied --> test.rs:2:5 | 2 | a == b; | ^^^^^^ can't compare `&T` with `T` | = help: the trait `std::cmp::PartialEq<T>` is not implemented for `&T` = help: consider adding a `where &T: std::cmp::PartialEq<T>` bound error: aborting due to previous error ```

bors · 2017-05-23T03:26:18Z

☀️ Test successful - status-appveyor, status-travis
Approved by: estebank
Pushing 2e91391 to master...

rust-highfive assigned pnkfelix Apr 26, 2017

GuillaumeGomez force-pushed the partial-eq-msg branch from 416e528 to be84188Compare April 26, 2017 19:59

alexcrichton added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 27, 2017

frewsxcv reviewed Apr 30, 2017
View reviewed changes

Comment threadsrc/librustc/traits/error_reporting.rs Outdated
frewsxcvApr 30, 2017
Copy link
Copy Markdown
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just let trait_display = trait_ref.to_string();

GuillaumeGomez force-pushed the partial-eq-msg branch from be84188 to 3f02379Compare May 1, 2017 08:59

estebank reviewed May 2, 2017
View reviewed changes

GuillaumeGomez force-pushed the partial-eq-msg branch from 3f02379 to a908964Compare May 2, 2017 09:02

Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 7, 2017

GuillaumeGomez force-pushed the partial-eq-msg branch from a908964 to 88082daCompare May 7, 2017 18:04

carols10cents added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 8, 2017

GuillaumeGomez force-pushed the partial-eq-msg branch from 88082da to 7bb85e9Compare May 15, 2017 21:51

estebank reviewed May 15, 2017
View reviewed changes

Add better error message when == operator is badly used
747287a

GuillaumeGomez force-pushed the partial-eq-msg branch from 7bb85e9 to 747287aCompare May 17, 2017 20:18

estebank reviewed May 17, 2017
View reviewed changes

bors merged commit 747287a into rust-lang:masterMay 23, 2017

bors mentioned this pull request May 23, 2017
Suppress trait errors that are implied by other errors #41840
Merged

GuillaumeGomez deleted the partial-eq-msg branch May 23, 2017 07:53

Uh oh!

Uh oh!

Conversation

GuillaumeGomez commented Apr 26, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Apr 26, 2017

Uh oh!

estebank commented Apr 26, 2017

Uh oh!

GuillaumeGomez commented Apr 26, 2017

Uh oh!

GuillaumeGomez commented Apr 26, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented May 1, 2017

Uh oh!

kennytm commented May 1, 2017

Uh oh!

GuillaumeGomez commented May 1, 2017

Uh oh!

kennytm commented May 1, 2017

Uh oh!

estebank left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented May 2, 2017

Uh oh!

Mark-Simulacrum commented May 7, 2017

Uh oh!

GuillaumeGomez commented May 7, 2017

Uh oh!

GuillaumeGomez commented May 7, 2017

Uh oh!

bors commented May 8, 2017

Uh oh!

arielb1 commented May 9, 2017

Uh oh!

GuillaumeGomez commented May 9, 2017

Uh oh!

alexcrichton commented May 11, 2017

Uh oh!

estebank commented May 12, 2017

Uh oh!

GuillaumeGomez commented May 12, 2017

Uh oh!

GuillaumeGomez commented May 15, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

estebank commented May 16, 2017

Uh oh!

GuillaumeGomez commented May 16, 2017

Uh oh!

GuillaumeGomez commented May 17, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

GuillaumeGomez commented Apr 26, 2017•
edited
Loading