Skip to content

Commit

Permalink
Fix/suppress clippy lints on nightly
Browse files Browse the repository at this point in the history
- 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.
- A few other minor fixes.
  • Loading branch information
hesuteia authored and chitoyuu committed Nov 27, 2021
1 parent a0241d1 commit 408972d
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 13 deletions.
2 changes: 2 additions & 0 deletions gdnative-bindings/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]

Expand Down
5 changes: 1 addition & 4 deletions gdnative-core/src/core_types/transform2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
2 changes: 1 addition & 1 deletion gdnative-core/src/core_types/variant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
29 changes: 23 additions & 6 deletions gdnative-core/src/export/user_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>,
Expand All @@ -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.
Expand Down Expand Up @@ -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 {
Expand All @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion gdnative-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]

Expand Down
2 changes: 1 addition & 1 deletion test/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![allow(clippy::blacklisted_name, clippy::if_then_panic)]
#![allow(clippy::blacklisted_name)]

use gdnative::prelude::*;

Expand Down

0 comments on commit 408972d

Please sign in to comment.