rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.97k stars 12.69k forks source link

Confusing diagnostics resulting from a function with the bound `T: FnOnce() -> T` that takes `T` and returns `T` #80638

Open PatchMixolydic opened 3 years ago

PatchMixolydic commented 3 years ago

I tried this code (playground):

fn closure_ret_closure<T: FnOnce() -> T>(f: T) -> T {
    f()
}

fn main() {
    closure_ret_closure(|| 4);
}

I expected to see this happen:

Either a diagnostic proclaiming that the type parameter for closure_ret_closure is recursive or a diagnostic that clearly states that || 4 does not return FnOnce() -> FnOnce() -> FnOnce() -> ....

Instead, this happened:

Two seemingly conflicting diagnostics are emitted:

error[E0277]: expected a `FnOnce<()>` closure, found `{integer}`
 --> src/main.rs:6:5
  |
1 | fn closure_ret_closure<T: FnOnce() -> T>(f: T) -> T {
  |                           ------------- required by this bound in `closure_ret_closure`
...
6 |     closure_ret_closure(|| 4);
  |     ^^^^^^^^^^^^^^^^^^^ expected an `FnOnce<()>` closure, found `{integer}`
  |
  = help: the trait `FnOnce<()>` is not implemented for `{integer}`
  = note: wrap the `{integer}` in a closure with no arguments: `|| { /* code */ }`

error[E0308]: mismatched types
 --> src/main.rs:6:25
  |
6 |     closure_ret_closure(|| 4);
  |                         ^^^^ expected integer, found closure
  |
  = note: expected type `{integer}`
          found closure `[closure@src/main.rs:6:25: 6:29]`

The first diagnostic claims that {integer}, the type of the value returned by the closure, is not a FnOnce() -> .... It is not immediately clear that this is in reference to the return type, so the user may believe the value in question is the parameter.

The second diagnostic claims that it expects the parameter to be of type {integer}. This is quite strange, because the previous diagnostic just told us that T: FnOnce() -> FnOnce() -> ..., and {integer} does not fit that requirement.

What may be happening is that since the result of calling || 4 is of type {integer}, the compiler believes T must be {integer}. Since {integer} does not implement FnOnce() -> {integer}, it doesn't fit the bounds on T, generating the first diagnostic. Then, the compiler sees that || 4 is not a valid argument to closure_ret_closure, since f: T and for this monomorphization, T = {integer}.

Even writing out the logic behind this error is quite confusing, so it would be helpful to improve these diagnostics in case a user accidentally stumbles upon them. I'm not entirely sure how to accomplish this, but making it clearer that the first diagnostic (E0277) is referring to the return type might help.

Unfortunately, I'm not sure if anything can be done regarding the rather strange T: FnOnce() -> T bound, since it's quite possible to implement a type that can satisfy T: FnOnce() -> T on nightly. This compiles and runs successfully (playground):

#![feature(fn_traits)]
#![feature(unboxed_closures)]

struct Foo;

impl FnOnce<()> for Foo {
    type Output = Self;

    extern "rust-call" fn call_once(self, _: ()) -> Self::Output {
        self
    }
}

fn closure_ret_closure<T: FnOnce() -> T>(f: T) -> T {
    f()
}

fn main() {
    closure_ret_closure(Foo);
}

Since (as far as I know) FnOnce can only be implemented on nightly, we may be able to get away with making trait bounds like T: FnOnce() -> T a deny-by-default lint, or at least warn-by-default. I'm not certain if this would be entirely feasible/possible with the current trait system, however, since that would be equivalent to linting against this:

trait Foo {
    type Bar;
}

fn baz<T: Foo<Bar = T>>(_: T) { unimplemented!() }

Meta

rustc --version --verbose:

rustc 1.48.0 (7eac88abb 2020-11-16)
binary: rustc
commit-hash: 7eac88abb2e57e752f3302f02be5f3ce3d7adfb4
commit-date: 2020-11-16
host: x86_64-unknown-linux-gnu
release: 1.48.0
LLVM version: 11.0

rustc +nightly --version --verbose:

rustc 1.51.0-nightly (257becbfe 2020-12-27)
binary: rustc
commit-hash: 257becbfe4987d1f7b12af5a8dd5ed96697cd2e8
commit-date: 2020-12-27
host: x86_64-unknown-linux-gnu
release: 1.51.0-nightly

@rustbot modify labels: +A-diagnostics +A-traits +C-enhancement +D-confusing +T-compiler

jyn514 commented 3 years ago

At very least, I think this could highlight 4 instead of the function call:

error[E0277]: expected a `FnOnce<()>` closure, found `{integer}`
 --> src/main.rs:6:5
  |
1 | fn closure_ret_closure<T: FnOnce() -> T>(f: T) -> T {
  |                           ------------- required by this bound in `closure_ret_closure`
...
6 |     closure_ret_closure(|| 4);
  |                            ^ expected an `FnOnce<()>` closure, found `{integer}`
  |
  = help: the trait `FnOnce<()>` is not implemented for `{integer}`
  = note: wrap the `{integer}` in a closure with no arguments: `|| { /* code */ }`

Some other things that would help:

jyn514 commented 3 years ago

@PatchMixolydic that said, this function isn't possible to write in Rust: if you return fn() -> T, that means the function you're currently in has type fn() -> (fn() -> T). So adding more || will always keep giving you more errors.

(I think in untyped languages like lisp this might be possible.)

PatchMixolydic commented 3 years ago

@jyn514 That's quite true (with the exception of manually implementing FnOnce as in the original report, but that's currently unstable as far as I'm aware). It may be possible to not emit note: wrap the T in a closure on E0277 for the case where the generic parameter returns itself, though I fear that may run into some tricky edge cases quite quickly.

I do feel that emitting 4 instead of { /* code */ } might help lead the user to figure out the problem, but the note itself may still be unhelpful since it still fails to compile. The changes to which spans are used would almost definitely be helpful.

estebank commented 8 months ago

Current output:

error[E0271]: expected `{closure@main.rs:6:25}` to be a closure that returns `{closure@main.rs:6:25}`, but it returns `{integer}`
 --> src/main.rs:6:25
  |
6 |     closure_ret_closure(|| 4);
  |     ------------------- ^^^^ expected closure, found integer
  |     |
  |     required by a bound introduced by this call
  |
  = note: expected closure `{closure@src/main.rs:6:25: 6:27}`
                found type `{integer}`
note: required by a bound in `closure_ret_closure`
 --> src/main.rs:1:39
  |
1 | fn closure_ret_closure<T: FnOnce() -> T>(f: T) -> T {
  |                                       ^ required by this bound in `closure_ret_closure`