Add PassiveGd as clone_weak/drop_weak RAII struct

Author: Bromeon

godot-core/src/obj/base.rs

@@ -15,7 +15,7 @@ use std::mem::ManuallyDrop;
 use std::rc::Rc;
 
 use crate::builtin::{Callable, Variant};
-use crate::obj::{bounds, Gd, GodotClass, InstanceId};
+use crate::obj::{bounds, Gd, GodotClass, InstanceId, PassiveGd};
 use crate::{classes, sys};
 
 thread_local! {
@@ -305,17 +305,16 @@ impl<T: GodotClass> Base<T> {
         self.obj.instance_id()
     }
 
-    /// Returns a [`Gd`] referencing the base object, for use in script contexts only.
-    pub(crate) fn to_script_gd(&self) -> Gd<T> {
+    /// Returns a passive reference to the base object, for use in script contexts only.
+    pub(crate) fn to_script_passive(&self) -> PassiveGd<'_, T> {
         #[cfg(debug_assertions)]
         assert_eq!(
             self.init_state.get(),
             InitState::Script,
-            "to_script_gd() can only be called on script-context Base objects"
+            "to_script_passive() can only be called on script-context Base objects"
         );
 
-        // SAFETY: ScriptBase{Ref,Mut} also drop weakly.
-        unsafe { (*self.obj).clone_weak() }
+        PassiveGd::from_strong_ref(&self.obj)
     }
 
     /// Returns `true` if this `Base<T>` is currently in the initializing state.
@@ -336,20 +335,35 @@ impl<T: GodotClass> Base<T> {
         (*self.obj).clone()
     }
 
+    /// Returns a [`PassiveGd`] referencing the base object, assuming the derived object is fully constructed.
+    ///
+    /// This method directly creates a PassiveGd from a weak reference, providing clean lifetime management
+    /// without the need for manual `drop_weak()` calls.
+    pub(crate) fn constructed_passive(&self) -> PassiveGd<'_, T> {
+        // SAFETY: returned lifetime here is re-bound to self. Covariant lifetime conversion 'static -> 'self.
+        unsafe { self.constructed_passive_unbounded() }
+    }
+
     /// Returns a weak [`Gd`] referencing the base object, assuming the derived object is fully constructed.
     ///
-    /// Unlike `__constructed_gd()`, this does not increment the reference count for ref-counted `T`s.
+    /// Unlike [`Self::__constructed_gd()`], this does not increment the reference count for ref-counted `T`s.
     /// The returned weak reference is safe to use only as long as the associated instance remains alive.
-    #[doc(hidden)]
-    pub fn __constructed_gd_weak(&self) -> Gd<T> {
+    ///
+    /// # Safety
+    /// This method disconnects the lifetime, as opposed to [`Self::constructed_passive()]. Caller is responsible of re-binding the
+    /// lifetime to the instance.
+    pub(crate) unsafe fn constructed_passive_unbounded(&self) -> PassiveGd<'static, T> {
         #[cfg(debug_assertions)] // debug_assert! still checks existence of symbols.
         assert!(
             !self.is_initializing(),
             "WithBaseField::base(), base_mut() can only be called on fully-constructed objects, after I*::init() or Gd::from_init_fn()"
         );
 
         // Create weak reference from the same object pointer without cloning (incrementing refcount).
-        unsafe { Gd::from_obj_sys_weak(self.obj.obj_sys()) }
+        let weak_gd = unsafe { Gd::from_obj_sys_weak(self.obj.obj_sys()) };
+
+        // SAFETY: weak_gd is a weakly created Gd, and remains valid as long as self is alive (per safety precondition of this fn).
+        unsafe { PassiveGd::from_weak_owned(weak_gd) }
     }
 }
 

godot-core/src/obj/guards.rs

@@ -15,11 +15,12 @@ use godot_cell::panicking::{InaccessibleGuard, MutGuard, RefGuard};
 use godot_ffi::out;
 
 use crate::obj::script::ScriptInstance;
-use crate::obj::{AsDyn, Gd, GodotClass};
+use crate::obj::{AsDyn, Gd, GodotClass, PassiveGd};
 
 /// Immutably/shared bound reference guard for a [`Gd`][crate::obj::Gd] smart pointer.
 ///
 /// See [`Gd::bind`][crate::obj::Gd::bind] for usage.
+// GdRef could technically implement Clone, but it wasn't needed so far.
 #[derive(Debug)]
 pub struct GdRef<'a, T: GodotClass> {
     guard: RefGuard<'a, T>,
@@ -45,8 +46,6 @@ impl<T: GodotClass> Drop for GdRef<'_, T> {
     }
 }
 
-// TODO Clone or Share
-
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 
 /// Mutably/exclusively bound reference guard for a [`Gd`][crate::obj::Gd] smart pointer.
@@ -199,32 +198,24 @@ macro_rules! make_base_ref {
         #[doc = concat!("This can be used to call methods on the base object of a ", $object_name, " that takes `&self` as the receiver.\n\n")]
         #[doc = concat!("See [`", stringify!($doc_type), "::base()`](", stringify!($doc_path), "::base()) for usage.")]
         pub struct $ident<'a, T: $bound> {
-            // Weak reference to the base object. Safe because 'a lifetime keeps the object (strong ref) alive.
-            // Option because Gd::drop_weak() takes ownership, thus can't use ManuallyDrop.
-            weak_gd: Option<Gd<T::Base>>,
+            passive_gd: PassiveGd<'a, T::Base>,
             _instance: &'a T,
         }
 
         impl<'a, T: $bound> $ident<'a, T> {
-            pub(crate) fn new(weak_gd: Gd<T::Base>, instance: &'a T) -> Self {
+            pub(crate) fn new(passive_gd: PassiveGd<'a, T::Base>, instance: &'a T) -> Self {
                 Self {
-                    weak_gd: Some(weak_gd),
+                    passive_gd,
                     _instance: instance,
                 }
             }
         }
 
-        impl<'a, T: $bound> Drop for $ident<'a, T> {
-            fn drop(&mut self) {
-                self.weak_gd.take().unwrap().drop_weak();
-            }
-        }
-
         impl<T: $bound> Deref for $ident<'_, T> {
             type Target = Gd<T::Base>;
 
             fn deref(&self) -> &Gd<T::Base> {
-                self.weak_gd.as_ref().unwrap()
+                &self.passive_gd
             }
         }
     };
@@ -240,41 +231,33 @@ macro_rules! make_base_mut {
         ///
         #[doc = concat!("See [`", stringify!($doc_type), "::base_mut()`](", stringify!($doc_path), "::base_mut()) for usage.\n")]
         pub struct $ident<'a, T: $bound> {
-            // Weak reference to the base object. Safe because 'a lifetime keeps the object (strong ref) alive.
-            // Option because Gd::drop_weak() takes ownership, thus can't use ManuallyDrop.
-            weak_gd: Option<Gd<T::Base>>,
+            passive_gd: PassiveGd<'a, T::Base>,
             _inaccessible_guard: InaccessibleGuard<'a, T>,
         }
 
         impl<'a, T: $bound> $ident<'a, T> {
             pub(crate) fn new(
-                weak_gd: Gd<T::Base>,
+                passive_gd: PassiveGd<'a, T::Base>,
                 inaccessible_guard: InaccessibleGuard<'a, T>,
             ) -> Self {
                 Self {
-                    weak_gd: Some(weak_gd),
+                    passive_gd,
                     _inaccessible_guard: inaccessible_guard,
                 }
             }
         }
 
-        impl<'a, T: $bound> Drop for $ident<'a, T> {
-            fn drop(&mut self) {
-                self.weak_gd.take().unwrap().drop_weak();
-            }
-        }
-
         impl<T: $bound> Deref for $ident<'_, T> {
             type Target = Gd<T::Base>;
 
             fn deref(&self) -> &Gd<T::Base> {
-                self.weak_gd.as_ref().unwrap()
+                &self.passive_gd
             }
         }
 
         impl<T: $bound> DerefMut for $ident<'_, T> {
             fn deref_mut(&mut self) -> &mut Gd<T::Base> {
-                self.weak_gd.as_mut().unwrap()
+                &mut self.passive_gd
             }
         }
     };

godot-core/src/obj/mod.rs

@@ -20,6 +20,7 @@ mod guards;
 mod instance_id;
 mod on_editor;
 mod on_ready;
+mod passive_gd;
 mod raw_gd;
 mod traits;
 
@@ -33,6 +34,7 @@ pub use guards::{BaseMut, BaseRef, DynGdMut, DynGdRef, GdMut, GdRef};
 pub use instance_id::*;
 pub use on_editor::*;
 pub use on_ready::*;
+pub(crate) use passive_gd::PassiveGd;
 pub use raw_gd::*;
 pub use traits::*;
 

godot-core/src/obj/passive_gd.rs

@@ -0,0 +1,80 @@
+/*
+ * Copyright (c) godot-rust; Bromeon and contributors.
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
+use std::marker::PhantomData;
+use std::mem::ManuallyDrop;
+use std::ops::{Deref, DerefMut};
+
+use crate::obj::{Gd, GodotClass};
+
+/// Passive (non-owning) reference to a Godot object.
+///
+/// `PassiveGd<'gd, T>` provides a safe abstraction for weak references to Godot objects. Unlike `Gd<T>`, it does not increment/decrement
+/// the reference count for `RefCounted` objects, and its `Drop` impl only cleans up metadata, not the Godot object.
+///
+/// The lifetime `'gd` can be used to tie it to a _strong_ `Gd<T>` reference, however it can also be `'static` if more flexibility is needed.
+///
+/// This type is primarily used internally for base object access in guards and traits, providing a clean alternative to manual
+/// [`Gd::clone_weak()`] and [`Gd::drop_weak()`] patterns.
+pub(crate) struct PassiveGd<'gd, T: GodotClass> {
+    weak_gd: ManuallyDrop<Gd<T>>,
+
+    // Covariant lifetime: PassiveGd<'a, T> can be used wherever PassiveGd<'b, T> is needed, if 'a: 'b.
+    _phantom: PhantomData<&'gd ()>,
+}
+
+impl<'gd, T: GodotClass> PassiveGd<'gd, T> {
+    pub fn from_strong_ref(gd: &Gd<T>) -> Self {
+        // SAFETY:
+        // - `clone_weak()` creates a pointer conforming to `from_weak_gd()` requirements.
+        // - PassiveGd will destroy the pointer with `drop_weak()`.
+        unsafe {
+            let weak_gd = gd.clone_weak();
+            Self::from_weak_owned(weak_gd)
+        }
+    }
+
+    /// Creates a passive reference directly from a weak `Gd<T>`.
+    ///
+    /// Will invoke `Gd::drop_weak()` when dropped. Since the parameter has no lifetime, you need to provide the lifetime `'gd` explicitly.
+    ///
+    /// # Safety
+    /// - `weak_gd` must be a weakly created `Gd`, e.g. from [`Gd::clone_weak()`] or [`Gd::from_obj_sys_weak()`].
+    /// - The caller must ensure that the `weak_gd` remains valid for the lifetime `'gd`.
+    pub unsafe fn from_weak_owned(weak_gd: Gd<T>) -> Self {
+        Self {
+            weak_gd: ManuallyDrop::new(weak_gd),
+            _phantom: PhantomData,
+        }
+    }
+}
+
+impl<T: GodotClass> Drop for PassiveGd<'_, T> {
+    fn drop(&mut self) {
+        // SAFETY: Only extracted once, in Drop.
+        let weak = unsafe { ManuallyDrop::take(&mut self.weak_gd) };
+
+        weak.drop_weak();
+    }
+}
+
+impl<T: GodotClass> Deref for PassiveGd<'_, T> {
+    type Target = Gd<T>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.weak_gd
+    }
+}
+
+impl<T: GodotClass> DerefMut for PassiveGd<'_, T> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.weak_gd
+    }
+}
+
+// Note: We intentionally do NOT implement Clone for PassiveGd, as cloning weak references requires careful lifetime management that
+// should be explicit.

godot-core/src/obj/script.rs

@@ -442,7 +442,8 @@ impl<'a, T: ScriptInstance> SiMut<'a, T> {
     /// }
     /// ```
     pub fn base(&self) -> ScriptBaseRef<'_, T> {
-        ScriptBaseRef::new(self.base_ref.to_script_gd(), self.mut_ref)
+        let passive_gd = self.base_ref.to_script_passive();
+        ScriptBaseRef::new(passive_gd, self.mut_ref)
     }
 
     /// Returns a mutable reference suitable for calling engine methods on this object.
@@ -505,8 +506,9 @@ impl<'a, T: ScriptInstance> SiMut<'a, T> {
     /// ```
     pub fn base_mut(&mut self) -> ScriptBaseMut<'_, T> {
         let guard = self.cell.make_inaccessible(self.mut_ref).unwrap();
+        let passive_gd = self.base_ref.to_script_passive();
 
-        ScriptBaseMut::new(self.base_ref.to_script_gd(), guard)
+        ScriptBaseMut::new(passive_gd, guard)
     }
 }
 

godot-core/src/obj/traits.rs

@@ -422,9 +422,8 @@ pub trait WithBaseField: GodotClass + Bounds<Declarer = bounds::DeclUser> {
     ///
     /// For this, use [`base_mut()`](WithBaseField::base_mut()) instead.
     fn base(&self) -> BaseRef<'_, Self> {
-        let weak_gd = self.base_field().__constructed_gd_weak();
-
-        BaseRef::new(weak_gd, self)
+        let passive_gd = self.base_field().constructed_passive();
+        BaseRef::new(passive_gd, self)
     }
 
     /// Returns a mutable reference suitable for calling engine methods on this object.
@@ -493,9 +492,12 @@ pub trait WithBaseField: GodotClass + Bounds<Declarer = bounds::DeclUser> {
     /// ```
     #[allow(clippy::let_unit_value)]
     fn base_mut(&mut self) -> BaseMut<'_, Self> {
-        let weak_gd = self.base_field().__constructed_gd_weak();
+        // We need to construct this first, as the mut-borrow below will block all other access.
+        // SAFETY: lifetime is re-established at the bottom BaseMut construction, since return type of this fn has lifetime bound to instance.
+        let passive_gd = unsafe { self.base_field().constructed_passive_unbounded() };
 
         let gd = self.to_gd();
+
         // SAFETY:
         // - We have a `Gd<Self>` so, provided that `storage_unbounded` succeeds, the associated instance
         //   storage has been created.
@@ -509,12 +511,13 @@ pub trait WithBaseField: GodotClass + Bounds<Declarer = bounds::DeclUser> {
         let storage = unsafe {
             gd.raw
                 .storage_unbounded()
-                .expect("we have a `Gd<Self>` so the raw should not be null")
+                .expect("we have Gd<Self>; its RawGd should not be null")
         };
 
         let guard = storage.get_inaccessible(self);
 
-        BaseMut::new(weak_gd, guard)
+        // Narrows lifetime again from 'static to 'self.
+        BaseMut::new(passive_gd, guard)
     }
 }