Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OutArray, a possibly typed array. #806

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions godot-codegen/src/conv/type_conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ fn to_hardcoded_rust_ident(full_ty: &GodotTy) -> Option<&str> {
// Others
("bool", None) => "bool",
("String", None) => "GString",
("Array", None) => "VariantArray",
("Array", None) => "OutArray",

// Types needed for native structures mapping
("uint8_t", None) => "u8",
Expand Down Expand Up @@ -217,7 +217,9 @@ fn to_rust_type_uncached(full_ty: &GodotTy, ctx: &mut Context) -> RustTy {
} else if let Some(elem_ty) = ty.strip_prefix("typedarray::") {
let rust_elem_ty = to_rust_type(elem_ty, None, ctx);
return if ctx.is_builtin(elem_ty) {
RustTy::BuiltinArray(quote! { Array<#rust_elem_ty> })
RustTy::BuiltinArray {
elem_type: quote! { Array<#rust_elem_ty> },
}
} else {
RustTy::EngineArray {
tokens: quote! { Array<#rust_elem_ty> },
Expand Down Expand Up @@ -256,6 +258,7 @@ fn to_rust_expr_inner(expr: &str, ty: &RustTy, is_inner: bool) -> TokenStream {
"true" => return quote! { true },
"false" => return quote! { false },
"[]" | "{}" if is_inner => return quote! {},
"[]" if ty.is_out_array() => return quote! { OutArray::new_untyped() },
"[]" => return quote! { Array::new() }, // VariantArray or Array<T>
"{}" => return quote! { Dictionary::new() },
"null" => {
Expand Down
13 changes: 3 additions & 10 deletions godot-codegen/src/generator/builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,16 +164,9 @@ fn make_special_builtin_methods(class_name: &TyName, _ctx: &Context) -> TokenStr
}

/// Get the safety docs of an unsafe method, or `None` if it is safe.
fn method_safety_doc(class_name: &TyName, method: &BuiltinMethod) -> Option<TokenStream> {
if class_name.godot_ty == "Array"
&& &method.return_value().type_tokens().to_string() == "VariantArray"
{
return Some(quote! {
/// # Safety
///
/// You must ensure that the returned array fulfils the safety invariants of [`Array`](crate::builtin::Array).
});
}
fn method_safety_doc(_class_name: &TyName, _method: &BuiltinMethod) -> Option<TokenStream> {
// Was previously used for InnerArray methods returning VariantArray.
// Now safe with OutArray -> conversion happens in client code.

None
}
Expand Down
12 changes: 9 additions & 3 deletions godot-codegen/src/models/domain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -582,11 +582,13 @@ pub struct GodotTy {

#[derive(Clone, Debug)]
pub enum RustTy {
/// `bool`, `Vector3i`
/// `bool`, `Vector3i`, `Array`
BuiltinIdent(Ident),

/// `Array<i32>`
BuiltinArray(TokenStream),
///
/// Note that untyped arrays are mapped as `BuiltinIdent("Array")`.
BuiltinArray { elem_type: TokenStream },

/// C-style raw pointer to a `RustTy`.
RawPointer { inner: Box<RustTy>, is_const: bool },
Expand Down Expand Up @@ -634,13 +636,17 @@ impl RustTy {
other => quote! { -> #other },
}
}

pub fn is_out_array(&self) -> bool {
matches!(self, Self::BuiltinIdent(id) if id == "OutArray")
}
}

impl ToTokens for RustTy {
fn to_tokens(&self, tokens: &mut TokenStream) {
match self {
RustTy::BuiltinIdent(ident) => ident.to_tokens(tokens),
RustTy::BuiltinArray(path) => path.to_tokens(tokens),
RustTy::BuiltinArray { elem_type } => elem_type.to_tokens(tokens),
RustTy::RawPointer {
inner,
is_const: true,
Expand Down
2 changes: 1 addition & 1 deletion godot-codegen/src/special_cases/codegen_special_cases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fn is_type_excluded(ty: &str, ctx: &mut Context) -> bool {
fn is_rust_type_excluded(ty: &RustTy) -> bool {
match ty {
RustTy::BuiltinIdent(_) => false,
RustTy::BuiltinArray(_) => false,
RustTy::BuiltinArray { .. } => false,
RustTy::RawPointer { inner, .. } => is_rust_type_excluded(inner),
RustTy::EngineArray { elem_class, .. } => is_class_excluded(elem_class.as_str()),
RustTy::EngineEnum {
Expand Down
4 changes: 2 additions & 2 deletions godot-core/src/builtin/callable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,14 +170,14 @@ impl Callable {
///
/// _Godot equivalent: `callv`_
pub fn callv(&self, arguments: VariantArray) -> Variant {
self.as_inner().callv(arguments)
self.as_inner().callv(arguments.to_out_array())
}

/// Returns a copy of this Callable with one or more arguments bound, reading them from an array.
///
/// _Godot equivalent: `bindv`_
pub fn bindv(&self, arguments: VariantArray) -> Self {
self.as_inner().bindv(arguments)
self.as_inner().bindv(arguments.to_out_array())
}

/// Returns the name of the method represented by this callable. If the callable is a lambda function,
Expand Down
147 changes: 92 additions & 55 deletions godot-core/src/builtin/collections/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::fmt;
use std::marker::PhantomData;

use crate::builtin::*;
use crate::meta::error::ArrayMismatch;
use crate::meta::error::{ConvertError, FromGodotError, FromVariantError};
use crate::meta::{
ArrayElement, ArrayTypeInfo, FromGodot, GodotConvert, GodotFfiVariant, GodotType, ToGodot,
Expand Down Expand Up @@ -107,7 +108,8 @@ use sys::{ffi_methods, interface_fn, GodotFfi};
/// concurrent modification on other threads (e.g. created through GDScript).
pub struct Array<T: ArrayElement> {
// Safety Invariant: The type of all values in `opaque` matches the type `T`.
opaque: sys::types::OpaqueArray,
// Visibility: shared with OutArray.
pub(super) opaque: sys::types::OpaqueArray,
_phantom: PhantomData<T>,
}

Expand Down Expand Up @@ -146,7 +148,8 @@ impl_builtin_froms!(VariantArray;
);

impl<T: ArrayElement> Array<T> {
fn from_opaque(opaque: sys::types::OpaqueArray) -> Self {
// Visibility: shared with OutArray.
pub(super) fn from_opaque(opaque: sys::types::OpaqueArray) -> Self {
// Note: type is not yet checked at this point, because array has not yet been initialized!
Self {
opaque,
Expand Down Expand Up @@ -417,13 +420,10 @@ impl<T: ArrayElement> Array<T> {

/// Appends another array at the end of this array. Equivalent of `append_array` in GDScript.
pub fn extend_array(&mut self, other: Array<T>) {
// SAFETY: `append_array` will only read values from `other`, and all types can be converted to `Variant`.
let other: VariantArray = unsafe { other.assume_type::<Variant>() };

// SAFETY: `append_array` will only write values gotten from `other` into `self`, and all values in `other` are guaranteed
// SAFETY: `append_array` only reads values from `other` and writes them to `self`, and all values in `other` are guaranteed
// to be of type `T`.
let mut inner_self = unsafe { self.as_inner_mut() };
inner_self.append_array(other);
inner_self.append_array(other.to_out_array());
}

/// Returns a shallow copy of the array. All array elements are copied, but any reference types
Expand All @@ -432,10 +432,9 @@ impl<T: ArrayElement> Array<T> {
/// To create a deep copy, use [`duplicate_deep()`][Self::duplicate_deep] instead.
/// To create a new reference to the same array data, use [`clone()`][Clone::clone].
pub fn duplicate_shallow(&self) -> Self {
// SAFETY: We never write to the duplicated array, and all values read are read as `Variant`.
let duplicate: VariantArray = unsafe { self.as_inner().duplicate(false) };
let duplicate: OutArray = self.as_inner().duplicate(false);

// SAFETY: duplicate() returns a typed array with the same type as Self, and all values are taken from `self` so have the right type.
// SAFETY: duplicate() returns a typed array with the same type as Self.
unsafe { duplicate.assume_type() }
}

Expand All @@ -446,10 +445,9 @@ impl<T: ArrayElement> Array<T> {
/// To create a shallow copy, use [`duplicate_shallow()`][Self::duplicate_shallow] instead.
/// To create a new reference to the same array data, use [`clone()`][Clone::clone].
pub fn duplicate_deep(&self) -> Self {
// SAFETY: We never write to the duplicated array, and all values read are read as `Variant`.
let duplicate: VariantArray = unsafe { self.as_inner().duplicate(true) };
let duplicate: OutArray = self.as_inner().duplicate(true);

// SAFETY: duplicate() returns a typed array with the same type as Self, and all values are taken from `self` so have the right type.
// SAFETY: duplicate() returns a typed array with the same type as Self.
unsafe { duplicate.assume_type() }
}

Expand Down Expand Up @@ -494,12 +492,11 @@ impl<T: ArrayElement> Array<T> {
let step = step.unwrap_or(1);

// SAFETY: The type of the array is `T` and we convert the returned array to an `Array<T>` immediately.
let subarray: VariantArray = unsafe {
let subarray: OutArray =
self.as_inner()
.slice(to_i64(begin), to_i64(end), step.try_into().unwrap(), deep)
};
.slice(to_i64(begin), to_i64(end), step.try_into().unwrap(), deep);

// SAFETY: slice() returns a typed array with the same type as Self
// SAFETY: slice() returns a typed array with the same type as Self.
unsafe { subarray.assume_type() }
}

Expand Down Expand Up @@ -554,8 +551,7 @@ impl<T: ArrayElement> Array<T> {
}

/// Searches the array backwards for the last occurrence of a value and returns its index, or
/// `None` if not found. Starts searching at index `from`; pass `None` to search the entire
/// array.
/// `None` if not found. Starts searching at index `from`; pass `None` to search the entire array.
pub fn rfind(&self, value: &T, from: Option<usize>) -> Option<usize> {
let from = from.map(to_i64).unwrap_or(-1);
let index = self.as_inner().rfind(value.to_variant(), from);
Expand Down Expand Up @@ -633,6 +629,10 @@ impl<T: ArrayElement> Array<T> {
unsafe { self.as_inner_mut() }.shuffle();
}

pub fn to_out_array(&self) -> OutArray {
OutArray::consume_typed_array(self.clone())
}

/// Asserts that the given index refers to an existing element.
///
/// # Panics
Expand Down Expand Up @@ -734,17 +734,45 @@ impl<T: ArrayElement> Array<T> {
///
/// In the current implementation, both cases will produce a panic rather than undefined
/// behavior, but this should not be relied upon.
unsafe fn assume_type<U: ArrayElement>(self) -> Array<U> {
// Visibility: shared with OutArray.
pub(super) unsafe fn assume_type<U: ArrayElement>(self) -> Array<U> {
// SAFETY: The memory layout of `Array<T>` does not depend on `T`.
unsafe { std::mem::transmute(self) }
std::mem::transmute(self)
}

/// # Safety
/// Returned type may be inaccurate and must be type-checked after the call.
// Visibility: shared with OutArray.
pub(super) unsafe fn default_unchecked() -> Array<T> {
Self::new_with_uninit(|self_ptr| {
let ctor = sys::builtin_fn!(array_construct_default);
ctor(self_ptr, std::ptr::null_mut())
})
}

/// # Safety
/// Returned type may be inaccurate and must be type-checked after the call.
// Visibility: shared with OutArray.
pub(super) unsafe fn clone_unchecked(&self) -> Array<T> {
Self::new_with_uninit(|self_ptr| {
let ctor = sys::builtin_fn!(array_construct_copy);
let args = [self.sys()];
ctor(self_ptr, args.as_ptr());
})
}

/// Returns the runtime type info of this array.
fn type_info(&self) -> ArrayTypeInfo {
// Visibility: shared with OutArray.
pub(super) fn type_info(&self) -> ArrayTypeInfo {
let variant_type = VariantType::from_sys(
self.as_inner().get_typed_builtin() as sys::GDExtensionVariantType
);
let class_name = self.as_inner().get_typed_class_name();

let class_name = if variant_type == VariantType::OBJECT {
Some(self.as_inner().get_typed_class_name())
} else {
None
};

ArrayTypeInfo {
variant_type,
Expand All @@ -760,10 +788,10 @@ impl<T: ArrayElement> Array<T> {
if self_ty == target_ty {
Ok(self)
} else {
Err(FromGodotError::BadArrayType {
Err(FromGodotError::BadArrayType(ArrayMismatch {
expected: target_ty,
actual: self_ty,
}
})
.into_error(self))
}
}
Expand All @@ -781,17 +809,49 @@ impl<T: ArrayElement> Array<T> {
if type_info.is_typed() {
let script = Variant::nil();

// A bit contrived because empty StringName is lazy-initialized but must also remain valid.
#[allow(unused_assignments)]
let mut empty_string_name = None;
let class_name = if let Some(class_name) = &type_info.class_name {
class_name.string_sys()
} else {
empty_string_name = Some(StringName::default());
empty_string_name.unwrap().string_sys()
};

// SAFETY: The array is a newly created empty untyped array.
unsafe {
interface_fn!(array_set_typed)(
self.sys_mut(),
type_info.variant_type.sys(),
type_info.class_name.string_sys(),
class_name, // must be empty if variant_type != OBJECT.
script.var_sys(),
);
}
}
}

/// # Safety
/// Does not validate the array element type; `with_checked_type()` should be called afterward.
// Visibility: shared with OutArray.
pub(super) unsafe fn unchecked_from_variant(variant: &Variant) -> Result<Self, ConvertError> {
if variant.get_type() != Self::variant_type() {
return Err(FromVariantError::BadType {
expected: Self::variant_type(),
actual: variant.get_type(),
}
.into_error(variant.clone()));
}

let array = unsafe {
sys::new_with_uninit_or_init::<Self>(|self_ptr| {
let array_from_variant = sys::builtin_fn!(array_from_variant);
array_from_variant(self_ptr, sys::SysPtr::force_mut(variant.var_sys()));
})
};

Ok(array)
}
}

// ----------------------------------------------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -885,14 +945,8 @@ impl<T: ArrayElement + fmt::Display> fmt::Display for Array<T> {
/// [`Array::duplicate_deep()`].
impl<T: ArrayElement> Clone for Array<T> {
fn clone(&self) -> Self {
// SAFETY: `self` is a valid array, since we have a reference that keeps it alive.
let array = unsafe {
Self::new_with_uninit(|self_ptr| {
let ctor = sys::builtin_fn!(array_construct_copy);
let args = [self.sys()];
ctor(self_ptr, args.as_ptr());
})
};
// SAFETY: `self` is a valid array, since we have a reference that keeps it alive. Type is checked below.
let array = unsafe { self.clone_unchecked() };

array
.with_checked_type()
Expand Down Expand Up @@ -952,12 +1006,7 @@ impl Export for Array<Variant> {
impl<T: ArrayElement> Default for Array<T> {
#[inline]
fn default() -> Self {
let mut array = unsafe {
Self::new_with_uninit(|self_ptr| {
let ctor = sys::builtin_fn!(array_construct_default);
ctor(self_ptr, std::ptr::null_mut())
})
};
let mut array = unsafe { Self::default_unchecked() };

// SAFETY: We just created this array, and haven't called `init_inner_type` before.
unsafe { array.init_inner_type() };
Expand Down Expand Up @@ -1010,22 +1059,10 @@ impl<T: ArrayElement> GodotFfiVariant for Array<T> {
}

fn ffi_from_variant(variant: &Variant) -> Result<Self, ConvertError> {
if variant.get_type() != Self::variant_type() {
return Err(FromVariantError::BadType {
expected: Self::variant_type(),
actual: variant.get_type(),
}
.into_error(variant.clone()));
}

let array = unsafe {
sys::new_with_uninit_or_init::<Self>(|self_ptr| {
let array_from_variant = sys::builtin_fn!(array_from_variant);
array_from_variant(self_ptr, sys::SysPtr::force_mut(variant.var_sys()));
})
};
// SAFETY: with_checked_type() is called right after, ensuring the array has the correct type.
let unchecked_array = unsafe { Self::unchecked_from_variant(variant) };

array.with_checked_type()
unchecked_array.and_then(|array| array.with_checked_type())
}
}

Expand Down
Loading
Loading