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

Rust claims that Ok(T) is the same as &T #54578

Closed
sgrif opened this issue Sep 25, 2018 · 3 comments · Fixed by #55423
Closed

Rust claims that Ok(T) is the same as &T #54578

sgrif opened this issue Sep 25, 2018 · 3 comments · Fixed by #55423
Labels
A-diagnostics Area: Messages for errors, warnings, and lints

Comments

@sgrif
Copy link
Contributor

sgrif commented Sep 25, 2018

Steps to reproduce:

  • Have a function that takes a reference
  • Call that function passing an expression using ?
  • Forget an &

Example code: https://play.rust-lang.org/?gist=c75cddbb4c5d8a1b63ec11dc3c5bca43&version=stable&mode=debug&edition=2015

Output:

error[E0308]: try expression alternatives have incompatible types
 --> src/main.rs:5:9
  |
5 |     foo(bar()?);
  |         ^^^^^^
  |         |
  |         expected &i32, found i32
  |         help: try wrapping with a success variant: `Ok(bar()?)`
  |
  = note: expected type `&i32`
             found type `i32`

Expected output:

error[E0308]: mismatched types
 --> src/main.rs:5:9
  |
5 |     foo(bar()?);
  |         ^^^^^^
  |         |
  |         expected &i32, found i32
  |         help: consider borrowing here: `&bar()?`
  |
  = note: expected type `&i32`
             found type `i32`
@sgrif
Copy link
Contributor Author

sgrif commented Sep 25, 2018

The error used to be mostly correct at least as recently as 1.26.1 (I don't have 1.27 or 1.28 installed to quickly check), though "mismatched types" read "match arm with an incompatible type". I suspect this regression occurred in whatever commit fixed that bug.

@Centril Centril added the A-diagnostics Area: Messages for errors, warnings, and lints label Sep 26, 2018
@zackmdavis
Copy link
Member

Related to #52537 (which regressed in #51938, sorry 😰 ).

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.
@zackmdavis
Copy link
Member

#55423 will/would change the output to

error[E0308]: try expression alternatives have incompatible types
 --> segrif.rs:5:9
  |
5 |     foo(bar()?);
  |         ^^^^^^ expected &i32, found i32
  |
  = note: expected type `&i32`
             found type `i32`

error: aborting due to previous error

pietroalbini added a commit to pietroalbini/rust that referenced this issue Oct 29, 2018
…ng_suggestion, r=estebank

back out bogus `Ok`-wrapping suggestion on `?` arm type mismatch

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, and incorrect suggestions carry more badness than marginal good suggestions do goodness. I regret not doing this earlier. 😞

Resolves rust-lang#52537, resolves rust-lang#54578.

r? @estebank
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

Successfully merging a pull request may close this issue.

3 participants