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 mismatched type error #43608

Open
joshlf opened this issue Aug 2, 2017 · 9 comments · Fixed by #106752
Open

Confusing mismatched type error #43608

joshlf opened this issue Aug 2, 2017 · 9 comments · Fixed by #106752
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@joshlf
Copy link
Contributor

joshlf commented Aug 2, 2017

If you forget to put a semicolon after the last expression in the body of a function that returns no values, rustc will infer that the type of the expression should be (). If the programmer didn't intend to return a value, but merely forgot to type a semicolon, the resulting error can be confusing. Consider, for example, the following toy program:

use std::ptr;

fn main() {
    let a = 0;
    ptr::read(&a)
}

This produces the following error message:

error[E0308]: mismatched types
 --> src/main.rs:5:15
  |
5 |     ptr::read(&a)
  |               ^^ expected (), found integral variable
  |
  = note: expected type `*const ()`
             found type `&{integer}`

In this simple example, it may be obvious, but in the somewhat more complicated code I was working with that led me to this issue, I spent a decent amount of time trying to figure out why I was supposedly passing a pointer to () to mem::replace, thus causing the type of the second argument to replace to fail to unify. It took me a while to figure out that this was actually the culprit.

@Mark-Simulacrum Mark-Simulacrum added A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Aug 3, 2017
@estebank
Copy link
Contributor

Triage: no change.

@estebank estebank added D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 15, 2019
@estebank
Copy link
Contributor

estebank commented Jan 5, 2023

Mentoring notes:

You need to take the &a expression, look at its parent to see if it is a method or function call, get its corresponding item return type and see if the return type and the argument that had a type mismatch are both the same type parameter (or really almost the same, to account for things like T and -> Option<&T>, you can use references_type for this). Once you have that, you can use the existing machinery that points at where type obligations might have come from, like the one that points at the let type in type mismatches for binding assignment.

@sulami
Copy link
Contributor

sulami commented Jan 7, 2023

I'd like to have a shot at this.

@rustbot claim

@estebank
Copy link
Contributor

estebank commented Jan 7, 2023

@sulami for an example of a similar case I worked on recently, you can look at https://github.com/rust-lang/rust/pull/106519/files#diff-f1e0bfc4e359c5e1d92ebdc4aafe6dfda183fbdbc9933a3b5e9a7fb274b9d84bR1675-R1717

@sulami
Copy link
Contributor

sulami commented Jan 8, 2023

I've pushed a WIP commit and would appreciate some early feedback if this is going in the right direction.

@estebank
Copy link
Contributor

estebank commented Jan 8, 2023

@sulami added some comments to the commit linked above your comment. Let me know if I was too terse, but the high level idea is you already have the HIR node for the call, which is great. From it you can look at the fields of the node to get the Res::Def if it is available, and with that you can get the original function/method type (I think that tcx.type_of(def_id) might work for you here, but I'm not sure). Once you have that, you need to check every single input type against the output to see if they are 1) a type parameter and 2) the same. That way you'll be able to tell that fn foo<T>(x: T) -> T { x } is affected by inference on the argument and the return type. Once we have that the rest should be easy, it's a matter of lining things up and tweaking the output to look the way we want :)

@sulami
Copy link
Contributor

sulami commented Jan 11, 2023

So I think I've got most things lined up now. I have two questions though:

  1. What do we actually want the help message to look like? I just picked something that seemed generally sensible, but I imagine it could be better/more idiomatic somehow.

  2. The blast radius of this is actually larger than I expected. It's currently affecting about a dozen ui tests, mostly because something like this is also caught in this:

    fn foo(x: Option<Option<u32>>) {}
    
    fn main() {
        foo(Some(42))
    }

    Because Some(..) is considered a call, the mismatch caught here is in the argument of Some(), not in the argument of foo(). I'm not sure that should be the case here, but I'm also not quite sure right now how to distinguish this case from the original example provided.

@estebank
Copy link
Contributor

estebank commented Jan 11, 2023

It looks great! Ideally I think that the output would look closer to

error[E0308]: arguments to this function are incorrect
  --> $DIR/wrong-call-return-type-due-to-generic-arg.rs:27:5
   |
LL |     function(0u32, 8u8)
   |     ^^^^^^^^ ----  --- expected `bool`, found `u8`
   |              |
   |              expected `()`, found `u32`
   |
help: the return type of this call is `u32` due to the type of the argument passed
  --> $DIR/wrong-call-return-type-due-to-generic-arg.rs:27:5
   |
LL |     function(0u32, 8u8)
   |     ^^^^^^^^^----^^^^^^
   |                  |
   |                  this argument determines the return type of `function`
note: function defined here
  --> $DIR/wrong-call-return-type-due-to-generic-arg.rs:1:4
   |
L  | fn function<T, N>(x: T, y: bool) -> T {
   |    ^^^^^^^^       ----  -------

You can accomplish that by doing something like

let mut multi_span: MultiSpan = parent_expr.span.into();
multi_span.push_span_label(args[arg_idx].span, format!("this argument influences the return type of `{}`", path.segments[0].ident));
err.span_help(multi_span, ...);

You can also look at the Res of the call to see what item type it resolved to, and only emit these if it is an Assoc Fn or Fn.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 14, 2023
Emit a hint for bad call return types due to generic arguments

When the return type of a function call depends on the type of an argument, e.g.

```
fn foo<T>(x: T) -> T {
    x
}
```

and the expected type is set due to either an explicitly typed binding, or because the call to the function is in a tail position without semicolon, the current error implies that the argument in the call has the wrong type.

This new hint highlights that the expected type doesn't match the returned type, which matches the argument type, and that that's why we're flagging the argument type.

Fixes rust-lang#43608.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 14, 2023
Emit a hint for bad call return types due to generic arguments

When the return type of a function call depends on the type of an argument, e.g.

```
fn foo<T>(x: T) -> T {
    x
}
```

and the expected type is set due to either an explicitly typed binding, or because the call to the function is in a tail position without semicolon, the current error implies that the argument in the call has the wrong type.

This new hint highlights that the expected type doesn't match the returned type, which matches the argument type, and that that's why we're flagging the argument type.

Fixes rust-lang#43608.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 14, 2023
Emit a hint for bad call return types due to generic arguments

When the return type of a function call depends on the type of an argument, e.g.

```
fn foo<T>(x: T) -> T {
    x
}
```

and the expected type is set due to either an explicitly typed binding, or because the call to the function is in a tail position without semicolon, the current error implies that the argument in the call has the wrong type.

This new hint highlights that the expected type doesn't match the returned type, which matches the argument type, and that that's why we're flagging the argument type.

Fixes rust-lang#43608.
@bors bors closed this as completed in a3cf382 Jan 14, 2023
@joshlf
Copy link
Contributor Author

joshlf commented Aug 1, 2023

Looks like the error message for the original example is still unchanged even with #106752. Is that expected behavior?

@estebank estebank reopened this Aug 1, 2023
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 C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. 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