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

Integral type mismatch error messages should use "expression" instead of "variable" #56115

Closed
dooblad opened this issue Nov 20, 2018 · 6 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints 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.

Comments

@dooblad
Copy link

dooblad commented Nov 20, 2018

Type mismatch error messages involving integral expressions use the word "variable", when the problematic term is an expression.

I tried this code:

fn main() {
    let f = if true {
        ()
    } else {
        1
    };
}

I expected to see this happen:

<code sample that causes the bug>
error[E0308]: if and else have incompatible types
 --> src/main.rs:2:13
  |
2 |       let f = if true {
  |  _____________^
3 | |         ()
4 | |     } else {
5 | |         1
6 | |     };
  | |_____^ expected (), found integral expression
  |
  = note: expected type `()`
             found type `{integer}`

Instead, this happened:

<code sample that causes the bug>
error[E0308]: if and else have incompatible types
 --> src/main.rs:2:13
  |
2 |       let f = if true {
  |  _____________^
3 | |         ()
4 | |     } else {
5 | |         1
6 | |     };
  | |_____^ expected (), found integral variable
  |
  = note: expected type `()`
             found type `{integer}`

Meta

rustc --version --verbose:

rustc 1.31.0-nightly (3e6f30ec3 2018-10-26)
binary: rustc
commit-hash: 3e6f30ec3e6bda159063fcd126dcb14725fef92d
commit-date: 2018-10-26
host: x86_64-apple-darwin
release: 1.31.0-nightly
LLVM version: 8.0

Backtrace: N/A

@dooblad dooblad changed the title "... found <dtype> variable" error messages should say "... found <dtype> expression" Integral type mismatch error messages should use "expression" instead of "variable" Nov 20, 2018
@varkor varkor added the A-diagnostics Area: Messages for errors, warnings, and lints label Nov 21, 2018
@varkor
Copy link
Member

varkor commented Dec 19, 2018

It's probably sufficient just to change:

ty::Infer(ty::IntVar(_)) => "integral variable".into(),
ty::Infer(ty::FloatVar(_)) => "floating-point variable".into(),

to:

            ty::Infer(ty::IntVar(_)) => "integer".into(),
            ty::Infer(ty::FloatVar(_)) => "floating-point number".into(),

@varkor varkor added 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. labels Dec 19, 2018
@hdhoang
Copy link
Contributor

hdhoang commented Dec 22, 2018

Do you think the FreshIntTy and FreshFloatTy below those should be changed similarly? I haven't seen the fresh ones in messages before.

@varkor
Copy link
Member

varkor commented Dec 22, 2018

Those should not appear in user error messages, so they can be ignored.

@codeworm96
Copy link
Contributor

Is there anyone working on this issue? If not, I would like to have a try.

@codeworm96
Copy link
Contributor

I almost finished the change, but I start to worry about whether we really want to change it.

  1. The old terminology is already used widely (I have to change many tests)
  2. The "variable" does have some meaning. It means type variable in the type inference. (However, it has nothing to do with the "variable" in the user's code, which caused the confusion.)

@hdhoang
Copy link
Contributor

hdhoang commented Jan 1, 2019

Well done! I'm certain this is an improvement for users, since we also emit this message for integer literals. In my head, "integer -> oh, my integer-typed variable" flows very nicely, compared to the original report above where I have to make the inference "integral variable -> uuuhh, the literal integer I put there". Reading the updated stderr files gave me similar feeling.

I have seen this message used in the submodules for src/doc/book/ and src/tools/clippy, so you will need to change those too. Then making PRs following this procedure: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#breaking-tools-built-with-the-compiler will update the repos in concert.

bors added a commit that referenced this issue Jan 2, 2019
Improve type mismatch error messages

Closes #56115.

Replace "integral variable" with "integer" and replace "floating-point variable" with "floating-point number" to make the message less confusing.

TODO the book and clippy needs to be changed accordingly later.

r? @varkor
bors added a commit that referenced this issue Jan 2, 2019
Improve type mismatch error messages

Closes #56115.

Replace "integral variable" with "integer" and replace "floating-point variable" with "floating-point number" to make the message less confusing.

TODO the book and clippy needs to be changed accordingly later.

r? @varkor
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 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.
Projects
None yet
Development

No branches or pull requests

4 participants