Skip to content

Commit

Permalink
Fix return virtual method type
Browse files Browse the repository at this point in the history
Add another virtual method return test
Make test that causes memory leak use `#[itest(skip)]`
Move the logic for determining whether to use `Ref` or not entirely into `Mem`
Remove some unnecessary manual ffi tests
Rename CallType
  • Loading branch information
lilizoey committed Apr 12, 2023
1 parent b5e74b3 commit 8206697
Show file tree
Hide file tree
Showing 42 changed files with 339 additions and 251 deletions.
4 changes: 2 additions & 2 deletions godot-codegen/src/central_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ fn make_sys_code(central_items: &CentralItems) -> String {
}

// SAFETY:
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
// This type is represented as `Self` in Godot, so `*mut Self` is sound.
unsafe impl GodotFfi for VariantType {
ffi_methods! { type GDExtensionTypePtr = *mut Self; .. }
}
Expand Down Expand Up @@ -208,7 +208,7 @@ fn make_sys_code(central_items: &CentralItems) -> String {
}

// SAFETY:
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
// This type is represented as `Self` in Godot, so `*mut Self` is sound.
unsafe impl GodotFfi for VariantOperator {
ffi_methods! { type GDExtensionTypePtr = *mut Self; .. }
}
Expand Down
4 changes: 4 additions & 0 deletions godot-codegen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ const SELECTED_CLASSES: &[&str] = &[
"AudioStreamPlayer",
"BaseButton",
"Button",
"BoxMesh",
"Camera2D",
"Camera3D",
"CanvasItem",
Expand All @@ -276,6 +277,7 @@ const SELECTED_CLASSES: &[&str] = &[
"Label",
"MainLoop",
"Marker2D",
"Mesh",
"Node",
"Node2D",
"Node3D",
Expand All @@ -285,9 +287,11 @@ const SELECTED_CLASSES: &[&str] = &[
"PackedScene",
"PathFollow2D",
"PhysicsBody2D",
"PrimitiveMesh",
"RefCounted",
"RenderingServer",
"Resource",
"ResourceFormatLoader",
"ResourceLoader",
"RigidBody2D",
"SceneTree",
Expand Down
2 changes: 2 additions & 0 deletions godot-codegen/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ pub fn make_enum_definition(enum_: &Enum) -> TokenStream {
self.ord
}
}
// SAFETY:
// The enums are transparently represented as an `i32`, so `*mut Self` is sound.
unsafe impl sys::GodotFfi for #enum_name {
sys::ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/aabb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl Aabb {
}

// SAFETY:
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
// This type is represented as `Self` in Godot, so `*mut Self` is sound.
unsafe impl GodotFfi for Aabb {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}
17 changes: 9 additions & 8 deletions godot-core/src/builtin/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -566,22 +566,23 @@ impl<T: VariantMetadata + ToVariant> Array<T> {
// ...
// }

// SAFETY:
// - `move_return_ptr`
// Nothing special needs to be done beyond a `std::mem::swap` when returning an Array.
// So we can just use `ffi_methods`.
//
// - `from_arg_ptr`
// Arrays are properly initialized through a `from_sys` call, but the ref-count should be incremented
// as that is the callee's responsibility. Which we do by calling `std::mem::forget(array.share())`.
unsafe impl<T: VariantMetadata> GodotFfi for Array<T> {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque;
fn from_sys;
fn sys;
fn from_sys_init;
// SAFETY:
// Nothing special needs to be done beyond a `std::mem::swap` when returning an Array.
fn move_return_ptr;
}

// SAFETY:
// Arrays are properly initialized through a `from_sys` call, but the ref-count should be
// incremented as that is the callee's responsibility.
//
// Using `std::mem::forget(array.share())` increments the ref count.
unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::CallType) -> Self {
unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self {
let array = Self::from_sys(ptr);
std::mem::forget(array.share());
array
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/basis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ impl Mul<Vector3> for Basis {
}

// SAFETY:
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
// This type is represented as `Self` in Godot, so `*mut Self` is sound.
unsafe impl GodotFfi for Basis {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/color.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ impl Color {
}

// SAFETY:
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
// This type is represented as `Self` in Godot, so `*mut Self` is sound.
unsafe impl GodotFfi for Color {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}
Expand Down
18 changes: 10 additions & 8 deletions godot-core/src/builtin/dictionary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,22 +239,24 @@ impl Dictionary {
// ----------------------------------------------------------------------------------------------------------------------------------------------
// Traits

// SAFETY:
// - `move_return_ptr`
// Nothing special needs to be done beyond a `std::mem::swap` when returning an Dictionary.
// So we can just use `ffi_methods`.
//
// - `from_arg_ptr`
// Dictionaries are properly initialized through a `from_sys` call, but the ref-count should be
// incremented as that is the callee's responsibility. Which we do by calling
// `std::mem::forget(dictionary.share())`.
unsafe impl GodotFfi for Dictionary {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque;
fn from_sys;
fn from_sys_init;
fn sys;
// SAFETY:
// Nothing special needs to be done beyond a `std::mem::swap` when returning a dictionary.
fn move_return_ptr;
}

// SAFETY:
// Dictionaries are properly initialized through a `from_sys` call, but the ref-count should be
// incremented as that is the callee's responsibility.
//
// Using `std::mem::forget(dictionary.share())` increments the ref count.
unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::CallType) -> Self {
unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self {
let dictionary = Self::from_sys(ptr);
std::mem::forget(dictionary.share());
dictionary
Expand Down
4 changes: 2 additions & 2 deletions godot-core/src/builtin/meta/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub trait SignatureTuple {
ret: sys::GDExtensionTypePtr,
func: fn(&mut C, Self::Params) -> Self::Ret,
method_name: &str,
call_type: sys::CallType,
call_type: sys::PtrcallType,
);
}

Expand Down Expand Up @@ -144,7 +144,7 @@ macro_rules! impl_signature_for_tuple {
ret: sys::GDExtensionTypePtr,
func: fn(&mut C, Self::Params) -> Self::Ret,
method_name: &str,
call_type: sys::CallType,
call_type: sys::PtrcallType,
) {
$crate::out!("ptrcall: {}", method_name);

Expand Down
18 changes: 10 additions & 8 deletions godot-core/src/builtin/node_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,24 @@ impl NodePath {
}
}

// SAFETY:
// - `move_return_ptr`
// Nothing special needs to be done beyond a `std::mem::swap` when returning a NodePath.
// So we can just use `ffi_methods`.
//
// - `from_arg_ptr`
// NodePaths are properly initialized through a `from_sys` call, but the ref-count should be
// incremented as that is the callee's responsibility. Which we do by calling
// `std::mem::forget(node_path.share())`.
unsafe impl GodotFfi for NodePath {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque;
fn from_sys;
fn sys;
fn from_sys_init;
// SAFETY:
// Nothing special needs to be done beyond a `std::mem::swap` when returning a NodePath.
fn move_return_ptr;
}

// SAFETY:
// NodePaths are properly initialized through a `from_sys` call, but the ref-count should be
// incremented as that is the callee's responsibility.
//
// Using `std::mem::forget(node_path.share())` increments the ref count.
unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::CallType) -> Self {
unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self {
let node_path = Self::from_sys(ptr);
std::mem::forget(node_path.clone());
node_path
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/packed_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ macro_rules! impl_packed_array {
// incremented as that is the callee's responsibility.
//
// Using `std::mem::forget(array.clone())` increments the ref count.
unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::CallType) -> Self {
unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self {
let array = Self::from_sys(ptr);
std::mem::forget(array.clone());
array
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/plane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ impl Neg for Plane {
}

// SAFETY:
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
// This type is represented as `Self` in Godot, so `*mut Self` is sound.
unsafe impl GodotFfi for Plane {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/projection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ impl GlamConv for Projection {
}

// SAFETY:
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
// This type is represented as `Self` in Godot, so `*mut Self` is sound.
unsafe impl GodotFfi for Projection {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/quaternion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ impl Mul<Quaternion> for Quaternion {
}

// SAFETY:
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
// This type is represented as `Self` in Godot, so `*mut Self` is sound.
unsafe impl GodotFfi for Quaternion {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/rect2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl Rect2 {
}

// SAFETY:
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
// This type is represented as `Self` in Godot, so `*mut Self` is sound.
unsafe impl GodotFfi for Rect2 {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}
2 changes: 1 addition & 1 deletion godot-core/src/builtin/rect2i.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ impl Rect2i {
}

// SAFETY:
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
// This type is represented as `Self` in Godot, so `*mut Self` is sound.
unsafe impl GodotFfi for Rect2i {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}
2 changes: 1 addition & 1 deletion godot-core/src/builtin/rid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ impl Rid {
}

// SAFETY:
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
// This type is represented as `Self` in Godot, so `*mut Self` is sound.
unsafe impl GodotFfi for Rid {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}
22 changes: 12 additions & 10 deletions godot-core/src/builtin/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ impl GodotString {
fn string_sys = sys;
}

/// Move `self` into a system pointer.
/// Move `self` into a system pointer. This transfers ownership and thus does not call the destructor.
///
/// # Safety
/// `dst` must be a pointer to a `GodotString` which is suitable for ffi with Godot.
pub unsafe fn move_string_ptr(self, dst: sys::GDExtensionStringPtr) {
self.move_return_ptr(dst as *mut _, sys::CallType::Standard);
self.move_return_ptr(dst as *mut _, sys::PtrcallType::Standard);
}

/// Gets the internal chars slice from a [`GodotString`].
Expand Down Expand Up @@ -75,22 +75,24 @@ impl GodotString {
}
}

// SAFETY:
// - `move_return_ptr`
// Nothing special needs to be done beyond a `std::mem::swap` when returning a String.
// So we can just use `ffi_methods`.
//
// - `from_arg_ptr`
// Strings are properly initialized through a `from_sys` call, but the ref-count should be
// incremented as that is the callee's responsibility. Which we do by calling
// `std::mem::forget(string.share())`.
unsafe impl GodotFfi for GodotString {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque;
fn from_sys;
fn sys;
fn from_sys_init;
// SAFETY:
// Nothing special needs to be done beyond a `std::mem::swap` when returning a GodotString.
fn move_return_ptr;
}

// SAFETY:
// GodotStrings are properly initialized through a `from_sys` call, but the ref-count should be
// incremented as that is the callee's responsibility.
//
// Using `std::mem::forget(string.share())` increments the ref count.
unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::CallType) -> Self {
unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self {
let string = Self::from_sys(ptr);
std::mem::forget(string.clone());
string
Expand Down
18 changes: 10 additions & 8 deletions godot-core/src/builtin/string_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,24 @@ impl StringName {
}
}

// SAFETY:
// - `move_return_ptr`
// Nothing special needs to be done beyond a `std::mem::swap` when returning a StringName.
// So we can just use `ffi_methods`.
//
// - `from_arg_ptr`
// StringNames are properly initialized through a `from_sys` call, but the ref-count should be
// incremented as that is the callee's responsibility. Which we do by calling
// `std::mem::forget(string_name.share())`.
unsafe impl GodotFfi for StringName {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque;
fn from_sys;
fn sys;
fn from_sys_init;
// SAFETY:
// Nothing special needs to be done beyond a `std::mem::swap` when returning a StringName.
fn move_return_ptr;
}

// SAFETY:
// StringNames are properly initialized through a `from_sys` call, but the ref-count should be
// incremented as that is the callee's responsibility.
//
// Using `std::mem::forget(string_name.share())` increments the ref count.
unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::CallType) -> Self {
unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self {
let string_name = Self::from_sys(ptr);
std::mem::forget(string_name.clone());
string_name
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/transform2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ impl GlamConv for Transform2D {
}

// SAFETY:
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
// This type is represented as `Self` in Godot, so `*mut Self` is sound.
unsafe impl GodotFfi for Transform2D {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/transform3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ impl GlamConv for Transform3D {
}

// SAFETY:
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
// This type is represented as `Self` in Godot, so `*mut Self` is sound.
unsafe impl GodotFfi for Transform3D {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}
Expand Down
4 changes: 2 additions & 2 deletions godot-core/src/builtin/vector2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ impl_vector_operators!(Vector2, real, (x, y));
impl_vector_index!(Vector2, real, (x, y), Vector2Axis, (X, Y));

// SAFETY:
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
// This type is represented as `Self` in Godot, so `*mut Self` is sound.
unsafe impl GodotFfi for Vector2 {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}
Expand All @@ -327,7 +327,7 @@ pub enum Vector2Axis {
}

// SAFETY:
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
// This type is represented as `Self` in Godot, so `*mut Self` is sound.
unsafe impl GodotFfi for Vector2Axis {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}
Expand Down
4 changes: 2 additions & 2 deletions godot-core/src/builtin/vector2i.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ impl_vector_operators!(Vector2i, i32, (x, y));
impl_vector_index!(Vector2i, i32, (x, y), Vector2iAxis, (X, Y));

// SAFETY:
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
// This type is represented as `Self` in Godot, so `*mut Self` is sound.
unsafe impl GodotFfi for Vector2i {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}
Expand All @@ -105,7 +105,7 @@ pub enum Vector2iAxis {
}

// SAFETY:
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
// This type is represented as `Self` in Godot, so `*mut Self` is sound.
unsafe impl GodotFfi for Vector2iAxis {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}
Loading

0 comments on commit 8206697

Please sign in to comment.