rust-lang / rust

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

Cannot use function pointer with dynamic dispatch without identity cast #86654

Open jonhoo opened 3 years ago

jonhoo commented 3 years ago

I tried this code (playground):

pub trait Foo {}

impl Foo for fn(*mut dyn Drop) {}

pub fn bar(_: &dyn Foo) {}

pub fn baz() {
    fn good(_: *mut dyn Drop) {}

    // All good:
    bar(&(good as fn(*mut dyn Drop)));

    // No good:
    bar(&good);
}

I expected to see this happen: the code should compile.

Instead, this happened: compilation fails with the error

error[E0277]: the trait bound `fn(*mut (dyn Drop + 'static)) {good}: Foo` is not satisfied
  --> src/lib.rs:14:9
   |
14 |     bar(&good);
   |         ^^^^^ the trait `Foo` is not implemented for `fn(*mut (dyn Drop + 'static)) {good}`
   |
   = note: required for the cast to the object type `dyn Foo`

This feels wrong, since the cast appears to be applying the identity function, but I could be missing something about how this is intended to work. Changing dyn Drop to (dyn Drop + 'static) (as the error message hints at) does not make a difference.

Meta

rustc --version --verbose:

rustc 1.53.0 (53cb7b09b 2021-06-17)
binary: rustc
commit-hash: 53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b
commit-date: 2021-06-17
host: x86_64-unknown-linux-gnu
release: 1.53.0
LLVM version: 12.0.1

The warning also occurs on nightly.

FabianWolff commented 3 years ago

I agree that the error message could be improved, but just to explain what happens here: The cast is not the identity transformation, because good has type FnDef (and not FnPtr). Function pointers are created by implicitly coercing function items into pointers (or explicitly with a cast, as you did); see the reference:

Function pointer types, written using the fn keyword, refer to a function whose identity is not necessarily known at compile-time. They can be created via a coercion from both function items and non-capturing closures.

jonhoo commented 3 years ago

Oh, interesting, thanks for the explanation. Yeah, the error message can certainly be improved here, since it doesn't actually show any meaningful difference between the types. I also wonder if this would be a good candidate for allowing implicit coercion, rather than requiring it to be explicit as is the case currently — is there ever a case where this coercion is undesirable?

FabianWolff commented 3 years ago

I think a simple coercion isn't enough here, because you have to make the reference point to something: good has the wrong type, so you'd have to (in principle) construct a temporary, coerce good to a function pointer and store it in the temporary, and then take a reference to that temporary to pass to bar. In your first case (with the explicit cast), the compiler actually seems to generate a constant in the MIR that holds the function pointer, and makes the reference point to that. In the second case, it doesn't do this (perhaps it is considered too complex a transformation for an implicit coercion), so the reference has nowhere to point to. Note that the same problem arises in this simplified example:

pub fn bar(_: &fn()) {}

pub fn baz() {
    fn good() {}

    // All good:
    bar(&(good as fn()));

    // No good:
    bar(&good);
}
error[E0308]: mismatched types
  --> src/lib.rs:10:9
   |
10 |     bar(&good);
   |         ^^^^^ expected fn pointer, found fn item
   |
   = note: expected reference `&fn()`
              found reference `&fn() {good}`

If you remove the references in lines 1, 7 and 10, it compiles.

jonhoo commented 3 years ago

Oh, right, fn itself is a pointer here, so &fn is a pointer to a pointer, and the question is where that intermediate pointer is stored. Makes me wonder how the compiler makes the cast work in the first place — it can't just be a constant, since that doesn't have a memory address. I'm guessing it's probably an anonymous static then, which is also weird that the cast can generate implicitly.

Stepping back for a second, I also wonder what the right way to support the "fn as dyn" use-case is. Does it just fundamentally require the extra indirection? That is, if I want &dyn Foo where there's an impl Foo for fn(), is there any way to communicate that the pointer that's implicit in fn() can be used directly as the data pointer in &dyn Foo and that the temporary isn't necessary? I feel like it should be totally fine to coerce fn() into &dyn Foo if there's an impl Foo for fn().

FabianWolff commented 3 years ago

Oh, right, fn itself is a pointer here, so &fn is a pointer to a pointer, and the question is where that intermediate pointer is stored. Makes me wonder how the compiler makes the cast work in the first place — it can't just be a constant, since that doesn't have a memory address. I'm guessing it's probably an anonymous static then, which is also weird that the cast can generate implicitly.

I think constants do have memory addresses, just not necessarily stable ones. See the reference:

References to the same constant are not necessarily guaranteed to refer to the same memory address.

But they need to have some address, because you can create references to them.


Stepping back for a second, I also wonder what the right way to support the "fn as dyn" use-case is. Does it just fundamentally require the extra indirection? That is, if I want &dyn Foo where there's an impl Foo for fn(), is there any way to communicate that the pointer that's implicit in fn() can be used directly as the data pointer in &dyn Foo and that the temporary isn't necessary? I feel like it should be totally fine to coerce fn() into &dyn Foo if there's an impl Foo for fn().

What is the use case here? I can only imagine that you might want to add a method m to Foo that calls the function/data pointer, but then you already have one indirection for the call to m via the vtable, so another indirection for the function pointer does not seem prohibitive, or am I missing something here? You can't call the &dyn Foo directly, obviously, because in general it's not known that it points to a function.

jonhoo commented 3 years ago

I think constants do have memory addresses, just not necessarily stable ones. See the reference:

References to the same constant are not necessarily guaranteed to refer to the same memory address.

But they need to have some address, because you can create references to them.

I think that's only in the contexts of other constants referring to them, as suggested by the following text:

Constants may refer to the address of other constants

This ties back to the fact that constants are inlined everywhere:

Constants are essentially inlined wherever they are used, meaning that they are copied directly into the relevant context when used.

Which means that while I suppose they may have an address in any given place, they don't have one in general. At least I don't see how they could?


What is the use case here? I can only imagine that you might want to add a method m to Foo that calls the function/data pointer, but then you already have one indirection for the call to m via the vtable, so another indirection for the function pointer does not seem prohibitive, or am I missing something here?

It's not prohibitive really, though very slightly unfortunate from a performance perspective. I'm more looking at this from the point of ergonomics — it seems unfortunate to have to explicitly coerce a function definition to a function pointer, and to have to coerce a function pointer to a fat pointer for a given dyn Trait, when both of those operations are, as far as I can tell "straightforward". For the latter case, for any function pointer f, turning it into a fat pointer should just be a matter of setting the data value to f and the vtable to the appropriate vtable for the impl Trait for fn.

FabianWolff commented 3 years ago

Thanks for the interesting discussion by the way, @jonhoo!

Constants are essentially inlined wherever they are used, meaning that they are copied directly into the relevant context when used.

Which means that while I suppose they may have an address in any given place, they don't have one in general. At least I don't see how they could?

I see your point, but I think we both mean more or less the same thing: Constants are usually inlined, so they conceptually don't have an address in memory, but if you take a reference to a constant, it has to point somewhere, so the compiler will create some (conceptually) ad-hoc storage location for the constant and make the reference point to that, and if you take another reference to the same constant, it may or may not point to the same storage location.

What is the use case here? I can only imagine that you might want to add a method m to Foo that calls the function/data pointer, but then you already have one indirection for the call to m via the vtable, so another indirection for the function pointer does not seem prohibitive, or am I missing something here?

It's not prohibitive really, though very slightly unfortunate from a performance perspective. I'm more looking at this from the point of ergonomics — it seems unfortunate to have to explicitly coerce a function definition to a function pointer, and to have to coerce a function pointer to a fat pointer for a given dyn Trait, when both of those operations are, as far as I can tell "straightforward". For the latter case, for any function pointer f, turning it into a fat pointer should just be a matter of setting the data value to f and the vtable to the appropriate vtable for the impl Trait for fn.

I think there may be another reason why the data pointer can't be the function pointer directly (i.e. there has to be an indirection): Storing the function pointer directly in the fat pointer would mean passing it by-value, i.e. the self argument of any called trait method would have to be passed by-value as well (because Self is the function pointer type), but you cannot call such functions through a fat pointer:

error[E0161]: cannot move a value of type dyn Foo: the size of dyn Foo cannot be statically determined

Also, of course, if you call a trait method that takes a &mut self, you need the indirection to be able to observe side-effects on self:

fn a() { println!("a"); }
fn b() { println!("b"); }

trait Foo { fn foo(&mut self); }
impl Foo for fn() {
    fn foo(&mut self) {
        *self = b;
    }
}

fn main() {
    let mut fn_ptr = a as fn();
    (&mut fn_ptr as &mut dyn Foo).foo();
    fn_ptr(); // prints "b"
}
jonhoo commented 3 years ago

Likewise!

Hmm, yeah, that's a good point that the caller expects to get a reference to a function pointer, and not "just" a function pointer. I suppose in my case the way to work around that would be to have the trait take self and implement it for &mut ..., though that does feel like a bit of a hack. Makes me wonder even more how the cast even makes this work though!

jonhoo commented 2 years ago

Hit a few more examples of these errors being relatively unhelpful yesterday (playground):

trait Foo {
    fn bar(&mut self);
}

impl Foo for fn() {
    fn bar(&mut self) {
        (*self)()
    }
}

fn takes_foo(_: impl Foo) {}

fn is_foo() {}

fn main() {
    takes_foo(is_foo);
    // this gives the error
    //
    //     error[E0277]: the trait bound `fn() {is_foo}: Foo` is not satisfied
    //       --> src/main.rs:16:15
    //        |
    //     18 |     takes_foo(is_foo);
    //        |     --------- ^^^^^^ the trait `Foo` is not implemented for `fn() {is_foo}`
    //        |     |
    //        |     required by a bound introduced by this call
    //        |
    //        = help: the following implementations were found:
    //                  <fn() as Foo>
    //
    // but the help doesn't make it clear that the `{is_foo}` part is relevant,
    // and that this can be fixed with a cast to a FnPtr:
    takes_foo(is_foo as fn());

    takes_foo(|| {});
    // this gives the error
    //
    //     error[E0277]: the trait bound `[closure@src/main.rs:34:15: 34:20]: Foo` is not satisfied
    //       --> src/main.rs:34:5
    //        |
    //     33 |     takes_foo(|_: &()| true);
    //        |     ^^^^^^^^^ the trait `Foo` is not implemented for `[closure@src/main.rs:34:15: 34:20]`
    //
    // which doesn't even mention that the closure can be cast to a FnPtr, which
    // _would_ satisfy the bound:
    takes_foo((|| {}) as fn());
    // (note also the extra () needed to avoid the cast being applied to `{}`)
}
bkolobara commented 2 years ago

I have also been struggling with this issue for some time now.

My use case is a bit specific, I use fn pointers to pass functions between a WebAssembly guest and host environment. It's the only way to get a reference to a guest function.

impl IntoFnPtr<Self> for fn(i32) {
    type Type = fn(i32);

    fn into_ptr(self) -> FnPtr<Self::Type> {
        let table_index = self as usize; // The Wasm table index belonging to this function.
        FnPtr(Phant table_index omData)
    }
}

I can't use Fn traits, because you can't turn a closure into a Wasm table index. This only works with function pointers.

But, because this coercion doesn't happen automatically the whole codebase is full of * as fn(a bunch of arguments) -> (and return types) and the ergonomics around this just suck.