rust-lang/rust

Cannot call implementations of the `Fn*` traits on references

Opened this issue ยท 14 comments

Attempting to implement FnOnce for a reference to a struct or a trait object:

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

struct S;

fn repro_ref(thing: &S) {
    thing();
}

impl<'a> FnOnce<()> for &'a S {
    type Output = ();

    extern "rust-call" fn call_once(self, _arg: ()) -> () {}
}

fn main() {}

Produces the error:

error[E0618]: expected function, found `&S`
 --> src/main.rs:7:5
  |
7 |     thing();
  |     ^^^^^^^
  |
note: defined here
 --> src/main.rs:6:14
  |
6 | fn repro_ref(thing: &S) {
  |              ^^^^^

Interestingly, both of these alternatives work:

fn ok_ref_ref_arg(thing: &&S) {
    thing();
}

fn ok_ref_ref(thing: &S) {
    (&thing)();
}

cc #29625.

Originally from this Stack Overflow question.

eddyb commented

cc @nikomatsakis Sounds like the overloaded call derefs too eagerly? Or is missing autoref?

I am the one that posted the relevant StackOverflow question. I found some more example:

trait T1 {
}

trait T2 {
}

impl<'a> T1 for &'a T2 {
}

struct S {
}

impl T2 for S {
}

fn main() {
    let t2:&T2 = &S {};
    let t1:&T1 = &t2; //This is OK
    let t3:&T1 = t2; //E0308: Expecting `T1`, found `T2`
}

Note the above does not require any features and behave the same in all channels. So, there has nothing to do with closures and Fn Traits. It is about implementing traits for a trait object reference type.

In the simplified example, I tried to defense the current compiler behavior.

The statement let t3: &T1 = t2; says

take t2 (which is a trait object of type &T2) and bind it to a variable t3, which is a trait object reference to something implements T1.

t2 is a reference, but pointing to T2, it is NOT a reference to T1. As t2 is a variable, not an expression, the compiler is not able to introduce code to convert the variable unless some automatic conversion applies here. Unfortunately, the language does not allow using implementation of traits to do automatic conversion.

On the other hand, let t1: &T1 = &t2; says

calculate expression &t2 and bind the result to a variable t1, which is a trait object reference to something implements T1.

The difference here is we now have an expression, so it have to be calculated. The type checker will make sure the result is a reference to T1, and to do this the compiler have to search for the trait implementation for &T2.

So, although counter intuitive, but the current compiler behavior is not wrong. I think to achieve the designated usage, what we want to do is not implement a trait on top of a trait object, but to implement the conversion traits instead (already in my to-study list), so the compiler can apply the conversion automatically.

I'm not sure what's going wrong. Have to trace through it I guess. I added this bug as a blocker to stabilizing. Going to call it P-medium.

triage: P-medium

Progress on this lately? :-)

Are we in a good place to tackle this now that #45510 has been solved?

One more example that works:

fn repro_ref(thing: impl FnOnce() -> ()) {
    thing();
}

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=b53ffaf9ecf6bc8f4856d93b8d491993

eddyb commented

@elycruz That's missing the reference though, this OTOH does error:

fn repro_ref(thing: &impl FnOnce()) {
    thing();
}

Interestingly, the error is different with a generic like that (it doesn't have to be impl FnOnce(), it could also be an F: FnOnce() named type parameter).

(Btw, your link is just to the playground, not to your example - you need to press the Share button and copy the first link from there)

@eddyb You're right (gotcha ๐Ÿ‘ (updated my link ๐Ÿ‘)).

Any news? If used wisely, generic callable "objects" can make for some very elegant APIs -- however here's a playground example of how this is still not possible.

@eddyb

.. this OTOH does error:

fn repro_ref(thing: &impl FnOnce()) {
    thing();
}

That code is simply wrong though. thing implements FnOnce so must be consumed, but it's only passed by reference.

Longer repro with current Nightly:


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


fn call_fn_owned(z: impl Fn()) {
   z();
}
fn call_fn_ref(z: &impl Fn()) {
   z();
}

struct RR;

impl Fn<()> for &'_ RR {
    extern "rust-call" fn call(&self, (): ()) -> Self::Output { dbg!(); }
}
impl FnOnce<()> for &'_ RR {
    type Output = ();
    extern "rust-call" fn call_once(self, (): ()) -> Self::Output { dbg!(); }
}
impl FnMut<()> for &'_ RR {
    extern "rust-call" fn call_mut(&mut self, (): ()) -> Self::Output { dbg!(); }
}

fn main() {
    call_fn_owned(|| dbg!());
    call_fn_ref(&|| dbg!());

    call_fn_owned(&RR);
    call_fn_ref(&&RR);

    // RR();     <--- ERROR expected function, found struct `RR`
    // (&RR)();  <--- ERROR expected function, found struct `&RR`
    (&&RR)();

    let r = RR;
    // r();      <--- ERROR expected function, found struct `RR`
    // (&r)();   <--- ERROR expected function, found struct `RR`
    (&&r)();
}

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=010b332c416581f94ff6d3a84c6c3e4d

I think the compiler isn't doing some autoref that it would for method despatch. Is this actually a blocking issue for #29625 (stabilising impl Fn) though? It can only bite you if you do something like impl FnOnce for &T. Why would you do that when you could impl Fn for T?

i think that if people really cared about this, it would have been fixed by now. can we just close this issue and make fn_traits stable.

For anyone interested, this issue is due to an explicit hack here:

// Hack: we know that there are traits implementing Fn for &F
// where F:Fn and so forth. In the particular case of types
// like `f: &mut FnMut()`, if there is a call `f()`, we would
// normally translate to `FnMut::call_mut(&mut f, ())`, but
// that winds up potentially requiring the user to mark their
// variable as `mut` which feels unnecessary and unexpected.
//
// fn foo(f: &mut impl FnMut()) { f() }
// ^ without this hack `f` would have to be declared as mutable
//
// The simplest fix by far is to just ignore this case and deref again,
// so we wind up with `FnMut::call_mut(&mut *f, ())`.

It was added in cdb10b8. The point is to avoid going through the impl<F: FnMut> FnMut for &mut F impl when F's FnMut impl can be used directly.

An attempt to change this earlier this year (#137252) was declined on the basis that the fn_traits feature is blocked on more important things than this. So, for anyone looking to move fn_traits forward, I'd consider this here issue "not a blocker", that shouldn't be too hard to resolve once the rest is settled.