Deduplicate push()/extend() helpers as a default impl on TaggedStructure

[MarijnS95]

Having these functions generated for every root struct in our definitions file contributes to significant bloat, in part because of their massive documentation comments and not containing any generator-dynamic code or identifiers.

Now that trait Extends{Root} was generalized into a single trait Extends<Root> with the root structure as generic argument, it becomes possible to build on top of that and define a default trait function on every Vulkan structure. These functions require the "pushed" type to derive Extends<Self>, hence limiting the function to be called without ever having to track if the root structure is being extended.

[MarijnS95]

Named lifetimes are back otherwise the trybuild test no longer fails as expected.

[Ralith]

Note to self: play with these lifetimes to see if we can simplify any further before we commit.

[Ralith]

Alright, spent some time messing around with this.

First: It seems like a lifetime parameter is needed on whatever trait push abstracts over, because that's the only way we can cause inference to constrain the lifetime of the base struct being extended. Clauses like Self: 'a set a minimum bound on the lifetime of Self, which is the opposite of what we want.

Separately, I'm not sure the object safety achieved here is useful. IIRC (though I can't find the discussion) the motivation was to support stuff like Vec<Box<dyn Extends<T>>>. However, with this change, we need to specify lifetimes as part of the trait object. A Vec<Box<dyn Extends<'a, T>>> is much less useful, since it's not 'static; for example, it can't be safely passed through a mpsc. Outright passing 'static for the lifetime means the trait objects can't be pushed as an extension, since that would require narrowing their lifetime. If the caller unsafely transmutes 'static to 'a, then they're not getting useful lifetime checking, which is most of the benefit of push.

If we remove the object safety constraint, then it's no longer necessary to have Extends be a subtrait of AnyTaggedStructure, so Extends no longer needs a lifetime at all. We can get the lifetime we need in push with independent bounds like T: Extends<Self> + TaggedStructure<'a> which gets us back to the nice simple interface prior to this PR, provided that we impl TaggedStructure<'a> for Foo<'a>. The impl TaggedStructure<'_> for Foo<'_> currently in this PR is actually impl<'a, 'b> TaggedStructure<'a> for Foo<'b> which is too weak.

In short, I think the following diff, on top of this PR, plus a generator rerun, is a sweet spot:

diff --git a/ash/src/vk.rs b/ash/src/vk.rs
index 71ae233..02210c3 100644
--- a/ash/src/vk.rs
+++ b/ash/src/vk.rs
@@ -51,7 +51,7 @@ pub trait Handle: Sized {
 
 /// Iterates through the pointer chain. Includes the item that is passed into the function. Stops at
 /// the last [`BaseOutStructure`] that has a null [`BaseOutStructure::p_next`] field.
-pub(crate) unsafe fn ptr_chain_iter<'a, T: AnyTaggedStructure<'a> + ?Sized>(
+pub(crate) unsafe fn ptr_chain_iter<'a, T: TaggedStructure<'a> + ?Sized>(
     ptr: &mut T,
 ) -> impl Iterator<Item = *mut BaseOutStructure<'_>> {
     let ptr = <*mut T>::cast::<BaseOutStructure<'_>>(ptr);
@@ -69,7 +69,9 @@ pub(crate) unsafe fn ptr_chain_iter<'a, T: AnyTaggedStructure<'a> + ?Sized>(
 /// Structures implementing this trait are layout-compatible with [`BaseInStructure`] and
 /// [`BaseOutStructure`]. Such structures have an `s_type` field indicating its type, which must
 /// always match the value of [`TaggedStructure::STRUCTURE_TYPE`].
-pub unsafe trait AnyTaggedStructure<'a> {
+pub unsafe trait TaggedStructure<'a> {
+    const STRUCTURE_TYPE: StructureType;
+
     /// Prepends the given extension struct between the root and the first pointer. This method is
     /// only available on structs that can be passed to a function directly. Only valid extension
     /// structs can be pushed into the chain.
@@ -79,13 +81,13 @@ pub unsafe trait AnyTaggedStructure<'a> {
     /// # Panics
     /// If `next` contains a pointer chain of its own, this function will panic.  Call `unsafe`
     /// [`Self::extend()`] to insert this chain instead.
-    fn push<T: Extends<'a, Self> + ?Sized>(mut self, next: &'a mut T) -> Self
+    fn push<T: Extends<Self> + TaggedStructure<'a> + ?Sized>(mut self, next: &'a mut T) -> Self
     where
         Self: Sized,
     {
-        // SAFETY: All implementors of `AnyTaggedStructure` are required to have the `BaseOutStructure` layout
+        // SAFETY: All implementors of `TaggedStructure` are required to have the `BaseOutStructure` layout
         let slf_base = unsafe { &mut *<*mut _>::cast::<BaseOutStructure<'_>>(&mut self) };
-        // SAFETY: All implementors of `T: Extends<'a, >: AnyTaggedStructure` are required to have the `BaseOutStructure` layout
+        // SAFETY: All implementors of `TaggedStructure` are required to have the `BaseOutStructure` layout
         let next_base = unsafe { &mut *<*mut T>::cast::<BaseOutStructure<'_>>(next) };
         // `next` here can contain a pointer chain.  This function refuses to insert the struct,
         // in favour of calling unsafe extend().
@@ -111,7 +113,10 @@ pub unsafe trait AnyTaggedStructure<'a> {
     ///
     /// The last struct in this chain (i.e. the one where `p_next` is `NULL`) must be writable
     /// memory, as its `p_next` field will be updated with the value of `self.p_next`.
-    unsafe fn extend<T: Extends<'a, Self> + ?Sized>(mut self, next: &'a mut T) -> Self
+    unsafe fn extend<T: Extends<Self> + TaggedStructure<'a> + ?Sized>(
+        mut self,
+        next: &'a mut T,
+    ) -> Self
     where
         Self: Sized,
     {
@@ -132,17 +137,6 @@ pub unsafe trait AnyTaggedStructure<'a> {
     }
 }
 
-/// Non-object-safe variant of [`AnyTaggedStructure`], meaning that a `dyn TaggedStructure` cannot
-/// exist but as a consequence the [`TaggedStructure::STRUCTURE_TYPE`] associated constant is
-/// available.
-///
-/// [`AnyTaggedStructure`]s have a [`BaseInStructure::s_type`] field indicating its type, which must
-/// always match the value of [`TaggedStructure::STRUCTURE_TYPE`].
-pub unsafe trait TaggedStructure<'a>: AnyTaggedStructure<'a> {
-    const STRUCTURE_TYPE: StructureType;
-}
-unsafe impl<'a, T: TaggedStructure<'a>> AnyTaggedStructure<'a> for T {}
-
 /// Implemented for every structure that extends base structure `B`. Concretely that means struct
 /// `B` is listed in its array of [`structextends` in the Vulkan registry][1].
 ///
@@ -150,7 +144,7 @@ unsafe impl<'a, T: TaggedStructure<'a>> AnyTaggedStructure<'a> for T {}
 /// their structure is layout-compatible [`BaseInStructure`] and [`BaseOutStructure`].
 ///
 /// [1]: https://registry.khronos.org/vulkan/specs/latest/styleguide.html#extensions-interactions
-pub unsafe trait Extends<'a, B: AnyTaggedStructure<'a>>: AnyTaggedStructure<'a> {}
+pub unsafe trait Extends<B> {}
 
 /// Holds 24 bits in the least significant bits of memory,
 /// and 8 bytes in the most significant bits of that memory,
@@ -281,7 +275,7 @@ pub(crate) fn debug_flags<Value: Into<u64> + Copy>(
 #[cfg(test)]
 mod tests {
     use crate::vk;
-    use crate::vk::AnyTaggedStructure as _;
+    use crate::vk::TaggedStructure as _;
     use alloc::vec::Vec;
     #[test]
     fn test_ptr_chains() {
@@ -343,21 +337,6 @@ mod tests {
         assert_eq!(chain, chain2);
     }
 
-    #[test]
-    fn test_dynamic_add_to_ptr_chain() {
-        let mut variable_pointers = vk::PhysicalDeviceVariablePointerFeatures::default();
-        let variable_pointers: &mut dyn vk::Extends<'_, vk::DeviceCreateInfo<'_>> =
-            &mut variable_pointers;
-        let chain = alloc::vec![<*mut _>::cast(variable_pointers)];
-        let mut device_create_info = vk::DeviceCreateInfo::default().push(variable_pointers);
-        let chain2: Vec<*mut vk::BaseOutStructure<'_>> = unsafe {
-            vk::ptr_chain_iter(&mut device_create_info)
-                .skip(1)
-                .collect()
-        };
-        assert_eq!(chain, chain2);
-    }
-
     #[test]
     fn test_debug_flags() {
         assert_eq!(
diff --git a/ash/tests/fail/long_lived_root_struct_borrow.rs b/ash/tests/fail/long_lived_root_struct_borrow.rs
index a73837a..79df8d7 100644
--- a/ash/tests/fail/long_lived_root_struct_borrow.rs
+++ b/ash/tests/fail/long_lived_root_struct_borrow.rs
@@ -1,5 +1,5 @@
 use ash::vk;
-use vk::AnyTaggedStructure as _;
+use vk::TaggedStructure as _;
 
 fn main() {
     let mut layers = vec![];
diff --git a/generator/src/lib.rs b/generator/src/lib.rs
index 1f2d391..e021f8e 100644
--- a/generator/src/lib.rs
+++ b/generator/src/lib.rs
@@ -2339,7 +2339,7 @@ fn derive_getters_and_setters(
         .map(|extends| {
             let base = name_to_tokens(extends);
             // Extension structs always have a pNext, and therefore always have a lifetime.
-            quote!(unsafe impl<'a> Extends<'a, #base<'a>> for #name<'a> {})
+            quote!(unsafe impl Extends<#base<'_>> for #name<'_> {})
         });
 
     let impl_structure_type_trait = structure_type_field.map(|member| {
@@ -2353,7 +2353,7 @@ fn derive_getters_and_setters(
 
         let value = variant_ident("VkStructureType", value);
         quote! {
-            unsafe impl TaggedStructure<'_> for #name<'_> {
+            unsafe impl<'a> TaggedStructure<'a> for #name<'a> {
                 const STRUCTURE_TYPE: StructureType = StructureType::#value;
             }
         }

[Ralith]

I also experimented briefly with something like

trait TaggedStructure {
    type Abstract<'a>;
    fn narrow<'a>(self) -> Self::Abstract<'a>
        where Self: 'a;
}

but that would require a fn narrow implementation on every type, which I think undermines much of the benefit.