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

suggestion for E0308 "try expression alternatives have incompatible types" is sometimes wrong #52598

Closed
zackmdavis opened this issue Jul 21, 2018 · 1 comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints

Comments

@zackmdavis
Copy link
Member

In 6cc78bf / #51632, we reworded the error methods for type-mismatches-in-matches when the match comes from a ?-expression, and added a suggestion to Ok-wrap the expression. While that's often the right fix (notably, when a ?-expression is in tail position of a function that returns Result), sometimes it isn't. Here's a counterexample:

fn maybe_numbers() -> Result<Vec<i32>, ()> {
    Ok(vec![1, 2, 3])
}

fn try_it() -> Result<String, ()> {
    let n: i32 = maybe_numbers()?;
    Ok(format!("{:?}", n))
}

fn main() {}

Currently, we emit:

error[E0308]: try expression alternatives have incompatible types
 --> success_variant.rs:6:18
  |
6 |     let n: i32 = maybe_numbers()?;
  |                  ^^^^^^^^^^^^^^^^
  |                  |
  |                  expected i32, found struct `std::vec::Vec`
  |                  help: try wrapping with a success variant: `Ok(maybe_numbers()?)`
  |
  = note: expected type `i32`
             found type `std::vec::Vec<i32>`

We should, somehow, not issue the "try wrapping with a success variant" suggestion (which lives in librustc/infer/error_reporting/mod.rs) in cases like these.

@zackmdavis zackmdavis changed the title suggestion for E0308 "try expression alternatives have incompatible types" are sometimes wrong suggestion for E0308 "try expression alternatives have incompatible types" is sometimes wrong Jul 21, 2018
@csmoe csmoe added the A-diagnostics Area: Messages for errors, warnings, and lints label Jul 21, 2018
@zackmdavis
Copy link
Member Author

Turns out this was already filed as #52537.

zackmdavis added a commit to zackmdavis/rust that referenced this issue Oct 27, 2018
This suggestion was introduced in rust-lang#51938 / 6cc78bf (while
introducing different language for type errors coming from `?` rather
than a `match`), but it has a lot of false-positives (as repeatedly
reported in Issues rust-lang#52537, rust-lang#52598, rust-lang#54578, rust-lang#55336), and incorrect
suggestions carry more badness than marginal good suggestions do
goodness. Just get rid of it (unless and until someone figures out how
to do it correctly).

Resolves rust-lang#52537, resolves rust-lang#54578.
pietroalbini pushed a commit to alexcrichton/rust that referenced this issue Oct 29, 2018
This suggestion was introduced in rust-lang#51938 / 6cc78bf (while
introducing different language for type errors coming from `?` rather
than a `match`), but it has a lot of false-positives (as repeatedly
reported in Issues rust-lang#52537, rust-lang#52598, rust-lang#54578, rust-lang#55336), and incorrect
suggestions carry more badness than marginal good suggestions do
goodness. Just get rid of it (unless and until someone figures out how
to do it correctly).

Resolves rust-lang#52537, resolves rust-lang#54578.
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

2 participants