bevy/crates/bevy_reflect/compile_fail
Gino Valente 1042f09c2e
bevy_reflect: Add DynamicClosure and DynamicClosureMut (#14141)
# Objective

As mentioned in
[this](https://github.com/bevyengine/bevy/pull/13152#issuecomment-2198387297)
comment, creating a function registry (see #14098) is a bit difficult
due to the requirements of `DynamicFunction`. Internally, a
`DynamicFunction` contains a `Box<dyn FnMut>` (the function that reifies
reflected arguments and calls the actual function), which requires `&mut
self` in order to be called.

This means that users would require a mutable reference to the function
registry for it to be useful— which isn't great. And they can't clone
the `DynamicFunction` either because cloning an `FnMut` isn't really
feasible (wrapping it in an `Arc` would allow it to be cloned but we
wouldn't be able to call the clone since we need a mutable reference to
the `FnMut`, which we can't get with multiple `Arc`s still alive,
requiring us to also slap in a `Mutex`, which adds additional overhead).

And we don't want to just replace the `dyn FnMut` with `dyn Fn` as that
would prevent reflecting closures that mutate their environment.

Instead, we need to introduce a new type to split the requirements of
`DynamicFunction`.

## Solution

Introduce new types for representing closures.

Specifically, this PR introduces `DynamicClosure` and
`DynamicClosureMut`. Similar to how `IntoFunction` exists for
`DynamicFunction`, two new traits were introduced: `IntoClosure` and
`IntoClosureMut`.

Now `DynamicFunction` stores a `dyn Fn` with a `'static` lifetime.
`DynamicClosure` also uses a `dyn Fn` but has a lifetime, `'env`, tied
to its environment. `DynamicClosureMut` is most like the old
`DynamicFunction`, keeping the `dyn FnMut` and also typing its lifetime,
`'env`, to the environment

Here are some comparison tables:

|   | `DynamicFunction` | `DynamicClosure` | `DynamicClosureMut` |
| - | ----------------- | ---------------- | ------------------- |
| Callable with `&self` |  |  |  |
| Callable with `&mut self` |  |  |  |
| Allows for non-`'static` lifetimes |  |  |  |

|   | `IntoFunction` | `IntoClosure` | `IntoClosureMut` |
| - | -------------- | ------------- | ---------------- |
| Convert `fn` functions |  |  |  |
| Convert `fn` methods |  |  |  |
| Convert anonymous functions |  |  |  |
| Convert closures that capture immutable references |  |  |  |
| Convert closures that capture mutable references |  |  |  |
| Convert closures that capture owned values | [^1] |  |  |

[^1]: Due to limitations in Rust, `IntoFunction` can't be implemented
for just functions (unless we forced users to manually coerce them to
function pointers first). So closures that meet the trait requirements
_can technically_ be converted into a `DynamicFunction` as well. To both
future-proof and reduce confusion, though, we'll just pretend like this
isn't a thing.

```rust
let mut list: Vec<i32> = vec![1, 2, 3];

// `replace` is a closure that captures a mutable reference to `list`
let mut replace = |index: usize, value: i32| -> i32 {
  let old_value = list[index];
  list[index] = value;
  old_value
};

// Convert the closure into a dynamic closure using `IntoClosureMut::into_closure_mut`
let mut func: DynamicClosureMut = replace.into_closure_mut();

// Dynamically call the closure:
let args = ArgList::default().push_owned(1_usize).push_owned(-2_i32);
let value = func.call_once(args).unwrap().unwrap_owned();

// Check the result:
assert_eq!(value.take::<i32>().unwrap(), 2);
assert_eq!(list, vec![1, -2, 3]);
```

### `ReflectFn`/`ReflectFnMut`

To make extending the function reflection system easier (the blanket
impls for `IntoFunction`, `IntoClosure`, and `IntoClosureMut` are all
incredibly short), this PR generalizes callables with two new traits:
`ReflectFn` and `ReflectFnMut`.

These traits mimic `Fn` and `FnMut` but allow for being called via
reflection. In fact, their blanket implementations are identical save
for `ReflectFn` being implemented over `Fn` types and `ReflectFnMut`
being implemented over `FnMut` types.

And just as `Fn` is a subtrait of `FnMut`, `ReflectFn` is a subtrait of
`ReflectFnMut`. So anywhere that expects a `ReflectFnMut` can also be
given a `ReflectFn`.

To reiterate, these traits aren't 100% necessary. They were added in
purely for extensibility. If we decide to split things up differently or
add new traits/types in the future, then those changes should be much
simpler to implement.

### `TypedFunction`

Because of the split into `ReflectFn` and `ReflectFnMut`, we needed a
new way to access the function type information. This PR moves that
concept over into `TypedFunction`.

Much like `Typed`, this provides a way to access a function's
`FunctionInfo`.

By splitting this trait out, it helps to ensure the other traits are
focused on a single responsibility.

### Internal Macros

The original function PR (#13152) implemented `IntoFunction` using a
macro which was passed into an `all_tuples!` macro invocation. Because
we needed the same functionality for these new traits, this PR has
copy+pasted that code for `ReflectFn`, `ReflectFnMut`, and
`TypedFunction`— albeit with some differences between them.

Originally, I was going to try and macro-ify the impls and where clauses
such that we wouldn't have to straight up duplicate a lot of this logic.
However, aside from being more complex in general, autocomplete just
does not play nice with such heavily nested macros (tried in both
RustRover and VSCode). And both of those problems told me that it just
wasn't worth it: we need to ensure the crate is easily maintainable,
even at the cost of duplicating code.

So instead, I made sure to simplify the macro code by removing all
fully-qualified syntax and cutting the where clauses down to the bare
essentials, which helps to clean up a lot of the visual noise. I also
tried my best to document the macro logic in certain areas (I may even
add a bit more) to help with maintainability for future devs.

### Documentation

Documentation for this module was a bit difficult for me. So many of
these traits and types are very interconnected. And each trait/type has
subtle differences that make documenting it in a single place, like at
the module level, difficult to do cleanly. Describing the valid
signatures is also challenging to do well.

Hopefully what I have here is okay. I think I did an okay job, but let
me know if there any thoughts on ways to improve it. We can also move
such a task to a followup PR for more focused discussion.

## Testing

You can test locally by running:

```
cargo test --package bevy_reflect
```

---

## Changelog

- Added `DynamicClosure` struct
- Added `DynamicClosureMut` struct
- Added `IntoClosure` trait
- Added `IntoClosureMut` trait
- Added `ReflectFn` trait
- Added `ReflectFnMut` trait
- Added `TypedFunction` trait
- `IntoFunction` now only works for standard Rust functions
- `IntoFunction` no longer takes a lifetime parameter
- `DynamicFunction::call` now only requires `&self`
- Removed `DynamicFunction::call_once`
- Changed the `IntoReturn::into_return` signature to include a where
clause

## Internal Migration Guide

> [!important]
> Function reflection was introduced as part of the 0.15 dev cycle. This
migration guide was written for developers relying on `main` during this
cycle, and is not a breaking change coming from 0.14.

### `IntoClosure`

`IntoFunction` now only works for standard Rust functions. Calling
`IntoFunction::into_function` on a closure that captures references to
its environment (either mutable or immutable), will no longer compile.

Instead, you will need to use either `IntoClosure::into_closure` to
create a `DynamicClosure` or `IntoClosureMut::into_closure_mut` to
create a `DynamicClosureMut`, depending on your needs:

```rust
let punct = String::from("!");
let print = |value: String| {
    println!("{value}{punct}");
};

// BEFORE
let func: DynamicFunction = print.into_function();

// AFTER
let func: DynamicClosure = print.into_closure();
```

### `IntoFunction` lifetime

Additionally, `IntoFunction` no longer takes a lifetime parameter as it
always expects a `'static` lifetime. Usages will need to remove any
lifetime parameters:

```rust
// BEFORE
fn execute<'env, F: IntoFunction<'env, Marker>, Marker>(f: F) {/* ... */}

// AFTER
fn execute<F: IntoFunction<Marker>, Marker>(f: F) {/* ... */}
```

### `IntoReturn`

`IntoReturn::into_return` now has a where clause. Any manual
implementors will need to add this where clause to their implementation.
2024-07-16 03:22:43 +00:00
..
src Move compile fail tests (#13196) 2024-05-03 13:35:21 +00:00
tests bevy_reflect: Add DynamicClosure and DynamicClosureMut (#14141) 2024-07-16 03:22:43 +00:00
.gitignore Move compile fail tests (#13196) 2024-05-03 13:35:21 +00:00
Cargo.toml bevy_reflect: Feature-gate function reflection (#14174) 2024-07-14 15:55:31 +00:00
README.md Move compile fail tests (#13196) 2024-05-03 13:35:21 +00:00

Compile fail tests for bevy_reflect

This crate is separate from bevy_reflect and not part of the Bevy workspace in order to not fail crater tests for Bevy. The tests assert on the exact compiler errors and can easily fail for new Rust versions due to updated compiler errors (e.g. changes in spans).

The CI workflow executes these tests on the stable rust toolchain (see tools/ci).

For information on writing tests see compile_fail_utils/README.md.