From 8c0d5db717800c36eac05d494e0f66fd4464af69 Mon Sep 17 00:00:00 2001 From: ecobiubiu Date: Sat, 27 Nov 2021 05:05:09 +0000 Subject: [PATCH] Fix/suppress clippy lints on nightly - Modified `LocalCell` 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. - A few other minor fixes. --- gdnative-bindings/src/lib.rs | 2 ++ gdnative-core/src/core_types/transform2d.rs | 5 +--- gdnative-core/src/core_types/variant.rs | 2 +- gdnative-core/src/export/user_data.rs | 29 ++++++++++++++++----- gdnative-core/src/lib.rs | 2 +- test/src/lib.rs | 2 +- 6 files changed, 29 insertions(+), 13 deletions(-) diff --git a/gdnative-bindings/src/lib.rs b/gdnative-bindings/src/lib.rs index 8a3cd64..53ef6cc 100644 --- a/gdnative-bindings/src/lib.rs +++ b/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)] diff --git a/gdnative-core/src/core_types/transform2d.rs b/gdnative-core/src/core_types/transform2d.rs index 67e190f..791762a 100644 --- a/gdnative-core/src/core_types/transform2d.rs +++ b/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 = diff --git a/gdnative-core/src/core_types/variant.rs b/gdnative-core/src/core_types/variant.rs index 6bb3431..b5eda7c 100644 --- a/gdnative-core/src/core_types/variant.rs +++ b/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) } } diff --git a/gdnative-core/src/export/user_data.rs b/gdnative-core/src/export/user_data.rs index 496de69..5e1a208 100644 --- a/gdnative-core/src/export/user_data.rs +++ b/gdnative-core/src/export/user_data.rs @@ -522,10 +522,16 @@ impl Clone for ArcData { } /// User-data wrapper analogous to a `Arc>`, 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 { inner: Arc>, @@ -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 { thread_id: ThreadId, - cell: RefCell, + cell: RefCell>, + } + + impl Drop for LocalCell { + 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, LocalCellError> { + fn inner(&self) -> Result<&RefCell>, 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, LocalCellError> { + pub fn try_borrow(&self) -> Result>, LocalCellError> { let inner = self.inner()?; inner.try_borrow().map_err(|_| LocalCellError::BorrowFailed) } #[inline] - pub fn try_borrow_mut(&self) -> Result, LocalCellError> { + pub fn try_borrow_mut(&self) -> Result>, LocalCellError> { let inner = self.inner()?; inner .try_borrow_mut() diff --git a/gdnative-core/src/lib.rs b/gdnative-core/src/lib.rs index 537fd98..142f512 100644 --- a/gdnative-core/src/lib.rs +++ b/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))] diff --git a/test/src/lib.rs b/test/src/lib.rs index a9437c5..4bdc9dc 100644 --- a/test/src/lib.rs +++ b/test/src/lib.rs @@ -1,4 +1,4 @@ -#![allow(clippy::blacklisted_name, clippy::if_then_panic)] +#![allow(clippy::blacklisted_name)] use gdnative::prelude::*;