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

Confusing "the caller chooses a type which can be different" [E308] #126547

Closed
jendrikw opened this issue Jun 16, 2024 · 4 comments · Fixed by #126558
Closed

Confusing "the caller chooses a type which can be different" [E308] #126547

jendrikw opened this issue Jun 16, 2024 · 4 comments · Fixed by #126558
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jendrikw
Copy link
Contributor

Code

fn f<T>(t: &T) -> T {
    t
}

Current output

error[E0308]: mismatched types
 --> src/lib.rs:2:5
  |
1 | fn f<T>(t: &T) -> T {
  |      -            - expected `T` because of return type
  |      |
  |      expected this type parameter
2 |     t
  |     ^ expected type parameter `T`, found `&T`
  |
  = note: expected type parameter `_`
                  found reference `&_`
  = note: the caller chooses a type for `T` which can be different from `&T`

Desired output

error[E0308]: mismatched types
 --> src/lib.rs:2:5
  |
1 | fn f<T>(t: &T) -> T {
  |      -            - expected `T` because of return type
  |      |
  |      expected this type parameter
2 |     t
  |     ^ expected type parameter `T`, found `&T`
  |
  = note: expected type parameter `_`
                  found reference `&_`

Rationale and extra context

The statement "the caller chooses a type for T which can be different from &T" is confusing because T and &T are always different. I think it would be best to supress that note when the types only differ in reference-ness. The message was introduced in #122195.

Other cases

No response

Rust Version

rustc 1.81.0-nightly (3cf924b93 2024-06-15)
binary: rustc
commit-hash: 3cf924b934322fd7b514600a7dc84fc517515346
commit-date: 2024-06-15
host: x86_64-unknown-linux-gnu
release: 1.81.0-nightly
LLVM version: 18.1.7

Anything else?

No response

@jendrikw jendrikw added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 16, 2024
@jendrikw
Copy link
Contributor Author

@rustbot label +D-confusing

@rustbot rustbot added the D-confusing Diagnostics: Confusing error or lint that should be reworked. label Jun 16, 2024
@fmease
Copy link
Member

fmease commented Jun 16, 2024

Thanks, that note definitely doesn't make sense.

CC @jieyouxu (#122195).

I suggested to move note_caller_chooses_ty_for_ty_param out of try_suggest_return_impl_trait here in order to catch more cases but that was a bit too aggressive as can be witnessed here — that's on me.

We should replicate some parts of try_suggest_return_impl_trait, namely checking that the type param in question does not occur in the return expression type. That did cross my mind back then but I didn't really follow up on that thought for some reason.

@jieyouxu jieyouxu self-assigned this Jun 16, 2024
@jieyouxu
Copy link
Member

Thanks for the cc @fmease, I'll take a look ~today/tomorrow

rust-cloud-vms bot pushed a commit to jieyouxu/rust that referenced this issue Jun 16, 2024
…ram" note

- Avoid "caller chooses ty for type param" note if the found type a.k.a.
  the return expression type *contains* the type parameter, because e.g.
  `&T` will always be different from `T` (i.e. "well duh").
- Rename `note_caller_chooses_ty_for_ty_param` to
  `try_note_caller_chooses_ty_for_ty_param` because the note is not
  always made.

Issue: rust-lang#126547
@jieyouxu
Copy link
Member

jieyouxu commented Jun 16, 2024

Opened #126558 to avoid making the note if found return type contains the expected return type (type parameter).

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jun 18, 2024
hir_typeck: be more conservative in making "note caller chooses ty param" note

In rust-lang#122195 I added a "caller chooses ty for type param" note for when the return expression type a.k.a. found type does not match the expected return type.

rust-lang#126547 found that this note was confusing when the found return type *contains* the expected type, e.g.

```rs
fn f<T>(t: &T) -> T {
    t
}
```

because the found return type `&T` will *always* be different from the expected return type `T`, so the note was needlessly redundant and confusing.

This PR addresses that by not making the note if the found return type contains the expected return type.

r? `@fmease` (since you reviewed the original PR)

Fixes rust-lang#126547
rust-cloud-vms bot pushed a commit to jieyouxu/rust that referenced this issue Jun 18, 2024
…ram" note

- Avoid "caller chooses ty for type param" note if the found type a.k.a.
  the return expression type *contains* the type parameter, because e.g.
  `&T` will always be different from `T` (i.e. "well duh").
- Rename `note_caller_chooses_ty_for_ty_param` to
  `try_note_caller_chooses_ty_for_ty_param` because the note is not
  always made.

Issue: rust-lang#126547
jieyouxu added a commit to jieyouxu/rust that referenced this issue Jun 19, 2024
hir_typeck: be more conservative in making "note caller chooses ty param" note

In rust-lang#122195 I added a "caller chooses ty for type param" note for when the return expression type a.k.a. found type does not match the expected return type.

rust-lang#126547 found that this note was confusing when the found return type *contains* the expected type, e.g.

```rs
fn f<T>(t: &T) -> T {
    t
}
```

because the found return type `&T` will *always* be different from the expected return type `T`, so the note was needlessly redundant and confusing.

This PR addresses that by not making the note if the found return type contains the expected return type.

r? `@fmease` (since you reviewed the original PR)

Fixes rust-lang#126547
@bors bors closed this as completed in bf841c4 Jun 19, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 19, 2024
Rollup merge of rust-lang#126558 - jieyouxu:caller-chooses-ty, r=fmease

hir_typeck: be more conservative in making "note caller chooses ty param" note

In rust-lang#122195 I added a "caller chooses ty for type param" note for when the return expression type a.k.a. found type does not match the expected return type.

rust-lang#126547 found that this note was confusing when the found return type *contains* the expected type, e.g.

```rs
fn f<T>(t: &T) -> T {
    t
}
```

because the found return type `&T` will *always* be different from the expected return type `T`, so the note was needlessly redundant and confusing.

This PR addresses that by not making the note if the found return type contains the expected return type.

r? ``@fmease`` (since you reviewed the original PR)

Fixes rust-lang#126547
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-confusing Diagnostics: Confusing error or lint that should be reworked. 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.

4 participants