From 39745eb0696cb0dfdd86e5c9544571f0827bb83f Mon Sep 17 00:00:00 2001 From: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Date: Sat, 29 Mar 2025 14:01:53 -0700 Subject: [PATCH] bevy_reflect: Fix `TypePath` string concatenation (#18609) # Objective Fixes #18606 When a type implements `Add` for `String`, the compiler can get confused when attempting to add a `&String` to a `String`. Unfortunately, this seems to be [expected behavior](https://github.com/rust-lang/rust/issues/77143#issuecomment-698369286) which causes problems for generic types since the current `TypePath` derive generates code that appends strings in this manner. ## Solution Explicitly use the `Add<&str>` implementation in the `TypePath` derive macro. ## Testing You can test locally by running: ``` cargo check -p bevy_reflect --tests ``` --- crates/bevy_reflect/derive/src/string_expr.rs | 2 +- crates/bevy_reflect/src/lib.rs | 35 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/crates/bevy_reflect/derive/src/string_expr.rs b/crates/bevy_reflect/derive/src/string_expr.rs index cc48a90b91..dc878f39a9 100644 --- a/crates/bevy_reflect/derive/src/string_expr.rs +++ b/crates/bevy_reflect/derive/src/string_expr.rs @@ -80,7 +80,7 @@ impl StringExpr { let owned = self.into_owned(); let borrowed = other.into_borrowed(); Self::Owned(quote! { - #owned + #borrowed + ::core::ops::Add::<&str>::add(#owned, #borrowed) }) } } diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 64d07513ea..31540a54d2 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -988,6 +988,41 @@ mod tests { assert_eq!(values, vec![1]); } + /// This test ensures that we are able to reflect generic types with one or more type parameters. + /// + /// When there is an `Add` implementation for `String`, the compiler isn't able to infer the correct + /// type to deref to. + /// If we don't append the strings in the `TypePath` derive correctly (i.e. explicitly specifying the type), + /// we'll get a compilation error saying that "`&String` cannot be added to `String`". + /// + /// So this test just ensures that we do do that correctly. + /// + /// This problem is a known issue and is unexpectedly expected behavior: + /// - + /// - + /// - + #[test] + fn should_reflect_generic() { + struct FakeString {} + + // This implementation confuses the compiler when trying to add a `&String` to a `String` + impl core::ops::Add for String { + type Output = Self; + fn add(self, _rhs: FakeString) -> Self::Output { + unreachable!() + } + } + + #[derive(Reflect)] + struct Foo(A); + + #[derive(Reflect)] + struct Bar(A, B); + + #[derive(Reflect)] + struct Baz(A, B, C); + } + #[test] fn should_reflect_clone() { // Struct