2023-01-02 21:07:33 +00:00
|
|
|
[package]
|
Move compile fail tests (#13196)
# Objective
- Follow-up of #13184 :)
- We use `ui_test` to test compiler errors for our custom macros.
- There are four crates related to compile fail tests
- `bevy_ecs_compile_fail_tests`, `bevy_macros_compile_fail_tests`, and
`bevy_reflect_compile_fail_tests`, which actually test the macros.
-
[`bevy_compile_test_utils`](https://github.com/bevyengine/bevy/tree/64c1c65783938facc59d9b36cbaa6deba435d84e/crates/bevy_compile_test_utils),
which provides helpers and common patterns for these tests.
- All of these crates reside within the `crates` directory.
- This can be confusing, especially for newcomers. All of the other
folders in `crates` are actual published libraries, except for these 4.
## Solution
- Move all compile fail tests to a `compile_fail` folder under their
corresponding crate.
- E.g. `crates/bevy_ecs_compile_fail_tests` would be moved to
`crates/bevy_ecs/compile_fail`.
- Move `bevy_compile_test_utils` to `tools/compile_fail_utils`.
There are a few benefits to this approach:
1. An internal testing detail is less intrusive (and confusing) for
those who just want to browse the public Bevy interface.
2. Follows a pre-existing approach of organizing related crates inside a
larger crate's folder.
- See `bevy_gizmos/macros` for an example.
4. Makes consistent the terms `compile_test`, `compile_fail`, and
`compile_fail_test` in code. It's all just `compile_fail` now, because
we are specifically testing the error messages on compiler failures.
- To be clear it can still be referred to by these terms in comments and
speech, just the names of the crates and the CI command are now
consistent.
## Testing
Run the compile fail CI command:
```shell
cargo run -p ci -- compile-fail
```
If it still passes, then my refactor was successful.
2024-05-03 13:35:21 +00:00
|
|
|
name = "bevy_reflect_compile_fail"
|
2023-01-02 21:07:33 +00:00
|
|
|
edition = "2021"
|
|
|
|
|
description = "Compile fail tests for Bevy Engine's reflection system"
|
|
|
|
|
homepage = "https://bevyengine.org"
|
|
|
|
|
repository = "https://github.com/bevyengine/bevy"
|
|
|
|
|
license = "MIT OR Apache-2.0"
|
|
|
|
|
publish = false
|
|
|
|
|
|
2024-04-27 00:00:57 +00:00
|
|
|
[dependencies]
|
2024-07-14 15:55:31 +00:00
|
|
|
bevy_reflect = { path = "../", features = ["functions"] }
|
2024-04-27 00:00:57 +00:00
|
|
|
|
2023-01-02 21:07:33 +00:00
|
|
|
[dev-dependencies]
|
Move compile fail tests (#13196)
# Objective
- Follow-up of #13184 :)
- We use `ui_test` to test compiler errors for our custom macros.
- There are four crates related to compile fail tests
- `bevy_ecs_compile_fail_tests`, `bevy_macros_compile_fail_tests`, and
`bevy_reflect_compile_fail_tests`, which actually test the macros.
-
[`bevy_compile_test_utils`](https://github.com/bevyengine/bevy/tree/64c1c65783938facc59d9b36cbaa6deba435d84e/crates/bevy_compile_test_utils),
which provides helpers and common patterns for these tests.
- All of these crates reside within the `crates` directory.
- This can be confusing, especially for newcomers. All of the other
folders in `crates` are actual published libraries, except for these 4.
## Solution
- Move all compile fail tests to a `compile_fail` folder under their
corresponding crate.
- E.g. `crates/bevy_ecs_compile_fail_tests` would be moved to
`crates/bevy_ecs/compile_fail`.
- Move `bevy_compile_test_utils` to `tools/compile_fail_utils`.
There are a few benefits to this approach:
1. An internal testing detail is less intrusive (and confusing) for
those who just want to browse the public Bevy interface.
2. Follows a pre-existing approach of organizing related crates inside a
larger crate's folder.
- See `bevy_gizmos/macros` for an example.
4. Makes consistent the terms `compile_test`, `compile_fail`, and
`compile_fail_test` in code. It's all just `compile_fail` now, because
we are specifically testing the error messages on compiler failures.
- To be clear it can still be referred to by these terms in comments and
speech, just the names of the crates and the CI command are now
consistent.
## Testing
Run the compile fail CI command:
```shell
cargo run -p ci -- compile-fail
```
If it still passes, then my refactor was successful.
2024-05-03 13:35:21 +00:00
|
|
|
compile_fail_utils = { path = "../../../tools/compile_fail_utils" }
|
2024-04-27 00:00:57 +00:00
|
|
|
|
|
|
|
|
[[test]]
|
|
|
|
|
name = "derive"
|
|
|
|
|
harness = false
|
bevy_reflect: Function reflection (#13152)
# Objective
We're able to reflect types sooooooo... why not functions?
The goal of this PR is to make functions callable within a dynamic
context, where type information is not readily available at compile
time.
For example, if we have a function:
```rust
fn add(left: i32, right: i32) -> i32 {
left + right
}
```
And two `Reflect` values we've already validated are `i32` types:
```rust
let left: Box<dyn Reflect> = Box::new(2_i32);
let right: Box<dyn Reflect> = Box::new(2_i32);
```
We should be able to call `add` with these values:
```rust
// ?????
let result: Box<dyn Reflect> = add.call_dynamic(left, right);
```
And ideally this wouldn't just work for functions, but methods and
closures too!
Right now, users have two options:
1. Manually parse the reflected data and call the function themselves
2. Rely on registered type data to handle the conversions for them
For a small function like `add`, this isn't too bad. But what about for
more complex functions? What about for many functions?
At worst, this process is error-prone. At best, it's simply tedious.
And this is assuming we know the function at compile time. What if we
want to accept a function dynamically and call it with our own
arguments?
It would be much nicer if `bevy_reflect` could alleviate some of the
problems here.
## Solution
Added function reflection!
This adds a `DynamicFunction` type to wrap a function dynamically. This
can be called with an `ArgList`, which is a dynamic list of
`Reflect`-containing `Arg` arguments. It returns a `FunctionResult`
which indicates whether or not the function call succeeded, returning a
`Reflect`-containing `Return` type if it did succeed.
Many functions can be converted into this `DynamicFunction` type thanks
to the `IntoFunction` trait.
Taking our previous `add` example, this might look something like
(explicit types added for readability):
```rust
fn add(left: i32, right: i32) -> i32 {
left + right
}
let mut function: DynamicFunction = add.into_function();
let args: ArgList = ArgList::new().push_owned(2_i32).push_owned(2_i32);
let result: Return = function.call(args).unwrap();
let value: Box<dyn Reflect> = result.unwrap_owned();
assert_eq!(value.take::<i32>().unwrap(), 4);
```
And it also works on closures:
```rust
let add = |left: i32, right: i32| left + right;
let mut function: DynamicFunction = add.into_function();
let args: ArgList = ArgList::new().push_owned(2_i32).push_owned(2_i32);
let result: Return = function.call(args).unwrap();
let value: Box<dyn Reflect> = result.unwrap_owned();
assert_eq!(value.take::<i32>().unwrap(), 4);
```
As well as methods:
```rust
#[derive(Reflect)]
struct Foo(i32);
impl Foo {
fn add(&mut self, value: i32) {
self.0 += value;
}
}
let mut foo = Foo(2);
let mut function: DynamicFunction = Foo::add.into_function();
let args: ArgList = ArgList::new().push_mut(&mut foo).push_owned(2_i32);
function.call(args).unwrap();
assert_eq!(foo.0, 4);
```
### Limitations
While this does cover many functions, it is far from a perfect system
and has quite a few limitations. Here are a few of the limitations when
using `IntoFunction`:
1. The lifetime of the return value is only tied to the lifetime of the
first argument (useful for methods). This means you can't have a
function like `(a: i32, b: &i32) -> &i32` without creating the
`DynamicFunction` manually.
2. Only 15 arguments are currently supported. If the first argument is a
(mutable) reference, this number increases to 16.
3. Manual implementations of `Reflect` will need to implement the new
`FromArg`, `GetOwnership`, and `IntoReturn` traits in order to be used
as arguments/return types.
And some limitations of `DynamicFunction` itself:
1. All arguments share the same lifetime, or rather, they will shrink to
the shortest lifetime.
2. Closures that capture their environment may need to have their
`DynamicFunction` dropped before accessing those variables again (there
is a `DynamicFunction::call_once` to make this a bit easier)
3. All arguments and return types must implement `Reflect`. While not a
big surprise coming from `bevy_reflect`, this implementation could
actually still work by swapping `Reflect` out with `Any`. Of course,
that makes working with the arguments and return values a bit harder.
4. Generic functions are not supported (unless they have been manually
monomorphized)
And general, reflection gotchas:
1. `&str` does not implement `Reflect`. Rather, `&'static str`
implements `Reflect` (the same is true for `&Path` and similar types).
This means that `&'static str` is considered an "owned" value for the
sake of generating arguments. Additionally, arguments and return types
containing `&str` will assume it's `&'static str`, which is almost never
the desired behavior. In these cases, the only solution (I believe) is
to use `&String` instead.
### Followup Work
This PR is the first of two PRs I intend to work on. The second PR will
aim to integrate this new function reflection system into the existing
reflection traits and `TypeInfo`. The goal would be to register and call
a reflected type's methods dynamically.
I chose not to do that in this PR since the diff is already quite large.
I also want the discussion for both PRs to be focused on their own
implementation.
Another followup I'd like to do is investigate allowing common container
types as a return type, such as `Option<&[mut] T>` and `Result<&[mut] T,
E>`. This would allow even more functions to opt into this system. I
chose to not include it in this one, though, for the same reasoning as
previously mentioned.
### Alternatives
One alternative I had considered was adding a macro to convert any
function into a reflection-based counterpart. The idea would be that a
struct that wraps the function would be created and users could specify
which arguments and return values should be `Reflect`. It could then be
called via a new `Function` trait.
I think that could still work, but it will be a fair bit more involved,
requiring some slightly more complex parsing. And it of course is a bit
more work for the user, since they need to create the type via macro
invocation.
It also makes registering these functions onto a type a bit more
complicated (depending on how it's implemented).
For now, I think this is a fairly simple, yet powerful solution that
provides the least amount of friction for users.
---
## Showcase
Bevy now adds support for storing and calling functions dynamically
using reflection!
```rust
// 1. Take a standard Rust function
fn add(left: i32, right: i32) -> i32 {
left + right
}
// 2. Convert it into a type-erased `DynamicFunction` using the `IntoFunction` trait
let mut function: DynamicFunction = add.into_function();
// 3. Define your arguments from reflected values
let args: ArgList = ArgList::new().push_owned(2_i32).push_owned(2_i32);
// 4. Call the function with your arguments
let result: Return = function.call(args).unwrap();
// 5. Extract the return value
let value: Box<dyn Reflect> = result.unwrap_owned();
assert_eq!(value.take::<i32>().unwrap(), 4);
```
## Changelog
#### TL;DR
- Added support for function reflection
- Added a new `Function Reflection` example:
https://github.com/bevyengine/bevy/blob/ba727898f2adff817838fc4cdb49871bbce37356/examples/reflection/function_reflection.rs#L1-L157
#### Details
Added the following items:
- `ArgError` enum
- `ArgId` enum
- `ArgInfo` struct
- `ArgList` struct
- `Arg` enum
- `DynamicFunction` struct
- `FromArg` trait (derived with `derive(Reflect)`)
- `FunctionError` enum
- `FunctionInfo` struct
- `FunctionResult` alias
- `GetOwnership` trait (derived with `derive(Reflect)`)
- `IntoFunction` trait (with blanket implementation)
- `IntoReturn` trait (derived with `derive(Reflect)`)
- `Ownership` enum
- `ReturnInfo` struct
- `Return` enum
---------
Co-authored-by: Periwink <charlesbour@gmail.com>
2024-07-01 13:49:08 +00:00
|
|
|
|
|
|
|
|
[[test]]
|
|
|
|
|
name = "func"
|
|
|
|
|
harness = false
|
bevy_reflect: Reflect remote types (#6042)
# Objective
The goal with this PR is to allow the use of types that don't implement
`Reflect` within the reflection API.
Rust's [orphan
rule](https://doc.rust-lang.org/book/ch10-02-traits.html#implementing-a-trait-on-a-type)
prevents implementing a trait on an external type when neither type nor
trait are owned by the implementor. This means that if a crate,
`cool_rust_lib`, defines a type, `Foo`, then a user cannot use it with
reflection. What this means is that we have to ignore it most of the
time:
```rust
#[derive(Reflect)]
struct SomeStruct {
#[reflect(ignore)]
data: cool_rust_lib::Foo
}
```
Obviously, it's impossible to implement `Reflect` on `Foo`. But does it
*have* to be?
Most of reflection doesn't deal with concrete types— it's almost all
using `dyn Reflect`. And being very metadata-driven, it should
theoretically be possible. I mean,
[`serde`](https://serde.rs/remote-derive.html) does it.
## Solution
> Special thanks to @danielhenrymantilla for their help reviewing this
PR and offering wisdom wrt safety.
Taking a page out of `serde`'s book, this PR adds the ability to easily
use "remote types" with reflection. In this context, a "remote type" is
the external type for which we have no ability to implement `Reflect`.
This adds the `#[reflect_remote(...)]` attribute macro, which is used to
generate "remote type wrappers". All you have to do is define the
wrapper exactly the same as the remote type's definition:
```rust
// Pretend this is our external crate
mod cool_rust_lib {
#[derive(Default)]
struct Foo {
pub value: String
}
}
#[reflect_remote(cool_rust_lib::Foo)]
struct FooWrapper {
pub value: String
}
```
> **Note:** All fields in the external type *must* be public. This could
be addressed with a separate getter/setter attribute either in this PR
or in another one.
The macro takes this user-defined item and transforms it into a newtype
wrapper around the external type, marking it as `#[repr(transparent)]`.
The fields/variants defined by the user are simply used to build out the
reflection impls.
Additionally, it generates an implementation of the new trait,
`ReflectRemote`, which helps prevent accidental misuses of this API.
Therefore, the output generated by the macro would look something like:
```rust
#[repr(transparent)]
struct FooWrapper(pub cool_rust_lib::Foo);
impl ReflectRemote for FooWrapper {
type Remote = cool_rust_lib::Foo;
// transmutation methods...
}
// reflection impls...
// these will acknowledge and make use of the `value` field
```
Internally, the reflection API will pass around the `FooWrapper` and
[transmute](https://doc.rust-lang.org/std/mem/fn.transmute.html) it
where necessary. All we have to do is then tell `Reflect` to do that. So
rather than ignoring the field, we tell `Reflect` to use our wrapper
using the `#[reflect(remote = ...)]` field attribute:
```rust
#[derive(Reflect)]
struct SomeStruct {
#[reflect(remote = FooWrapper)]
data: cool_rust_lib::Foo
}
```
#### Other Macros & Type Data
Because this macro consumes the defined item and generates a new one, we
can't just put our macros anywhere. All macros that should be passed to
the generated struct need to come *below* this macro. For example, to
derive `Default` and register its associated type data:
```rust
// ✅ GOOD
#[reflect_remote(cool_rust_lib::Foo)]
#[derive(Default)]
#[reflect(Default)]
struct FooWrapper {
pub value: String
}
// ❌ BAD
#[derive(Default)]
#[reflect_remote(cool_rust_lib::Foo)]
#[reflect(Default)]
struct FooWrapper {
pub value: String
}
```
#### Generics
Generics are forwarded to the generated struct as well. They should also
be defined in the same order:
```rust
#[reflect_remote(RemoteGeneric<'a, T1, T2>)]
struct GenericWrapper<'a, T1, T2> {
pub foo: &'a T1,
pub bar: &'a T2,
}
```
> Naming does *not* need to match the original definition's. Only order
matters here.
> Also note that the code above is just a demonstration and doesn't
actually compile since we'd need to enforce certain bounds (e.g. `T1:
Reflect`, `'a: 'static`, etc.)
#### Nesting
And, yes, you can nest remote types:
```rust
#[reflect_remote(RemoteOuter)]
struct OuterWrapper {
#[reflect(remote = InnerWrapper)]
pub inner: RemoteInner
}
#[reflect_remote(RemoteInner)]
struct InnerWrapper(usize);
```
#### Assertions
This macro will also generate some compile-time assertions to ensure
that the correct types are used. It's important we catch this early so
users don't have to wait for something to panic. And it also helps keep
our `unsafe` a little safer.
For example, a wrapper definition that does not match its corresponding
remote type will result in an error:
```rust
mod external_crate {
pub struct TheirStruct(pub u32);
}
#[reflect_remote(external_crate::TheirStruct)]
struct MyStruct(pub String); // ERROR: expected type `u32` but found `String`
```
<details>
<summary>Generated Assertion</summary>
```rust
const _: () = {
#[allow(non_snake_case)]
#[allow(unused_variables)]
#[allow(unused_assignments)]
#[allow(unreachable_patterns)]
#[allow(clippy::multiple_bound_locations)]
fn assert_wrapper_definition_matches_remote_type(
mut __remote__: external_crate::TheirStruct,
) {
__remote__.0 = (|| -> ::core::option::Option<String> { None })().unwrap();
}
};
```
</details>
Additionally, using the incorrect type in a `#[reflect(remote = ...)]`
attribute should result in an error:
```rust
mod external_crate {
pub struct TheirFoo(pub u32);
pub struct TheirBar(pub i32);
}
#[reflect_remote(external_crate::TheirFoo)]
struct MyFoo(pub u32);
#[reflect_remote(external_crate::TheirBar)]
struct MyBar(pub i32);
#[derive(Reflect)]
struct MyStruct {
#[reflect(remote = MyBar)] // ERROR: expected type `TheirFoo` but found struct `TheirBar`
foo: external_crate::TheirFoo
}
```
<details>
<summary>Generated Assertion</summary>
```rust
const _: () = {
struct RemoteFieldAssertions;
impl RemoteFieldAssertions {
#[allow(non_snake_case)]
#[allow(clippy::multiple_bound_locations)]
fn assert__foo__is_valid_remote() {
let _: <MyBar as bevy_reflect::ReflectRemote>::Remote = (|| -> ::core::option::Option<external_crate::TheirFoo> {
None
})().unwrap();
}
}
};
```
</details>
### Discussion
There are a couple points that I think still need discussion or
validation.
- [x] 1. `Any` shenanigans
~~If we wanted to downcast our remote type from a `dyn Reflect`, we'd
have to first downcast to the wrapper then extract the inner type. This
PR has a [commit](b840db9f74cb6d357f951cb11b150d46bac89ee2) that
addresses this by making all the `Reflect::*any` methods return the
inner type rather than the wrapper type. This allows us to downcast
directly to our remote type.~~
~~However, I'm not sure if this is something we want to do. For
unknowing users, it could be confusing and seemingly inconsistent. Is it
worth keeping? Or should this behavior be removed?~~
I think this should be fine. The remote wrapper is an implementation
detail and users should not need to downcast to the wrapper type. Feel
free to let me know if there are other opinions on this though!
- [x] 2. Implementing `Deref/DerefMut` and `From`
~~We don't currently do this, but should we implement other traits on
the generated transparent struct? We could implement `Deref`/`DerefMut`
to easily access the inner type. And we could implement `From` for
easier conversion between the two types (e.g. `T: Into<Foo>`).~~ As
mentioned in the comments, we probably don't need to do this. Again, the
remote wrapper is an implementation detail, and should generally not be
used directly.
- [x] 3. ~~Should we define a getter/setter field attribute in this PR
as well or leave it for a future one?~~ I think this should be saved for
a future PR
- [ ] 4. Any foreseeable issues with this implementation?
#### Alternatives
One alternative to defining our own `ReflectRemote` would be to use
[bytemuck's
`TransparentWrapper`](https://docs.rs/bytemuck/1.13.1/bytemuck/trait.TransparentWrapper.html)
(as suggested by @danielhenrymantilla).
This is definitely a viable option, as `ReflectRemote` is pretty much
the same thing as `TransparentWrapper`. However, the cost would be
bringing in a new crate— though, it is already in use in a few other
sub-crates like bevy_render.
I think we're okay just defining `ReflectRemote` ourselves, but we can
go the bytemuck route if we'd prefer offloading that work to another
crate.
---
## Changelog
* Added the `#[reflect_remote(...)]` attribute macro to allow `Reflect`
to be used on remote types
* Added `ReflectRemote` trait for ensuring proper remote wrapper usage
2024-08-12 19:12:53 +00:00
|
|
|
|
|
|
|
|
[[test]]
|
|
|
|
|
name = "remote"
|
|
|
|
|
harness = false
|
