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

use the new "unconstrained type parameter" callbacks more often #40013

Closed
nikomatsakis opened this issue Feb 21, 2017 · 2 comments
Closed

use the new "unconstrained type parameter" callbacks more often #40013

nikomatsakis opened this issue Feb 21, 2017 · 2 comments
Labels
A-allocators Area: Custom and system allocators E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Feb 21, 2017

#39361 improved our error messages around uninferable types, but there is still room to do better. For one thing, as @arielb1 pointed out, we currently detect these errors in a kind of odd place (when a Sized obligation is not matchable). We could generalize this to any obligation that includes an unresolved inference variable, I suspect. Moreover, there are cases where we don't encounter the error about an unconstrained inference variable until writeback:

fn main() {
    let x = [];
}

Therefore, the error reporting for unknown types in writeback ought to use the new helpers introduced in #39361

cc @cengizio -- would you like to keep on working on this?

UPDATE: I was wrong to say this behavior applies any time we have an unresolved obligation. Sometimes it does, but not always.

@nikomatsakis nikomatsakis added A-allocators Area: Custom and system allocators E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Feb 21, 2017
@cengiz-io
Copy link
Contributor

cengiz-io commented Feb 21, 2017

Sure! Would love to help!

EDIT: Just started working on this

@cengiz-io
Copy link
Contributor

Well that wasn't as easy as I predicted 😝

But I think it's finally done.

Currently waiting for @nikomatsakis's approval

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. 🍻
@bors bors closed this as completed in 6383de1 Apr 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-allocators Area: Custom and system allocators E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. 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

2 participants