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

Improve error message for statements used as expressions #105431

Closed
mcoblenz opened this issue Dec 7, 2022 · 3 comments · Fixed by #121153
Closed

Improve error message for statements used as expressions #105431

mcoblenz opened this issue Dec 7, 2022 · 3 comments · Fixed by #121153
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@mcoblenz
Copy link

mcoblenz commented Dec 7, 2022

The code below has superfluous semicolons in the branches of the if expression:

fn test() -> i32 {
    let x = if (true) {
        3;
    }
    else {
        4;
    };
    x
}

The error message that results is:

error[E0308]: mismatched types
  --> src/lib.rs:11:5
   |
4  | fn test() -> i32 {
   |              --- expected `i32` because of return type
...
11 |     x
   |     ^ expected `i32`, found `()`

This can be confusing to beginners, who may not understand unit type. However, since the expressions 3 and 4 can be shown to have no side effects, the code (as is) can't be what the user intended. Could the compiler produce a more targeted error message that mentions the superfluous semicolon?

@fmease

This comment was marked as resolved.

@rustbot rustbot added A-diagnostics Area: Messages for errors, warnings, and lints D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 7, 2022
@mcoblenz mcoblenz changed the title Improve error message for expressions used as statements Improve error message for statements used as expressions Dec 8, 2022
@cassaundra
Copy link
Contributor

@rustbot claim

@cassaundra
Copy link
Contributor

cassaundra commented Jan 11, 2023

Thinking more about this, I don't think we should extend the "remove semicolon to make tail expression" suggestion to cover this case, since we don't know the expected type of x at the point at which the suggestion would be made. For example, we don't make the suggestion to change "a" to 'a' here:

fn test() -> char {
    let x = "a";
    x
}

But that's just a guess based on my limited knowledge of type mismatch suggestions. Maybe someone else could weigh in?

@cassaundra cassaundra removed their assignment Jan 12, 2023
@chenyukang chenyukang self-assigned this Feb 14, 2024
@bors bors closed this as completed in b8cdcfa Mar 2, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Mar 2, 2024
Rollup merge of rust-lang#121153 - chenyukang:yukang-fix-105431-type-mismatch, r=estebank

Suggest removing superfluous semicolon when statements used as expression

Fixes rust-lang#105431

- it's not a pure recursive visitor, so I guess there may be some more complex scenarios not covered.
- moved `consider_removing_semicolon` to `compiler/rustc_infer` for reusing this helper function.
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 D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants