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

Fix non-capturing closure return type coercion #88147

Merged
merged 2 commits into from
Sep 11, 2021

Conversation

FabianWolff
Copy link
Contributor

Fixes #88097. For the example given there:

fn peculiar() -> impl Fn(u8) -> u8 {
    return |x| x + 1
}

which incorrectly reports an error, I noticed something weird in the debug log:

DEBUG rustc_typeck::check::coercion coercion::try_find_coercion_lub([closure@test.rs:2:12: 2:21], [closure@test.rs:2:12: 2:21], exprs=1 exprs)

Apparently, try_find_coercion_lub() thinks that the LUB for two closure types always has to be a function pointer (which explains the expected closure, found fn pointer error in #88097). There is one corner case where that isn't true, though — namely, when the two closure types are equal, in which case the trivial LUB is the type itself. This PR fixes this by inserting an explicit check for type equality in try_find_coercion_lub().

@rust-highfive
Copy link
Collaborator

r? @cjgillot

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 18, 2021
@inquisitivecrystal inquisitivecrystal added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 24, 2021
@cjgillot
Copy link
Contributor

r? @jackh726

@rust-highfive rust-highfive assigned jackh726 and unassigned cjgillot Aug 31, 2021
@jackh726
Copy link
Member

So, this is certainly a fix. But I have to think a little harder on the subtlety of this error. Specifically, I would expect that the check if the two types are equal to be an optimization and this could fail in other cases still.

Regardless, in the meantime, can you add a comment?

@FabianWolff
Copy link
Contributor Author

Regardless, in the meantime, can you add a comment?

Sure, where and about what exactly? A reference to #88097 and/or this PR?

@@ -941,6 +941,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
exprs.len()
);

if prev_ty == new_ty {
Copy link
Member

Choose a reason for hiding this comment

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

Comment here referencing #88097 and coercion of a closure to itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it; fixed now.

@jackh726
Copy link
Member

Thanks, I'll try to get back to this soon and think more about this in depth.

@jackh726
Copy link
Member

I'm not 100% sure this is the only way to do this, but it's a very reasonable change, so I'm going to go ahead and @bors r+ rollup

@bors
Copy link
Contributor

bors commented Sep 11, 2021

📌 Commit bbe3be9 has been approved by jackh726

@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 Sep 11, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 11, 2021
Fix non-capturing closure return type coercion

Fixes rust-lang#88097. For the example given there:
```rust
fn peculiar() -> impl Fn(u8) -> u8 {
    return |x| x + 1
}
```
which incorrectly reports an error, I noticed something weird in the debug log:
```
DEBUG rustc_typeck::check::coercion coercion::try_find_coercion_lub([closure@test.rs:2:12: 2:21], [closure@test.rs:2:12: 2:21], exprs=1 exprs)
```
Apparently, `try_find_coercion_lub()` thinks that the LUB for two closure types always has to be a function pointer (which explains the `expected closure, found fn pointer` error in rust-lang#88097). There is one corner case where that isn't true, though — namely, when the two closure types are equal, in which case the trivial LUB is the type itself. This PR fixes this by inserting an explicit check for type equality in `try_find_coercion_lub()`.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 11, 2021
Fix non-capturing closure return type coercion

Fixes rust-lang#88097. For the example given there:
```rust
fn peculiar() -> impl Fn(u8) -> u8 {
    return |x| x + 1
}
```
which incorrectly reports an error, I noticed something weird in the debug log:
```
DEBUG rustc_typeck::check::coercion coercion::try_find_coercion_lub([closure@test.rs:2:12: 2:21], [closure@test.rs:2:12: 2:21], exprs=1 exprs)
```
Apparently, `try_find_coercion_lub()` thinks that the LUB for two closure types always has to be a function pointer (which explains the `expected closure, found fn pointer` error in rust-lang#88097). There is one corner case where that isn't true, though — namely, when the two closure types are equal, in which case the trivial LUB is the type itself. This PR fixes this by inserting an explicit check for type equality in `try_find_coercion_lub()`.
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 11, 2021
…ingjubilee

Rollup of 10 pull requests

Successful merges:

 - rust-lang#87904 (Reword description of automatic impls of `Unsize`.)
 - rust-lang#88147 (Fix non-capturing closure return type coercion)
 - rust-lang#88209 (Improve error message when _ is used for in/inout asm operands)
 - rust-lang#88668 (Change more x64 size checks to not apply to x32.)
 - rust-lang#88733 (Fix ICE for functions with more than 65535 arguments)
 - rust-lang#88757 (Suggest wapping expr in parentheses on invalid unary negation)
 - rust-lang#88779 (Use more accurate spans for "unused delimiter" lint)
 - rust-lang#88830 (Add help for E0463)
 - rust-lang#88849 (don't clone types that are Copy (clippy::clone_on_copy))
 - rust-lang#88850 (don't convert types into identical types)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 94cbefb into rust-lang:master Sep 11, 2021
@rustbot rustbot added this to the 1.57.0 milestone Sep 11, 2021
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.

"Expected type [closure@...], found fn pointer" when unnecessarily returning a closure
7 participants