ash-rs
Struct lifetimes make `ash` "unusable" in FFI crates
We removed builder structs, together with the .build() footgun and moving their lifetimes to the underlying repr(C) struct in #602. This goes somewhat against the design of -sys crates which ash was anyway ahead of, and is in this funny spot where it provides both low-level representations as well as high-level hand-holds within the same crate and on the same types.
On many FFI crates that I've been generating lately we want to use our repr(C) structures in generated FFI bindings from bindgen using allowlist/blocklist rather than letting it generate completely unusable Vulkan FFI types for us (that are incompatible and require transmutes):
use ash::vk::{SubmitInfo as VkSubmitInfo};
// include!("bindings.rs");
extern "C" {
pub fn SomethingExternal(..., submits: *const VkSubmitInfo) -> ...;
}
Which will fail to compile because of the missing lifetime on VkSubmitInfo. There is a workaround however, but it doesn't seem ideal:
type VkSubmitInfo = ash::vk::SubmitInfo<'static>;
// include!("bindings.rs");
extern "C" {
pub fn SomethingExternal(..., submits: *const VkSubmitInfo) -> ...;
}
This requires transmuting the lifetime away into 'static if this struct is borrowing something on the heap, which is true for VkSubmitInfo as it references a bunch of slices.
As for a solution, I'm not sure if we should be reintroducing builders because of this. Or something else entirely, while keeping a .build() much better hidden/guarded by for example requiring all high-level Ash functions to take a high-level lifetimed structure?
If we perform such a type split again however, it paves the way for proper ash-sys separation. Additionally we've discussed borrows (for non-DSTs) being ABI compatible with raw pointers, such that we could even generate structs twice with borrows on (most) fields to allow the user to use regular FRU instead of the builder pattern; all while having a zero-copy (unsafe) transmute into the FFI type-struct.
There is a workaround however, but it doesn't seem ideal:
I don't think this is worse than using a type that has no lifetimes at all. Ideally, of course, you should have bindings that accurately annotate lifetimes, but I appreciate that bindgen may not make this easy -- and in that case there's no harm in what you've illustrated where necessary, while still allowing better behavior where possible.
I'm not sure if we should be reintroducing builders because of this
It's not clear why we should do this, when the solution you illustrated works just fine.
Additionally we've discussed borrows (for non-DSTs) being ABI compatible with raw pointers, such that we could even generate structs twice with borrows on (most) fields
Last time we discussed this, we concluded that it was best to be consistent. I don't think that's changed.
I don't think this is worse than using a type that has no lifetimes at all. Ideally, of course, you should have bindings that accurately annotate lifetimes, but I appreciate that bindgen may not make this easy -- and in that case there's no harm in what you've illustrated where necessary, while still allowing better behavior where possible.
Right, we may have discussed in the past that this was an acceptable trade-off but I can no longer find the sample to reference.
I'm not sure if we should be reintroducing builders because of this
It's not clear why we should do this, when the solution you illustrated works just fine.
While it works, a transmute seems somewhat unclear and not very self-descriptive (even if the user has to place a sensible safety comment either way). At the same time type renames are also necessary to add the Vk prefix, likewise because of lacking a -sys crate.
Not sure if there's really something actionable here, besides documenting our outcomes in some sort of FFI chapter for bindgen usage? Additionally, we could make an ffi.rs module with contents like the following:
use super::vk::*;
pub type VkStructName = StructName<'static>;
impl<'a> StructName<'a> {
pub const unsafe fn drop_lifetime_for_ffi(&self) -> &VkStructName {
core::mem::transmute(self)
}
}
Additionally we've discussed borrows (for non-DSTs) being ABI compatible with raw pointers, such that we could even generate structs twice with borrows on (most) fields
Last time we discussed this, we concluded that it was best to be consistent. I don't think that's changed.
You mean consistency between static-sized versus dynamic-sized types (i.e. slices)? Not sure if that was the outcome, or if we just "never tried" to generate Option<&T> for non-count fields 😅
Additionally, we could make an ffi.rs module with contents like the following
That seems reasonable to me, if there's no more direct way to tell bindgen to translate the types. Could feature-gate it if it has measurable compile-time impact. Could even be a separate crate, but then the lifetime cast would have to be an extension trait and that's a bit ugly, not to mention the extra release management labor.
You mean consistency between static-sized versus dynamic-sized types (i.e. slices)?
Right. I think switching to Option<&T> everywhere is really appealing, but it wouldn't entirely solve the problem and would increase surprise for cases that don't work. We could consider doing it anyway (it would save a lot of setters...) but I think that needs to wait for after the next major release at minimum. Doesn't seem related to the FFI problem anyway.
It is possible with bindgen's ParseCallbacks::item_name(), but a .strip_prefix("Vk") is a way too large hammer. Replacing that with something that returns ash::vk::PrefixStrippedType is not allowed, because that's "not a valid Ident…
Additionally libraries may be defining their own wrapper types that start with Vk, requiring extra work to filter them out (ffx-api has VkQueueInfoFFX already for example).
Not to mention, this would somehow have to append the <'static> lifetime to the name too. Doing both in an ffi.rs of sorts in ash seems like the easier route at that point, with drop_lifetime_for_ffi() as a bonus.
If there's any compile-time hit at all, we could even decide to hide the module behind cfg(feature = "ffi").
But let's first finish the generator changes that hide experimental things behind a cfg before we move on.
👍 for leaving borrows in FFI pointers for another time, we should have a tracking issue open around it anyway.