Do not use desugared ident when suggesting adding a type #52418

estebank · 2018-07-16T05:31:54Z

nikomatsakis

While this does seem like an improvement, it doesn't feel like a fix to me. But I'm trying to think what would be the better fix -- it's probably at least somewhat involved.

I imagine we could do this:

given some unresolved type variable ?X
- perhaps just those that are found in a for loop pattern
we look for a pending trait obligation where ?X is an "output", so something like ?Y: IntoIterator<Item = ?X> in this case, and find an unresolved type variable in the inputs (here, ?Y)
and then we try to find a good suggestion for ?Y

nikomatsakis · 2018-07-16T11:35:37Z

src/test/ui/issue-51116.stderr

+  --> $DIR/issue-51116.rs:16:13
+   |
+LL |         for tile in row {
+   |             ---- consider giving this a type


Can you give tile a type? I don't think you can. It seems like it'd be better to put the suggestion on tiles...

While I was in the shower thinking about PR this I realized that no, you can't. I'm tempted at completely removing that label for a partial "fix" and call it a day until I have time to explore the suggestion above.

nikomatsakis · 2018-07-16T11:36:17Z

src/test/ui/issue-51116.stderr

+   |             ---- consider giving this a type
+LL |             //~^ NOTE consider giving this a type
+LL |             *tile = 0;
+   |             ^^^^^ cannot infer type for `_`


Cannot infer type for _ is not particularly clear I fear :(

This less than ideal text appears more than I would like... I'm not sure if there's already a ticket for it. I'll create one.

nikomatsakis · 2018-07-16T13:04:41Z

Looking at the code, it seems like implementing my suggestion would be not entirely trivial. I wonder if we could do something simpler to start -- e.g., if we notice that we are part of a for loop desugaring, we could walk up the HIR tree to find the for loop, and then highlight the collection expression that we are iterating over.

Then we might say something like:

error[E0282]: type annotations needed
 --> src/main.rs:5:13
  |
4 |         for tile in row {
  |                     --- the element type for this iterator is not specified
5 |             *tile = 0;
  |             ^^^^^ cannot infer type for `_`

not... great I guess. But easy-ish and better than where we are.

nikomatsakis · 2018-07-16T13:04:56Z

Maybe worth doing the better thing though

estebank · 2018-07-16T15:28:30Z

Your last suggestion might be able to be implemented by giving __next the span of the iterator (to avoid having to traverse the hir), but that change might break or make less clear other diagnostics. I would have to try it out.

estebank · 2018-07-16T21:07:40Z

@nikomatsakis changed the output to #52418 (comment). I feel this is enough of an improvement for this PR, but we can leave the original ticket open.

nikomatsakis · 2018-07-17T11:16:30Z

@bors r+

@estebank much nicer!

estebank · 2018-07-17T18:55:15Z

@nikomatsakis Just took a look at the existing test output that generates the cannot infer type for '_' label:

~/personal/rust (rust:master 0☲)$ more src/test/ui/issue-12187-1.stderr
error[E0282]: type annotations needed
  --> $DIR/issue-12187-1.rs:16:10
   |
LL |     let &v = new();
   |         -^
   |         ||
   |         |cannot infer type for `_`
   |         consider giving the pattern a type

error: aborting due to previous error

For more information about this error, try `rustc --explain E0282`.
~/personal/rust (rust:master 0☲)$ more src/test/ui/issue-23041.stderr
error[E0282]: type annotations needed
  --> $DIR/issue-23041.rs:16:22
   |
LL |     b.downcast_ref::<fn(_)->_>(); //~ ERROR E0282
   |                      ^^^^^^^^ cannot infer type for `_`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0282`.
~/personal/rust (rust:master 0☲)$ more src/test/ui/issue-12187-2.stderr
error[E0282]: type annotations needed
  --> $DIR/issue-12187-2.rs:16:10
   |
LL |     let &v = new();
   |         -^
   |         ||
   |         |cannot infer type for `_`
   |         consider giving the pattern a type

error: aborting due to previous error

For more information about this error, try `rustc --explain E0282`.
~/personal/rust (rust:master 0☲)$ more src/test/ui/span/issue-42234-unknown-receiver-type.stderr
error[E0282]: type annotations needed
  --> $DIR/issue-42234-unknown-receiver-type.rs:17:5
   |
LL |     let x: Option<_> = None;
   |         - consider giving `x` a type
LL |     x.unwrap().method_that_could_exist_on_some_type();
   |     ^^^^^^^^^^ cannot infer type for `T`
   |
   = note: type must be known at this point

error[E0282]: type annotations needed
  --> $DIR/issue-42234-unknown-receiver-type.rs:22:5
   |
LL | /     data.iter() //~ ERROR 22:5: 23:20: type annotations needed
LL | |         .sum::<_>()
   | |___________________^ cannot infer type for `_`
   |
   = note: type must be known at this point

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0282`.
~/personal/rust (rust:master 0☲)$ more src/test/ui/span/method-and-field-eager-resolution.stderr
error[E0282]: type annotations needed
  --> $DIR/method-and-field-eager-resolution.rs:15:5
   |
LL |     let mut x = Default::default();
   |         ----- consider giving `x` a type
LL |     x.0;
   |     ^ cannot infer type for `_`
   |
   = note: type must be known at this point

error[E0282]: type annotations needed
  --> $DIR/method-and-field-eager-resolution.rs:22:5
   |
LL |     let mut x = Default::default();
   |         ----- consider giving `x` a type
LL |     x[0];
   |     ^ cannot infer type for `_`
   |
   = note: type must be known at this point

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0282`.
~/personal/rust (rust:master 0☲)$ more src/test/ui/error-codes/E0282.stderr
error[E0282]: type annotations needed
  --> $DIR/E0282.rs:12:9
   |
LL |     let x = "hello".chars().rev().collect(); //~ ERROR E0282
   |         ^
   |         |
   |         cannot infer type for `_`
   |         consider giving `x` a type

error: aborting due to previous error

For more information about this error, try `rustc --explain E0282`.
~/personal/rust (rust:master 0☲)$ more src/test/ui/type-check/cannot_infer_local_or_array.stderr
error[E0282]: type annotations needed
  --> $DIR/cannot_infer_local_or_array.rs:12:13
   |
LL |     let x = []; //~ ERROR type annotations needed
   |         -   ^^ cannot infer type for `_`
   |         |
   |         consider giving `x` a type

error: aborting due to previous error

For more information about this error, try `rustc --explain E0282`.
~/personal/rust (rust:master 0☲)$ more src/test/ui/issue-23041.stderr
error[E0282]: type annotations needed
  --> $DIR/issue-23041.rs:16:22
   |
LL |     b.downcast_ref::<fn(_)->_>(); //~ ERROR E0282
   |                      ^^^^^^^^ cannot infer type for `_`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0282`.
~/personal/rust (rust:master 0☲)$ more src/test/ui/issue-7813.stderr
error[E0282]: type annotations needed
  --> $DIR/issue-7813.rs:12:13
   |
LL |     let v = &[]; //~ ERROR type annotations needed
   |         -   ^^^ cannot infer type for `_`
   |         |
   |         consider giving `v` a type

error: aborting due to previous error

For more information about this error, try `rustc --explain E0282`.
~/personal/rust (rust:master 0☲)$ more src/test/ui/issue-12187-1.stderr
error[E0282]: type annotations needed
  --> $DIR/issue-12187-1.rs:16:10
   |
LL |     let &v = new();
   |         -^
   |         ||
   |         |cannot infer type for `_`
   |         consider giving the pattern a type

error: aborting due to previous error

For more information about this error, try `rustc --explain E0282`.

I can't really think of an improvement at this time for them :-/

nikomatsakis · 2018-07-18T04:33:02Z

It doesn't seem to me that the "for _" is adding anything at all in these examples. I cannot even tell what it is trying to name, to be honest... maybe we should just remove it?

For example, isn't

error[E0282]: type annotations needed
  --> $DIR/issue-12187-1.rs:16:10
   |
LL |     let &v = new();
   |         -^
   |         ||
   |         |cannot infer type
   |         consider giving the pattern a type

ever so slightly better than

error[E0282]: type annotations needed
  --> $DIR/issue-12187-1.rs:16:10
   |
LL |     let &v = new();
   |         -^
   |         ||
   |         |cannot infer type for `_`
   |         consider giving the pattern a type

?

nikomatsakis · 2018-07-18T11:45:06Z

@estebank

[00:43:39] diff of stderr:
[00:43:39] 
[00:43:39] 2   --> $DIR/issue-20261.rs:14:11
[00:43:39] 3    |
[00:43:39] 4 LL |     for (ref i,) in [].iter() {
[00:43:39] -    |         -------- consider giving `__next` a type
[00:43:39] +    |                     --------- the element type for this iterator is not specified
[00:43:39] 6 LL |         i.clone();
[00:43:39] 7    |           ^^^^^ cannot infer type for `_`
[00:43:39]

nikomatsakis · 2018-07-18T11:45:38Z

r=me

estebank · 2018-07-18T18:24:01Z

@bors r=nikomatsakis rollup

bors · 2018-07-18T18:24:02Z

📌 Commit 2c5b60d has been approved by nikomatsakis

…sakis Do not use desugared ident when suggesting adding a type Re rust-lang#51116.

@ghost

Rollup of 13 pull requests Successful merges: - #51628 (use checked write in `LineWriter` example) - #52116 (Handle array manually in str case conversion methods) - #52218 (Amend option.take examples) - #52418 (Do not use desugared ident when suggesting adding a type) - #52439 (Revert some changes from #51917 to fix custom libdir) - #52455 (Fix doc comment: use `?` instead of `.unwrap()`) - #52458 (rustc: Fix a suggestion for the `proc_macro` feature) - #52464 (Allow clippy to be installed with make install) - #52472 (rustc: Enable `use_extern_macros` in 2018 edition) - #52477 (Clarify short-circuiting behvaior of Iterator::zip.) - #52480 (Cleanup #24958) - #52487 (Don't build twice the sanitizers on Linux) - #52510 (rustdoc: remove FIXME about macro redirects) Failed merges: r? @ghost

rust-highfive assigned nikomatsakis Jul 16, 2018