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

Emit a hint for bad call return types due to generic arguments #106752

Merged
merged 1 commit into from
Jan 14, 2023

Conversation

sulami
Copy link
Contributor

@sulami sulami commented Jan 12, 2023

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 #43608.

@rustbot
Copy link
Collaborator

rustbot commented Jan 12, 2023

r? @TaKO8Ki

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 12, 2023
@sulami
Copy link
Contributor Author

sulami commented Jan 12, 2023

r? @estebank

@rustbot rustbot assigned estebank and unassigned TaKO8Ki Jan 12, 2023
match parent_expr.kind {
hir::ExprKind::Call(fun, args) => {
let hir::ExprKind::Path(hir::QPath::Resolved(_, path)) = fun.kind else { return; };
let hir::def::Res::Def(_kind, def_id) = path.res else { return; };
Copy link
Contributor

@estebank estebank Jan 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To filter the enum variants and tuple structs away you could look at _kind here to see that it is not a Ctor ("constructor"), but I'm seeing some cases where this note helps.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm torn on this as well because it's wide-reaching, but it does help in some cases because constructors are prone to hit the same confusing errors as regular function calls.

LL | let _: Option<(i32,)> = Some(5_usize);
| ^^^^^-------^
| |
| this argument influences the return type of `Some`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe what we should do is tweak the wording depending on the item type? Because I'm sure people are going to be confused by talking about the "return type of an enum variant". Seeing tuple structs as functions is natural from the compiler's point of view, but not from the users'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've put something in that tweaks the wording. It's slightly awkward for constructors, because they return a type that's different from the argument, and I don't know if there is a reliable way of getting that type regardless of wrapping, but this now is at least coherent.

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.
@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jan 13, 2023

📌 Commit a3cf382 has been approved by estebank

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 13, 2023
Comment on lines +9 to +15
help: the type constructed contains `()` due to the type of the argument passed
--> $DIR/issue-84128.rs:13:9
|
LL | Foo(())
| ^^^^--^
| |
| this argument influences the type of `Foo`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm noticing that there are some cases where the existing machinery to detect the source of expectations isn't as robust as we'd like, but that's independent of this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #106821

Comment on lines +14 to +20
help: the type constructed contains `{integer}` due to the type of the argument passed
--> $DIR/args-instead-of-tuple-errors.rs:6:34
|
LL | let _: Option<(i32, bool)> = Some(1, 2);
| ^^^^^-^^^^
| |
| this argument influences the type of `Some`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case should probably have been silenced.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request 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 pull request 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 added a commit to rust-lang-ci/rust that referenced this pull request Jan 14, 2023
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#106046 (Fix mir-opt tests for big-endian platforms)
 - rust-lang#106470 (tidy: Don't include wasm32 in compiler dependency check)
 - rust-lang#106566 (Emit a single error for contiguous sequences of unknown tokens)
 - rust-lang#106644 (Update the wasi-libc used for the wasm32-wasi target)
 - rust-lang#106665 (Add note when `FnPtr` vs. `FnDef` impl trait)
 - rust-lang#106752 (Emit a hint for bad call return types due to generic arguments)
 - rust-lang#106788 (Tweak E0599 and elaborate_predicates)
 - rust-lang#106831 (Use GitHub yaml templates for ICE, Docs and Diagnostics tickets)
 - rust-lang#106846 (Improve some comments and names in parser)
 - rust-lang#106848 (Fix wrong path in triage bot autolabel for wg-trait-solver-refactor)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 108b5f4 into rust-lang:master Jan 14, 2023
@rustbot rustbot added this to the 1.68.0 milestone Jan 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Confusing mismatched type error
5 participants