nikis05 / derive-visitor

MIT License
20 stars 6 forks source link

VisitorRef / DriveRef : storing references is the visitor #15

Open Wardenfar opened 8 months ago

Wardenfar commented 8 months ago

Hi,

First of all, thanks for this helpful crate. I need to store references of visited structs in my visitor, after a bit of trying it seems impossible with the current definition of Drive and Visitor.

I propose to add a new pair of trait DriveRef and VisitorRef (see the code below). The only downside I've found is the incompatibility with interior mutable types like Mutex, RefCell, ... For example, when you call borrow() on a RefCell, you get back a guard that can be deref() to my struct, but this reference is tied to the guard.

In case this proposal fit to your crate, I will gladly submit a PR :)

trait VisitorRef<'a> {
    fn visit(&mut self, item: &'a dyn Any, event: Event);
}

trait DriveRef: Any {
    fn drive<'a, V: VisitorRef<'a>>(&'a self, visitor: &mut V);
}

#[derive(VisitorRef)]
struct ExampleVisitor<'a> {
    stored_ref: Option<&'a ExampleDrive>,
}

#[derive(DriveRef)]
struct ExampleDrive {
    foo: bool
}
nikis05 commented 8 months ago

Hey @Wardenfar, you're welcome!

I don't think it is a good idea to create too many different versions of visitor / drive, we already have 2 for mutable / non-mutable visits. So I think it would be simpler if you just added lifetimes to existing traits, since it would still be pretty trivial to use with 'static non-reference-storing visitor types.

If you wanna do it, please update the docs to reflect the change. I think it would also be helpful to add an example illustrating a reference-storing visitor, specifically denoting the name of the lifetime to use, and a note on why DriveMut / VisitorMut don't have lifetimes (can't hold multiple references into the same thing).

Also, since the lifetime will be brought into the scope by the macro, I think we need to give it a more long and descriptive name, like 'visitor. This way it will not conflict with other user declared lifetimes, and it will be clearer what it is borrowing from.

#[derive(Visitor)]
struct SomeStructThatAlsoHoldsReferenceToSomethingUnrelated<'a, 'visitor> {
  stored_ref: &'visitor ExampleItem,
  something_unrelated: &'a SomethingUnrelated,
}