Relaxing 'static lifetime bounds?

[robert-chiniquy]

Just ran into this one trying to pass an object (which impl's Language) holding a non-static-lifetime reference to this method:

impl<L: Language, N: Analysis<L>> Rewrite<L, N>
{
    /// Create a new [`Rewrite`]. You typically want to use the
    /// [`rewrite!`] macro instead.
    ///
    pub fn new(
        name: impl Into<Symbol>,
        searcher: impl Searcher<L, N> + Send + Sync + 'static,
        applier: impl Applier<L, N> + Send + Sync + 'static,
    ) -> Result<Self, String> {

Of course, this pops up in more places than just here. I know it's a big task. Is this already on your radar, or something you'd welcome a PR for?

[mwillsey]

Yes, I'm open to a PR that removes this bound. Last time I recall messing with this (a while ago), it was a little tricky. But if you can remove those bounds without breaking the API go for it.

[Evan-Zhao]

Hi @robert-chiniquy I'd like to take a stab at this, but I need some more context on your use case. Say we make it 'a instead of 'static. How long is the 'a in your object that you're passing?

Also if we put an 'a to the new function, I don't see a way to not also do this:

pub struct Rewrite<'a, L, N> {
    // ...
    pub searcher: Arc<dyn Searcher<L, N> + Sync + Send + 'a>
    // ...
}

The added lifetime param is fixable within the crate (well I'm still figuring out how to), but it also breaks the public API. Maybe we can do something like
type Rewrite<L, N> = RewriteInternal<'static, L, N>? Then if you need the non-static version, you import and work with RewriteInternal.

[robert-chiniquy]

Hi @Evan-Zhao ! Awesome to hear you're thinking about this.

I've been kind of stumped myself, this quickly became harder than I expected to extricate.

I like your RewriteInternal suggestion, that hadn't occurred to me.

The 'a in my use case is for AST/IR objects which don't derive from static strings but which are longer than the lifetime of the egraph/runner, and which don't need to be mutable, so I would think keeping any references to them within the rewrites should be fine.

[Evan-Zhao]

I see, makes a lot of sense.

I was looking at the codebase and happy to find out the searcher and applier (which we'll have to change type signature of) aren't actually used very intensively -- because I saw Arc and thought of threads, async, etc., but none of that exists in the use cases.
So I actually have a ~20 lines patch which just involves replacing 'static with 'a in the right places.
It compiles and passes the existing tests, but these tests don't use this 'a lifetime and we need a few tests that do.
(I hope I didn't mess something up and it is actually straightforward like this.)

How about I open a PR today, and @robert-chiniquy would you take a look at that PR, and make your use case into a test case that practices non-static lifetime in the rewriter? Then we'll see if @mwillsey likes it.

Also I decided to name it RewriteA<'a, L, N> because we'll also be exposing RewriteA and it shouldn't be called internal. Suggestions for better naming is always welcome :)

[robert-chiniquy]

RewriteBorrow or something maybe? No idea about names. I can probably whip up a fake test case which exercises the non-static lifetimes. Thanks!