Issue with lifetimes and Vec (lifetime may not live long enough requires that `'bar` must outlive `'static`)

[lowlevl]

Hi there,

Upon rewriting some of my binrw-using code with lifetimes, I stumbled upon some interesting issue with what the macro generates about Vec and lifetimes, mainly that they seem incompatible, while the same struct without the Vec seems to compile fine.
I managed to make a minimal reproduction of the issue.

So considering a struct Zoo<'bar>, defined as such:

#[binrw]
struct Zoo<'bar>(std::marker::PhantomData<&'bar ()>);

This snippet compiles fine, even with the 'bar lifetime.

#[binrw]
struct Foo<'bar> { 
    zoo: Zoo<'bar>,
}

But this one fails.

#[binrw]
struct Foo<'bar> {
    #[br(count = 1)]
    zoos: Vec<Zoo<'bar>>,
}

Here is the compiling error:

error: lifetime may not live long enough
    |
249 | #[binrw]
    | ^^^^^^^^ requires that `'bar` must outlive `'static`
250 | struct Foo<'bar> {
    |            ---- lifetime `'bar` defined here
    |
    = note: this error originates in the attribute macro `binrw` (in Nightly builds, run with -Z macro-backtrace for more info)

I started investigating the macro expansion, but unfortunately was unable to pinpoint an exact cause for this.

Let me know if you need any more context or help !
Thanks =)

[csnover]

Thanks for your report! This lifetime issue exists within the generated BinWrite implementation. Investigating, it is not immediately clear to me where the 'static constraint is coming from. Manually knocking out errors eventually gets to an error that self is escaping into the write function and that doesn’t make a lot of sense to me either. I think will have to consult with others to understand.

[kitlith]

@csnover The 'static constraint is coming from the implementations of BinRead and BinWrite on Vec<T>:

binrw/binrw/src/binwrite/impls.rs

Lines 123 to 148 in 22d9a4d

	impl<T> BinWrite for Vec<T>
	where
	T: BinWrite + 'static,
	for<'a> T::Args<'a>: Clone,
	{
	type Args<'a> = T::Args<'a>;
	fn write_options<W: Write + Seek>(
	&self,
	writer: &mut W,
	endian: Endian,
	args: Self::Args<'_>,
	) -> BinResult<()> {
	if let Some(this) = <dyn Any>::downcast_ref::<Vec<u8>>(self) {
	writer.write_all(this)?;
	} else if let Some(this) = <dyn Any>::downcast_ref::<Vec<i8>>(self) {
	writer.write_all(bytemuck::cast_slice(this.as_slice()))?;
	} else {
	for item in self {
	T::write_options(item, writer, endian, args.clone())?;
	}
	}
	Ok(())
	}
	}

which is a direct result of implementing fake-specialization for Vec<{integer}>.

In order to remove the 'static constraint, we either need to find a different method of fake-specialization that works for our needs, or we need to provide a secondary type/parser/writer that doesn't specialize.

@lowlevl you can work around this by defining a different Vec<T> type that's less complicated. With the caveat that I haven't tried compiling this yet, and it may need tweaking, something like:

struct SlowVec<T>(pub Vec<T>);

impl<T> BinRead for SlowVec<T>
where
    T: BinRead,
    for<'a> T::Args<'a>: Clone,
{
    type Args<'a> = binrw::binread::VecArgs<T::Args<'a>>;

    fn read_options<R: Read + Seek>(
        reader: &mut R,
        endian: Endian,
        args: Self::Args<'_>,
    ) -> BinResult<Self> {
        Ok(SlowVec(core::iter::repeat_with(|| T::read_options(reader, endian, args.clone()))
                .take(n)
                .collect()?))
    }
}

impl<T> BinWrite for SlowVec<T>
where
    T: BinWrite,
    for<'a> T::Args<'a>: Clone,
{
    type Args<'a> = T::Args<'a>;

    fn write_options<W: Write + Seek>(
        &self,
        writer: &mut W,
        endian: Endian,
        args: Self::Args<'_>,
    ) -> BinResult<()> {
        for item in self.0 {
            T::write_options(item, writer, endian, args.clone())?;
        }

        Ok(())
    }
}

and then using SlowVec instead of Vec where appropriate.

[kitlith]

In terms of moving away from this 'static requirement, https://users.rust-lang.org/t/emulate-specialization-in-stable-rust-with-macro-rules/74338/4 describes a way we could make this work.

Essentially, we'd add a new marker trait (bikeshed name) NaiveVec that is implemented on most types, and then the default binread/write impls for Vec would be bounded on NaiveVec. The integers (and any other types we want to specialize) would not implement NaiveVec, and therefore not overlap with the generic impl. Our derive macros would also implement NaiveVec by default, with an opt-out of #[brw(specialize_vec)] to allow the user to write their own impl. (maybe split into seperate read/write traits? idk.)

[csnover]

Oh, I’ll look into that. Initially when you said that, I just thought about autoref specialisation. If there is no sign of stable specialisation on the horizon at some point I wonder if just changing to always require helpers for optimisations is a way to go, since these workarounds for missing language features are clever but they also seem to end up like evergreen footguns, and the workarounds for the workarounds get less cute.

[kitlith]

we could also avoid specialization by adding another method that reads/writes multiple items at once, which defaults to the naive method. Then, the Vec impl would forward to the read/write multiple method, which could be overridden on a per-type basis.

on the other hand, if we go the route of removing the specialization-like optimization entirely, we should re-evaluate how we push people towards the helpers and/or bufreader.

[kitlith]

It appears dtolnay has struck again, and has written a typeid crate that has an option to not rely on 'static: https://github.com/dtolnay/typeid

The example usecase is basically exactly our usecase.

EDIT: It doesn't actually implement downcast(_mut), just obtaining a TypeId. Implementing downcast_mut on our end would require unsafe, which we currently appear to have informally forbidden. (informally because #![forbid(unsafe_code)] does not appear anywhere, but we have two instances of comments talking about avoiding the use of unsafe.)