Merge pull request #1142 from Yarwin/document-type-inference-for-dyngd

Document `DynGd<_, D>` type inference.

Author: Bromeon

godot-core/src/obj/dyn_gd.rs

@@ -155,11 +155,75 @@ use std::{fmt, ops};
 ///
 /// `#[export]` for a `DynGd<T, D>` allows you to limit the available choices to implementors of a given trait `D` whose base inherits the specified `T`
 /// (for example, `#[export] Option<DynGd<Resource, dyn MyTrait>>` won't include Rust classes with an Object base, even if they implement `MyTrait`).
+///
+/// # Type inference
+///
+/// If a class implements more than one `AsDyn<D>` relation (usually via `#[godot_dyn]`), type inference will only work when the trait
+/// used for `D` explicitly declares a `: 'static` bound.
+/// Otherwise, if only one `impl AsDyn` is present for a given class, the type can always be inferred.
+///
+/// ```no_run
+/// # use godot::prelude::*;
+/// trait Health: 'static { /* ... */ }
+///
+/// // Exact equivalent to:
+/// trait OtherHealth
+/// where
+///     Self: 'static
+/// { /* ... */ }
+///
+/// trait NoInference { /* ... */ }
+///
+/// #[derive(GodotClass)]
+/// # #[class(init)]
+/// struct Monster { /* ... */ }
+///
+/// #[godot_dyn]
+/// impl Health for Monster { /* ... */ }
+///
+/// #[godot_dyn]
+/// impl NoInference for Monster { /* ... */ }
+///
+/// // Two example functions accepting trait object, to check type inference.
+/// fn deal_damage(h: &mut dyn Health) { /* ... */ }
+/// fn no_inference(i: &mut dyn NoInference) { /* ... */ }
+///
+/// // Type can be inferred since 'static bound is explicitly declared for Health trait.
+/// let mut dyn_gd = Monster::new_gd().into_dyn();
+/// deal_damage(&mut *dyn_gd.dyn_bind_mut());
+///
+/// // Otherwise type can't be properly inferred.
+/// let mut dyn_gd = Monster::new_gd().into_dyn::<dyn NoInference>();
+/// no_inference(&mut *dyn_gd.dyn_bind_mut());
+/// ```
+///
+/// ```compile_fail
+/// # use godot::prelude::*;
+/// trait Health { /* ... */ }
+///
+/// trait OtherTrait { /* ... */ }
+///
+/// #[derive(GodotClass)]
+/// # #[class(init)]
+/// struct Monster { /* ... */ }
+/// #[godot_dyn]
+/// impl Health for Monster { /* ... */ }
+/// #[godot_dyn]
+/// impl OtherTrait for Monster { /* ... */ }
+///
+/// fn deal_damage(h: &mut dyn Health) { /* ... */ }
+///
+/// // Type can't be inferred.
+/// // Would result in confusing compilation error
+/// // since compiler would try to enforce 'static *lifetime* (&'static mut ...) on our reference.
+/// let mut dyn_gd = Monster::new_gd().into_dyn();
+/// deal_damage(&mut *dyn_gd.dyn_bind_mut());
+/// ```
 pub struct DynGd<T, D>
 where
     // T does _not_ require AsDyn<D> here. Otherwise, it's impossible to upcast (without implementing the relation for all base classes).
     T: GodotClass,
-    D: ?Sized,
+    D: ?Sized + 'static,
 {
     // Potential optimizations: use single Gd; use Rc/Arc instead of Box+clone; store a downcast fn from Gd<T>; ...
     obj: Gd<T>,
@@ -169,7 +233,7 @@ where
 impl<T, D> DynGd<T, D>
 where
     T: AsDyn<D> + Bounds<Declarer = bounds::DeclUser>,
-    D: ?Sized,
+    D: ?Sized + 'static,
 {
     pub(crate) fn from_gd(gd_instance: Gd<T>) -> Self {
         let erased_obj = Box::new(gd_instance.clone());
@@ -185,7 +249,7 @@ impl<T, D> DynGd<T, D>
 where
     // Again, T deliberately does not require AsDyn<D> here. See above.
     T: GodotClass,
-    D: ?Sized,
+    D: ?Sized + 'static,
 {
     /// Acquires a shared reference guard to the trait object `D`.
     ///
@@ -297,7 +361,7 @@ where
 impl<T, D> DynGd<T, D>
 where
     T: GodotClass + Bounds<Memory = bounds::MemManual>,
-    D: ?Sized,
+    D: ?Sized + 'static,
 {
     /// Destroy the manually-managed Godot object.
     ///
@@ -311,7 +375,7 @@ where
 impl<T, D> Clone for DynGd<T, D>
 where
     T: GodotClass,
-    D: ?Sized,
+    D: ?Sized + 'static,
 {
     fn clone(&self) -> Self {
         Self {
@@ -355,7 +419,7 @@ where
 impl<T, D> ops::Deref for DynGd<T, D>
 where
     T: GodotClass,
-    D: ?Sized,
+    D: ?Sized + 'static,
 {
     type Target = Gd<T>;
 
@@ -367,7 +431,7 @@ where
 impl<T, D> ops::DerefMut for DynGd<T, D>
 where
     T: GodotClass,
-    D: ?Sized,
+    D: ?Sized + 'static,
 {
     fn deref_mut(&mut self) -> &mut Self::Target {
         &mut self.obj
@@ -398,7 +462,10 @@ where
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Type erasure
 
-trait ErasedGd<D: ?Sized> {
+trait ErasedGd<D>
+where
+    D: ?Sized + 'static,
+{
     fn dyn_bind(&self) -> DynGdRef<D>;
     fn dyn_bind_mut(&mut self) -> DynGdMut<D>;
 
@@ -408,7 +475,7 @@ trait ErasedGd<D: ?Sized> {
 impl<T, D> ErasedGd<D> for Gd<T>
 where
     T: AsDyn<D> + Bounds<Declarer = bounds::DeclUser>,
-    D: ?Sized,
+    D: ?Sized + 'static,
 {
     fn dyn_bind(&self) -> DynGdRef<D> {
         DynGdRef::from_guard::<T>(Gd::bind(self))
@@ -549,7 +616,7 @@ where
 impl<T, D> GodotConvert for OnEditor<DynGd<T, D>>
 where
     T: GodotClass,
-    D: ?Sized,
+    D: ?Sized + 'static,
 {
     type Via = Option<<DynGd<T, D> as GodotConvert>::Via>;
 }

godot-core/src/obj/gd.rs

@@ -482,7 +482,7 @@ impl<T: GodotClass> Gd<T> {
     pub fn into_dyn<D>(self) -> DynGd<T, D>
     where
         T: crate::obj::AsDyn<D> + Bounds<Declarer = bounds::DeclUser>,
-        D: ?Sized,
+        D: ?Sized + 'static,
     {
         DynGd::<T, D>::from_gd(self)
     }

godot-core/src/obj/guards.rs

@@ -104,7 +104,10 @@ pub struct DynGdRef<'a, D: ?Sized> {
     cached_ptr: *const D,
 }
 
-impl<'a, D: ?Sized> DynGdRef<'a, D> {
+impl<'a, D> DynGdRef<'a, D>
+where
+    D: ?Sized + 'static,
+{
     #[doc(hidden)]
     pub fn from_guard<T: AsDyn<D>>(guard: GdRef<'a, T>) -> Self {
         let obj = &*guard;
@@ -147,7 +150,10 @@ pub struct DynGdMut<'a, D: ?Sized> {
     cached_ptr: *mut D,
 }
 
-impl<'a, D: ?Sized> DynGdMut<'a, D> {
+impl<'a, D> DynGdMut<'a, D>
+where
+    D: ?Sized + 'static,
+{
     #[doc(hidden)]
     pub fn from_guard<T: AsDyn<D>>(mut guard: GdMut<'a, T>) -> Self {
         let obj = &mut *guard;

godot-core/src/obj/traits.rs

@@ -148,7 +148,10 @@ unsafe impl<T: GodotClass> Inherits<T> for T {}
 // Note: technically, `Trait` doesn't _have to_ implement `Self`. The Rust type system provides no way to verify that a) D is a trait object,
 // and b) that the trait behind it is implemented for the class. Thus, users could any another reference type, such as `&str` pointing to a field.
 // This should be safe, since lifetimes are checked throughout and the class instance remains in place (pinned) inside a DynGd.
-pub trait AsDyn<Trait: ?Sized>: GodotClass {
+pub trait AsDyn<Trait>: GodotClass
+where
+    Trait: ?Sized + 'static,
+{
     fn dyn_upcast(&self) -> &Trait;
     fn dyn_upcast_mut(&mut self) -> &mut Trait;
 }

godot-core/src/registry/callbacks.rs

@@ -192,14 +192,15 @@ pub unsafe extern "C" fn to_string<T: cap::GodotToString>(
     out_string: sys::GDExtensionStringPtr,
 ) {
     // Note: to_string currently always succeeds, as it is only provided for classes that have a working implementation.
-    // is_valid output parameter thus not needed.
 
     let storage = as_storage::<T>(instance);
     let instance = storage.get();
     let string = T::__godot_to_string(&*instance);
 
     // Transfer ownership to Godot
     string.move_into_string_ptr(out_string);
+
+    // Note: is_valid comes uninitialized and must be set.
     *is_valid = sys::conv::SYS_TRUE;
 }
 

itest/rust/src/object_tests/dyn_gd_test.rs

@@ -56,25 +56,47 @@ fn dyn_gd_creation_deref() {
 
 #[itest]
 fn dyn_gd_creation_deref_multiple_traits() {
-    let obj = foreign::NodeHealth::new_alloc();
-    let original_id = obj.instance_id();
+    let original_obj = foreign::NodeHealth::new_alloc();
+    let original_id = original_obj.instance_id();
 
-    // `dyn Health` must be explicitly declared if multiple AsDyn<...> trait implementations exist.
-    let mut obj = obj.into_dyn::<dyn Health>();
+    // Type can be inferred because `Health` explicitly declares a 'static bound.
+    let mut obj = original_obj.clone().into_dyn();
 
     let dyn_id = obj.instance_id();
     assert_eq!(dyn_id, original_id);
 
     deal_20_damage(&mut *obj.dyn_bind_mut());
     assert_eq!(obj.dyn_bind().get_hitpoints(), 80);
 
+    // Otherwise type inference doesn't work and type must be explicitly declared.
+    let mut obj = original_obj
+        .clone()
+        .into_dyn::<dyn InstanceIdProvider<Id = InstanceId>>();
+    assert_eq!(get_instance_id(&mut *obj.dyn_bind_mut()), original_id);
+
+    // Not recommended – for presentational purposes only.
+    // Works because 'static bound on type is enforced in function signature.
+    // I.e. this wouldn't work with fn get_instance_id(...).
+    let mut obj = original_obj.into_dyn();
+    get_instance_id_explicit_static_bound(&mut *obj.dyn_bind_mut());
+
     obj.free();
 }
 
 fn deal_20_damage(h: &mut dyn Health) {
     h.deal_damage(20);
 }
 
+fn get_instance_id(i: &mut dyn InstanceIdProvider<Id = InstanceId>) -> InstanceId {
+    i.get_id_dynamic()
+}
+
+fn get_instance_id_explicit_static_bound(
+    i: &mut (dyn InstanceIdProvider<Id = InstanceId> + 'static),
+) -> InstanceId {
+    i.get_id_dynamic()
+}
+
 #[itest]
 fn dyn_gd_upcast() {
     let original = foreign::NodeHealth::new_alloc();
@@ -428,7 +450,8 @@ fn dyn_gd_multiple_traits() {
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Example symbols
 
-trait Health {
+// 'static bound must be explicitly declared to make type inference work.
+trait Health: 'static {
     fn get_hitpoints(&self) -> u8;
 
     fn deal_damage(&mut self, damage: u8);