gimli-rs / object

A unified interface for reading and writing object file formats
https://docs.rs/object/
Apache License 2.0
658 stars 149 forks source link

Remove 'file lifetime from the Object trait #655

Closed mstange closed 6 months ago

mstange commented 6 months ago

The Object trait currently is defined as follows:

pub trait Object<'data: 'file, 'file>: read::private::Sealed {
    type Section: ObjectSection<'data>;
    fn section_by_index(&'file self, index: SectionIndex) -> Result<Self::Section>;
    // ...
}

It would be nicer if it were just this:

pub trait Object<'data>: read::private::Sealed {
    type Section<'file>: ObjectSection<'data> where Self: 'file, 'data: 'file;
    fn section_by_index(&self, index: SectionIndex) -> Result<Self::Section<'_>>;
    // ...
}

The 'file lifetime in the trait parameters is rather cumbersome to work with. I think it's only there because pre-1.65 versions of Rust didn't let you use associated traits which are generic over a lifetime.

If we're ready to make 1.65 the MSRV, then I think it's worth dropping this lifetime so that it's easier to write generic code which uses the Object trait. Here's a PR which implements that change.

This isn't blocking anything for me at the moment but it would allow some code cleanup.

philipc commented 6 months ago

I'm happy to see this change.

mstange commented 6 months ago

I don't understand the CI failure - is it compiling a mix of the crates.io version of object and the code in the repo?

philipc commented 6 months ago

It seems to be testing this PR merged with master. I wasn't aware github actions did that. I think you need to rebase and fix the error.

philipc commented 6 months ago

Do you want to try rebasing this?

mstange commented 6 months ago

Oh right! Yes, doing that now.