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

error message for uninferrable type #38812

Closed
dherman opened this issue Jan 4, 2017 · 7 comments
Closed

error message for uninferrable type #38812

dherman opened this issue Jan 4, 2017 · 7 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints

Comments

@dherman
Copy link
Contributor

dherman commented Jan 4, 2017

The error message when an unused variable leads to an uninferrable type is confusing:

fn main() {
    let v = vec![];
}
// rustc 1.14.0 (e8a012324 2016-12-16)
// error[E0282]: unable to infer enough type information about `_`
//  --> <anon>:2:13
//   |
// 2 |     let v = vec![];
//   |             ^^^^^^ cannot infer type for `_`
//   |
//   = note: type annotations or generic parameter binding required
//   = note: this error originates in a macro outside of the current crate

@nikomatsakis pointed me to #36554. I'm not sure how much that issue was aiming to fix.

If I'm not mistaken, most (all?) of these kinds of situations occur when an expression is flowing into an unannotated local variable binding. It seems like a better error message would highlight the binding as well as the vec![] expression (or whatever the uninferrable expression is), and suggest that the variable should either be used elsewhere in the function or annotated with a type.

/cc @jonathandturner

@steveklabnik steveklabnik added the A-diagnostics Area: Messages for errors, warnings, and lints label Jan 4, 2017
@estebank
Copy link
Contributor

estebank commented Jan 6, 2017

#36554 is aimed at the following case:

fn foo<T>(bar: T) {}

fn main() {
  let x = vec![];
  foo(x);
}
error[E0282]: unable to infer enough type information about `T`
 --> file.rs:4:13
  |
4 |     let x = vec![];
  |             ^^^^^^ cannot infer type for `T`
  |
  = note: type annotations or generic parameter binding required

The case in this issue is slightly different.

@nikomatsakis
Copy link
Contributor

I think it's basically the same underlying problem: when we are unable to infer a type, we should be as helpful as we can in telling you (a) what type we are failing to infer and (b) how can you specify it yourself. #36554 really just took a baby step by providing you at least the name of the type parameter. But it's true it was targeting the case of calling a fn.

@nikomatsakis
Copy link
Contributor

So I have a plan for how to solve this. I have been talking to @cengizio who is going to take a stab at implementing it, but I wanted to write out the outlines here for future reference. Roughly speaking, the idea is that when we have an error that we need more type info about some type variable ?T, we will go search the HIR of the current function to try to find places we might put annotations that would have told us what ?T is. The first thing along these lines is to search all let definitions and see if their type includes ?T; if so, they are a candidate where an annotation might have been useful.

So the idea is that before we print the current message, we will try each of these heuristics (for now only local variables, but hopefully we'll grow more later). If we find a good suggestion for where to annotate, great, we can give a better error. Otherwise we fallback to the current one.

This current error is reported in the function need_type_info() in rustc::traits::error_reporting. This is a method defined on an InferCtxt, which has I think all the context we are going to need. But the first thing is that we need to find the current fn body -- luckily we can get that. The callers of need_type_info() have an ObligationCause which happens to have a body_id. This is the node-id of the innermost enclosing fn body, I think. Not perfect but good enough for a start. So we have to modify those callers to thread that id into need_type_info.

need_type_info() can then do a lookup using self.tcx.map.find(), which will return a librustc::hir::map::Node data structure. We expect to see a NodeBlock, I think, or maybe a NodeItem. These are the HIR nodes for the fn body. We can then setup a visitor (see librustc::hir::intravisit for docs on how to do that) and look for let statements (i.e., override visit_local, I think). For each of those, we can lookup its type in self.tables.borrow().node_types -- beware that some of them may not yet have types, if type inference is not complete.

Each type will potentially contain other inferences and so forth, so once we fetch the type for a let (let's call that let_ty) we have to "refresh" it with the most recent information by doing something like let let_ty = self.resolve_type_variables_if_possible(&let_ty);. This will give us the up-to-date let_ty. So, in our example, the type of x might wind up being Vec<?T> (where ?T is the variable we are looking for).

To see whether this type includes ?T, we can use ty.walk() which returns an iterator of all the types references (so here it would visit Vec<?T>, then ?T). We will be looking for the type ty that was passed in to need_type_info(), I imagine.

If we find it, then we can give an error pointing at the local variable, saying something like
"more type annotations needed", with a suggestion toward annotating the type of that variable. I'm not sure of the best wording, we can figure that out when we get that far I guess!

@cengiz-io
Copy link
Contributor

Hello @nikomatsakis!

Thanks again for mentoring me on rust internals 😉

I started working on this.

@cengiz-io
Copy link
Contributor

My PR seems to be failing only for llvm-3.7 on Travis.

Investigating now

frewsxcv added a commit to frewsxcv/rust that referenced this issue Feb 7, 2017
Improve error message for uninferrable types rust-lang#38812

Hello,

I tried to improve the error message for uninferrable types. The error code is `E0282`.

```rust

error[E0282]: type annotations needed
 --> /home/cengizIO/issue38812.rs:2:11
  |
2 |   let x = vec![];
  |       -   ^^^^^^ cannot infer type for `T`
  |       |
  |       consider giving `x` a type
  |
  = note: this error originates in a macro outside of the current crate
```

and

```rust

error[E0282]: type annotations needed
 --> /home/cengizIO/issue38812.rs:2:15
  |
2 |   let (x,) = (vec![],);
  |       ----    ^^^^^^ cannot infer type for `T`
  |       |
  |       consider giving a type to pattern
  |
  = note: this error originates in a macro outside of the current crate
```

Rust compiler now tries to find uninferred `local`s with type `_` and adds them into the error message.

I'm probably wrong on wording that I used. Please feel free to suggest better alternatives.

Thanks @nikomatsakis for mentoring 🍺

Any comments/feedback is more than welcome!

Thank you
frewsxcv added a commit to frewsxcv/rust that referenced this issue Feb 7, 2017
Improve error message for uninferrable types rust-lang#38812

Hello,

I tried to improve the error message for uninferrable types. The error code is `E0282`.

```rust

error[E0282]: type annotations needed
 --> /home/cengizIO/issue38812.rs:2:11
  |
2 |   let x = vec![];
  |       -   ^^^^^^ cannot infer type for `T`
  |       |
  |       consider giving `x` a type
  |
  = note: this error originates in a macro outside of the current crate
```

and

```rust

error[E0282]: type annotations needed
 --> /home/cengizIO/issue38812.rs:2:15
  |
2 |   let (x,) = (vec![],);
  |       ----    ^^^^^^ cannot infer type for `T`
  |       |
  |       consider giving a type to pattern
  |
  = note: this error originates in a macro outside of the current crate
```

Rust compiler now tries to find uninferred `local`s with type `_` and adds them into the error message.

I'm probably wrong on wording that I used. Please feel free to suggest better alternatives.

Thanks @nikomatsakis for mentoring 🍺

Any comments/feedback is more than welcome!

Thank you
frewsxcv added a commit to frewsxcv/rust that referenced this issue Feb 8, 2017
Improve error message for uninferrable types rust-lang#38812

Hello,

I tried to improve the error message for uninferrable types. The error code is `E0282`.

```rust

error[E0282]: type annotations needed
 --> /home/cengizIO/issue38812.rs:2:11
  |
2 |   let x = vec![];
  |       -   ^^^^^^ cannot infer type for `T`
  |       |
  |       consider giving `x` a type
  |
  = note: this error originates in a macro outside of the current crate
```

and

```rust

error[E0282]: type annotations needed
 --> /home/cengizIO/issue38812.rs:2:15
  |
2 |   let (x,) = (vec![],);
  |       ----    ^^^^^^ cannot infer type for `T`
  |       |
  |       consider giving a type to pattern
  |
  = note: this error originates in a macro outside of the current crate
```

Rust compiler now tries to find uninferred `local`s with type `_` and adds them into the error message.

I'm probably wrong on wording that I used. Please feel free to suggest better alternatives.

Thanks @nikomatsakis for mentoring 🍺

Any comments/feedback is more than welcome!

Thank you
frewsxcv added a commit to frewsxcv/rust that referenced this issue Feb 8, 2017
Improve error message for uninferrable types rust-lang#38812

Hello,

I tried to improve the error message for uninferrable types. The error code is `E0282`.

```rust

error[E0282]: type annotations needed
 --> /home/cengizIO/issue38812.rs:2:11
  |
2 |   let x = vec![];
  |       -   ^^^^^^ cannot infer type for `T`
  |       |
  |       consider giving `x` a type
  |
  = note: this error originates in a macro outside of the current crate
```

and

```rust

error[E0282]: type annotations needed
 --> /home/cengizIO/issue38812.rs:2:15
  |
2 |   let (x,) = (vec![],);
  |       ----    ^^^^^^ cannot infer type for `T`
  |       |
  |       consider giving a type to pattern
  |
  = note: this error originates in a macro outside of the current crate
```

Rust compiler now tries to find uninferred `local`s with type `_` and adds them into the error message.

I'm probably wrong on wording that I used. Please feel free to suggest better alternatives.

Thanks @nikomatsakis for mentoring 🍺

Any comments/feedback is more than welcome!

Thank you
@cengiz-io
Copy link
Contributor

I think this can be closed since we landed this with #39361

@nikomatsakis
Copy link
Contributor

I agree this example is fixed. I opened some new issues for other cases (e.g., #40013, #40014, and #40015)

alexcrichton added a commit to alexcrichton/rust that referenced this issue Mar 10, 2017
fix rust-lang#40294 obligation cause.body_id is not always a NodeExpr

Hello!

This fixes rust-lang#40294 and moves tests related to rust-lang#38812 to a much more sensible directory.

Thanks to @nikomatsakis and @eddyb
alexcrichton added a commit to alexcrichton/rust that referenced this issue Mar 10, 2017
fix rust-lang#40294 obligation cause.body_id is not always a NodeExpr

Hello!

This fixes rust-lang#40294 and moves tests related to rust-lang#38812 to a much more sensible directory.

Thanks to @nikomatsakis and @eddyb
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 10, 2017
fix rust-lang#40294 obligation cause.body_id is not always a NodeExpr

Hello!

This fixes rust-lang#40294 and moves tests related to rust-lang#38812 to a much more sensible directory.

Thanks to @nikomatsakis and @eddyb
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 10, 2017
fix rust-lang#40294 obligation cause.body_id is not always a NodeExpr

Hello!

This fixes rust-lang#40294 and moves tests related to rust-lang#38812 to a much more sensible directory.

Thanks to @nikomatsakis and @eddyb
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 10, 2017
fix rust-lang#40294 obligation cause.body_id is not always a NodeExpr

Hello!

This fixes rust-lang#40294 and moves tests related to rust-lang#38812 to a much more sensible directory.

Thanks to @nikomatsakis and @eddyb
alexcrichton added a commit to alexcrichton/rust that referenced this issue Mar 11, 2017
fix rust-lang#40294 obligation cause.body_id is not always a NodeExpr

Hello!

This fixes rust-lang#40294 and moves tests related to rust-lang#38812 to a much more sensible directory.

Thanks to @nikomatsakis and @eddyb
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 11, 2017
fix rust-lang#40294 obligation cause.body_id is not always a NodeExpr

Hello!

This fixes rust-lang#40294 and moves tests related to rust-lang#38812 to a much more sensible directory.

Thanks to @nikomatsakis and @eddyb
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 11, 2017
fix rust-lang#40294 obligation cause.body_id is not always a NodeExpr

Hello!

This fixes rust-lang#40294 and moves tests related to rust-lang#38812 to a much more sensible directory.

Thanks to @nikomatsakis and @eddyb
frewsxcv added a commit to frewsxcv/rust that referenced this issue Apr 18, 2017
Move E0101 and E0102 logic into new E0282 mechanism rust-lang#40013

Hello there!

Previously, me and @nikomatsakis worked on error messages of uninferred locals. (rust-lang#38812)

This aims to build up on that by moving certain type checks from `writeback`.

With this, `E0101` and `E0102` errors are getting obsoleted and no longer thrown.

They're replaced with customized versions of `E0282`s instead.

```rust
error[E0282]: type annotations needed
 --> test.rs:2:14
  |
2 |     let x = |_| {};
  |              ^ consider giving this closure parameter a type

error: aborting due to previous error
```

```rust
error[E0282]: type annotations needed
 --> test.rs:2:9
  |
2 |     let x = [];
  |         ^
  |         |
  |         consider giving `x` a type
  |         cannot infer type for `[_; 0]`

error: aborting due to previous error
```

- I think we need to change our way of type name resolving in relevant places, because that `[_; 0]` looks horrible IMHO.
- I'm not terribly happy with the note ordering of errors. So please do point to code that might help me accomplish this.

Tests of `E0101` and `E0102` are getting converted from `compile-fail` to `ui` tests.

Please help me with documentation update. There are some confusing places that needed an update but I'm not sure if I did the right ones.

Please do comment on messages, layouts and other details.

Huge thanks goes to @nikomatsakis for being a patient and humble mentor along this long journey. 🍻
frewsxcv added a commit to frewsxcv/rust that referenced this issue Apr 18, 2017
Move E0101 and E0102 logic into new E0282 mechanism rust-lang#40013

Hello there!

Previously, me and @nikomatsakis worked on error messages of uninferred locals. (rust-lang#38812)

This aims to build up on that by moving certain type checks from `writeback`.

With this, `E0101` and `E0102` errors are getting obsoleted and no longer thrown.

They're replaced with customized versions of `E0282`s instead.

```rust
error[E0282]: type annotations needed
 --> test.rs:2:14
  |
2 |     let x = |_| {};
  |              ^ consider giving this closure parameter a type

error: aborting due to previous error
```

```rust
error[E0282]: type annotations needed
 --> test.rs:2:9
  |
2 |     let x = [];
  |         ^
  |         |
  |         consider giving `x` a type
  |         cannot infer type for `[_; 0]`

error: aborting due to previous error
```

- I think we need to change our way of type name resolving in relevant places, because that `[_; 0]` looks horrible IMHO.
- I'm not terribly happy with the note ordering of errors. So please do point to code that might help me accomplish this.

Tests of `E0101` and `E0102` are getting converted from `compile-fail` to `ui` tests.

Please help me with documentation update. There are some confusing places that needed an update but I'm not sure if I did the right ones.

Please do comment on messages, layouts and other details.

Huge thanks goes to @nikomatsakis for being a patient and humble mentor along this long journey. 🍻
frewsxcv added a commit to frewsxcv/rust that referenced this issue Apr 18, 2017
Move E0101 and E0102 logic into new E0282 mechanism rust-lang#40013

Hello there!

Previously, me and @nikomatsakis worked on error messages of uninferred locals. (rust-lang#38812)

This aims to build up on that by moving certain type checks from `writeback`.

With this, `E0101` and `E0102` errors are getting obsoleted and no longer thrown.

They're replaced with customized versions of `E0282`s instead.

```rust
error[E0282]: type annotations needed
 --> test.rs:2:14
  |
2 |     let x = |_| {};
  |              ^ consider giving this closure parameter a type

error: aborting due to previous error
```

```rust
error[E0282]: type annotations needed
 --> test.rs:2:9
  |
2 |     let x = [];
  |         ^
  |         |
  |         consider giving `x` a type
  |         cannot infer type for `[_; 0]`

error: aborting due to previous error
```

- I think we need to change our way of type name resolving in relevant places, because that `[_; 0]` looks horrible IMHO.
- I'm not terribly happy with the note ordering of errors. So please do point to code that might help me accomplish this.

Tests of `E0101` and `E0102` are getting converted from `compile-fail` to `ui` tests.

Please help me with documentation update. There are some confusing places that needed an update but I'm not sure if I did the right ones.

Please do comment on messages, layouts and other details.

Huge thanks goes to @nikomatsakis for being a patient and humble mentor along this long journey. 🍻
frewsxcv added a commit to frewsxcv/rust that referenced this issue Apr 18, 2017
Move E0101 and E0102 logic into new E0282 mechanism rust-lang#40013

Hello there!

Previously, me and @nikomatsakis worked on error messages of uninferred locals. (rust-lang#38812)

This aims to build up on that by moving certain type checks from `writeback`.

With this, `E0101` and `E0102` errors are getting obsoleted and no longer thrown.

They're replaced with customized versions of `E0282`s instead.

```rust
error[E0282]: type annotations needed
 --> test.rs:2:14
  |
2 |     let x = |_| {};
  |              ^ consider giving this closure parameter a type

error: aborting due to previous error
```

```rust
error[E0282]: type annotations needed
 --> test.rs:2:9
  |
2 |     let x = [];
  |         ^
  |         |
  |         consider giving `x` a type
  |         cannot infer type for `[_; 0]`

error: aborting due to previous error
```

- I think we need to change our way of type name resolving in relevant places, because that `[_; 0]` looks horrible IMHO.
- I'm not terribly happy with the note ordering of errors. So please do point to code that might help me accomplish this.

Tests of `E0101` and `E0102` are getting converted from `compile-fail` to `ui` tests.

Please help me with documentation update. There are some confusing places that needed an update but I'm not sure if I did the right ones.

Please do comment on messages, layouts and other details.

Huge thanks goes to @nikomatsakis for being a patient and humble mentor along this long journey. 🍻
bors added a commit that referenced this issue Apr 19, 2017
Move E0101 and E0102 logic into new E0282 mechanism #40013

Hello there!

## What's this?
Previously, me and @nikomatsakis worked on error messages of uninferred locals. (#38812)

This aims to build up on that by moving certain type checks from `writeback`.

With this, `E0101` and `E0102` errors are getting obsoleted and no longer thrown.

They're replaced with customized versions of `E0282`s instead.

## Sample Error Messages

#### `E0101` is getting converted into:
```rust
error[E0282]: type annotations needed
 --> test.rs:2:14
  |
2 |     let x = |_| {};
  |              ^ consider giving this closure parameter a type

error: aborting due to previous error
```

#### `E0102` is getting converted into:
```rust
error[E0282]: type annotations needed
 --> test.rs:2:9
  |
2 |     let x = [];
  |         ^
  |         |
  |         consider giving `x` a type
  |         cannot infer type for `[_; 0]`

error: aborting due to previous error
```

## Annoyances
- I think we need to change our way of type name resolving in relevant places, because that `[_; 0]` looks horrible IMHO.
- I'm not terribly happy with the note ordering of errors. So please do point to code that might help me accomplish this.

## Tests
Tests of `E0101` and `E0102` are getting converted from `compile-fail` to `ui` tests.

## Documentation
Please help me with documentation update. There are some confusing places that needed an update but I'm not sure if I did the right ones.

Please do comment on messages, layouts and other details.

## Appreciation
Huge thanks goes to @nikomatsakis for being a patient and humble mentor along this long journey. 🍻
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
Projects
None yet
Development

No branches or pull requests

5 participants