gtk-rs-core icon indicating copy to clipboard operation
gtk-rs-core copied to clipboard

[FEATURE REQUEST] PtrArray has no methods

Open arcnmx opened this issue 2 years ago • 4 comments

The only way to interact with a PtrArray is to use ToGlibContainerFromSlice, a very unwieldy extension trait (honestly just impl FromIterator and Extend instead?). It's impossible to even construct an empty array without doing some type gymnastics to invoke the trait. Its methods aren't bound at all, and the lack oftranslate traits like FromGlib/ToGlibPtr complicates its ownership as there's no public way to get a pointer to it once created.

It's a bit of a complicated type due to the nature of its (lack of) memory management, but at least some ability to manipulate them would be nice!

EDIT: FromGlibPtrArrayContainerAsVec and friends are frankly horrifying.

arcnmx avatar Jan 31 '22 19:01 arcnmx

The support for it is intentionally minimal because it's not a very useful type: for Rust code, use a Vec and for FFI it's badly designed (see e.g. https://gitlab.gnome.org/GNOME/gobject-introspection/-/issues/305).

Why/where do you want to use it?

FWIW, for the next release I'm planning to get rid of the strange container FFI traits.

sdroege avatar Feb 01 '22 08:02 sdroege

Any updates on this @arcnmx ?

bilelmoussaoui avatar Sep 20 '23 07:09 bilelmoussaoui

I'm still using glib::ffi::g_ptr_array_xxx manually here. Regarding the linked issue and related safety concerns, I suppose I'd opine that most methods can and should be bound, and at the very least it should impl the usual translate traits like FromGlib/ToGlibPtr (regardless of its contents, the array itself is fine to ref/unref as usual). The main question becomes what usage is considered unsafe... I see two main approaches here:

  1. it could be treated as explicitly only a safe wrapper around a GPtrArray with a NULL free_func. Then transfer is irrelevant, there are no guarantees about the validity of any pointers contained, making it roughly equivalent to a Vec<gpointer> in terms of usability and safety - the safety concerns only become an issue once it is passed to/from a function, crossing ffi.
    • new_full/new_with_free_func/set_free_func would be considered unsafe, and would create an array value that needs to be kept private to ensure only valid pointers are added to it - it would be unsafe to hand out references to safe code (even retroactively)
    • from_glib_full(ptr_array) would only create a "safe" value if the underlying array is guaranteed to not have a free func
    • for the most part, this type would exist to facilitate FFI bindings that want to construct or access an array provided to/from an external library
  2. You could get away with making all arrays safe to instantiate and pass around, but then any functions that add or extend the array would need to be unsafe
    • from_glib_full(ptr_array) would (assuming it contains valid pointers and a valid free func) in most cases create a "safe" value, making it less dangerous to "expose" these values to pub code
    • even so, you generally wouldn't make it part of a public API without a wrapper, because all you get are gpointers out of the array without any type information
    • instantiation with values could still be made safe (PtrArray::empty(), impl Default for PtrArray, impl FromIterator<gpointer> for PtrArray, PtrArray::with_slice()?, etc), and it could even expose helper constructors like PtrArray::with_objects() that sets it up with set_free_func(g_object_unref)

I think I'm leaning toward option 2 personally? Future improvement here might mean introducing strongly-typed ObjectPtrArray<T> wrappers or something to make these safe and actually useful to work with as arrays in Rust.

arcnmx avatar Sep 22 '23 16:09 arcnmx

Personally I would go with the current situation and directly use FFI when these types are needed, and expose them per use-case in a meaningful way in Rust. As there are no general rules for these types, you'll basically end up doing that anyway unless you want to make life hard for your users.

sdroege avatar Sep 25 '23 09:09 sdroege