Merge #822

822: Fix/suppress clippy lints on nightly r=Bromeon a=chitoyuu

- Modified `LocalCell<T>` so the destructor is not called on a different thread. Allowed `clippy::non_send_fields_in_send_ty` otherwise.
- Removed mentions of `if_then_panic` which is no longer a thing (`https://github.com/rust-lang/rust-clippy/pull/7810`).
- A few other minor fixes.

Co-authored-by: Chitose Yuuzaki <[email protected]>

Author: bors[bot]

gdnative-bindings/src/lib.rs

@@ -5,6 +5,8 @@
 #![allow(unused_unsafe)]
 // False positives on generated drops that enforce lifetime
 #![allow(clippy::drop_copy)]
+// False positives on thread-safe singletons
+#![allow(clippy::non_send_fields_in_send_ty)]
 // Disable non-critical lints for generated code
 #![allow(clippy::style, clippy::complexity, clippy::perf)]
 

gdnative-core/src/core_types/transform2d.rs

@@ -360,10 +360,7 @@ fn test_transform2d_behavior_impl() {
     let rotation_rust = new_transform_rust.rotation();
     let rotation_godot = unsafe { (api.godot_transform2d_get_rotation)(new_transform_rust.sys()) };
 
-    assert!(
-        (rotation_rust - rotation_godot) < f32::EPSILON,
-        "Rotation getters should return equal results"
-    );
+    approx::assert_relative_eq!(rotation_rust, rotation_godot);
 
     let scale_rust = new_transform_rust.scale();
     let scale_godot =

gdnative-core/src/core_types/variant.rs

@@ -623,7 +623,7 @@ impl Default for Variant {
 impl fmt::Debug for Variant {
     #[inline]
     fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
-        write!(f, "{:?}({})", self.get_type(), self.to_string())
+        write!(f, "{:?}({})", self.get_type(), self)
     }
 }
 

gdnative-core/src/export/user_data.rs

@@ -522,10 +522,16 @@ impl<T> Clone for ArcData<T> {
 }
 
 /// User-data wrapper analogous to a `Arc<RefCell<T>>`, that is restricted to the thread
-/// where it was originally created.
+/// where it was originally created. The destructor of `T` is not guaranteed to be run if
+/// this is actually shared across multiple threads.
 ///
 /// This works by checking `ThreadId` before touching the underlying reference. If the id
 /// doesn't match the original thread, `map` and `map_mut` will return an error.
+///
+/// Since it isn't possible to control where the last reference is dropped, the destructor
+/// isn't run if the last reference is dropped on a different thread than the one where the
+/// wrapper was originally created. To ensure that values are cleaned up properly, keep the
+/// game single-threaded, or use a real thread-safe wrapper such as [`MutexData`] instead.
 #[derive(Debug)]
 pub struct LocalCellData<T> {
     inner: Arc<local_cell::LocalCell<T>>,
@@ -535,12 +541,23 @@ pub use self::local_cell::LocalCellError;
 
 mod local_cell {
     use std::cell::{Ref, RefCell, RefMut};
+    use std::mem::ManuallyDrop;
     use std::thread::{self, ThreadId};
 
     #[derive(Debug)]
     pub struct LocalCell<T> {
         thread_id: ThreadId,
-        cell: RefCell<T>,
+        cell: RefCell<ManuallyDrop<T>>,
+    }
+
+    impl<T> Drop for LocalCell<T> {
+        fn drop(&mut self) {
+            if self.thread_id == thread::current().id() {
+                unsafe {
+                    ManuallyDrop::drop(self.cell.get_mut());
+                }
+            }
+        }
     }
 
     /// Error indicating that a borrow has failed.
@@ -578,12 +595,12 @@ mod local_cell {
         pub fn new(val: T) -> Self {
             LocalCell {
                 thread_id: thread::current().id(),
-                cell: RefCell::new(val),
+                cell: RefCell::new(ManuallyDrop::new(val)),
             }
         }
 
         #[inline]
-        fn inner(&self) -> Result<&RefCell<T>, LocalCellError> {
+        fn inner(&self) -> Result<&RefCell<ManuallyDrop<T>>, LocalCellError> {
             let current = thread::current().id();
 
             if self.thread_id == current {
@@ -597,13 +614,13 @@ mod local_cell {
         }
 
         #[inline]
-        pub fn try_borrow(&self) -> Result<Ref<T>, LocalCellError> {
+        pub fn try_borrow(&self) -> Result<Ref<ManuallyDrop<T>>, LocalCellError> {
             let inner = self.inner()?;
             inner.try_borrow().map_err(|_| LocalCellError::BorrowFailed)
         }
 
         #[inline]
-        pub fn try_borrow_mut(&self) -> Result<RefMut<T>, LocalCellError> {
+        pub fn try_borrow_mut(&self) -> Result<RefMut<ManuallyDrop<T>>, LocalCellError> {
             let inner = self.inner()?;
             inner
                 .try_borrow_mut()

gdnative-core/src/lib.rs

@@ -23,7 +23,7 @@
 #![allow(
     clippy::transmute_ptr_to_ptr,
     clippy::missing_safety_doc,
-    clippy::if_then_panic
+    clippy::non_send_fields_in_send_ty
 )]
 #![cfg_attr(feature = "gd_test", allow(clippy::blacklisted_name))]
 

test/src/lib.rs

@@ -1,4 +1,4 @@
-#![allow(clippy::blacklisted_name, clippy::if_then_panic)]
+#![allow(clippy::blacklisted_name)]
 
 use gdnative::prelude::*;