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

Remove note: prefix from type mismatch errors #38902

Closed
wants to merge 7 commits into from

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Jan 7, 2017

Given file

fn main() {
    let x: usize = "";
}

provide the following output

error[E0308]: mismatched types
 --> file.rs:2:20
  |
2 |     let x: usize = "";
  |                    ^^ expected usize, found reference
  |
  = expected type `usize`
  =    found type `&'static str`
  = help: here are some functions which might fulfill your needs:
 - .len()

Fix #38901.

Given file

```
fn main() {
    let x: usize = "";
}
```

provide the following output

```rust
error[E0308]: mismatched types
 --> file.rs:2:20
  |
2 |     let x: usize = "";
  |                    ^^ expected usize, found reference
  |
  = expected type `usize`
  =    found type `&'static str`
  = help: here are some functions which might fulfill your needs:
 - .len()
```
@rust-highfive
Copy link
Collaborator

r? @eddyb

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

= note: expected type `()`
= note: found type `std::result::Result<bool, std::io::Error>`
= expected type `()`
= found type `std::result::Result<bool, std::io::Error>`
= help: here are some functions which might fulfill your needs:
- .unwrap()
Copy link
Contributor

@durka durka Jan 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider indenting these suggestions? (out of scope for this PR maybe)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that, I'll prepare a separate PR for it.

Copy link
Contributor Author

@estebank estebank Jan 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #38916 for this.

@eddyb
Copy link
Member

eddyb commented Jan 7, 2017

r? @jonathandturner

@rust-highfive rust-highfive assigned sophiajt and unassigned eddyb Jan 7, 2017
@@ -527,6 +529,7 @@ impl Level {
Fatal | PhaseFatal | Error => "error",
Warning => "warning",
Note => "note",
TopLevel => "note",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh this is unfortunate, external tools parsing rustc JSON output for example wouldn't be able to tell this apart. Which means unavoidable inconsistency between rustc output and external tools.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is. I went with this option to avoid breaking existing tooling. If we wan't to make top level comments a general thing, we can give this a differentiated &str identifier. If this will only be used for expected/found messages, then I'd rename the enum variant to something else and the identifier can then be "expected" and "found". I think that'd be beneficial for tools.

On errors like the following:

```
error[E0308]: mismatched types
 --> file.rs:2:20
  |
2 |     let x: usize = "";
  |                    ^^ expected usize, found reference
  |
  = expected type `usize`
  =    found type `&'static str`
```

colorize the expected and found types within '`' with
Style::UnderlinePrimary`.
@estebank
Copy link
Contributor Author

estebank commented Jan 8, 2017

Current output colorized:

@nrc
Copy link
Member

nrc commented Jan 9, 2017

relevant comment: #38901 (comment)

@bors
Copy link
Contributor

bors commented Jan 11, 2017

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

@estebank
Copy link
Contributor Author

This PR is superseded by #38955 and #38916:

@estebank estebank closed this Jan 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants