nikis05/derive-visitor

derive-visitor's Issues

Fallible visitors / early exit?

I can see some use cases where visitors might want to quit walking the hierarchy early, e.g. a validator visitor that exits on the first error, or a counter visitor that exits when a certain count is reached.

A few questions need to be answered:

1. Does anyone need this?
2. Should Visitor / Drive be subtraits of FallibleVisitor / FallibleDrive that panic on error, or should these be completely separate traits?
3. Are Result type, "fallible", etc applicable terminology for this? Early exit doesn't always mean error.

License

Hi @nikis05, I'm not sure what your plans are for this crate / repository, but I was wondering if you could include a license? It would make it easier for people. Thanks for this.

Zipping two visitors

Is it possible to zip two visitors? That is, walk through two structures of the same type in parallel and visit pairs of values?

Impl `Drive` on `&T`?

I was trying something like:

#[derive(Drive)]
struct MyType { ... }
let x: Option<&MyType> = ...;
x.drive(..)

However this does not work because:

429 |                         body.drive(&mut graph);
    |                              ^^^^^ method cannot be called on `Option<&Body>` due to unsatisfied trait bounds
   --> /rustc/65ea825f4021eaf77f1b25139969712d65b435a4/library/core/src/option.rs:563:1
    |
    = note: doesn't satisfy `std::option::Option<&gast::Body>: Drive`
    |
    = note: the following trait bounds were not satisfied:
            `&'a &gast::Body: derive_visitor::DerefAndDrive`
            which is required by `std::option::Option<&gast::Body>: Drive`

Here's my question: why not implement &T: Drive for all T: Drive and remove DerefAndDrive? Is there a specific reason for this approach?

Mutable visiting

Hello!
I have a use case for this crate where I would like to mutate a structure that is being visited. It looks like it is not currently possible. I would be happy to contribute a PR if you think this would be an useful addition to the crate.

breakage due to a bugfix in rustc

Your crate will unfortunately stop compiling once rust-lang/rust#121848 is merged. It unintentionally relies on an unsound bug of the type system which we are now fixing: rust-lang/rust#114061.

derive-visitor/derive-visitor/src/lib.rs

Lines 538 to 544 in 39dc342

	impl<T> Drive for T
	where
	T: 'static,
	// HRTB, because this needs to be true for late-bound lifetimes,
	// as both &self and &visitor have anonymous lifetimes.
	for<'a> &'a T: IntoIterator,
	for<'a> <&'a T as IntoIterator>::Item: DerefAndDrive,

This can overlap with the following impl

derive-visitor/derive-visitor/src/lib.rs

Lines 566 to 568 in 39dc342

	impl<T> Drive for Box<T>
	where
	T: Drive,

causing Box<Local> to implement Drive using two separate overlapping impls in the following example:

use std::iter;
struct Local;
impl<'a> IntoIterator for &'a Box<Local> {
    type IntoIter = iter::Once<&'a ()>;
    type Item = &'a ();
    fn into_iter(self) -> Self::IntoIter {
        iter::once(&())
    }
}

impl derive_visitor::Drive for Local {
    fn drive<V>(&self, _: &mut V) {}
}

fn impls_drive<T: derive_visitor::Drive>() {}

fn main() {
    impls_drive::<Box<Local>>();
}

The underlying issue is that, during the coherence overlap check, we consider for<'a> <&'a Box<_> as IntoIterator>::Item: DerefAndDrive to never apply, even though as shown above, it is possible to writean impl so that it does.

I think that there is unfortunately no way to fix the new error except by remove the blanket impl. I apologize for the this.

impl Drive for String and some others

I'm trying to use derive(Drive) with types generated by https://github.com/badicsalex/peginator/.

Unfortunately, a lot of these types contain String fields (and ops::range::RangeInclusive, but maybe that could be skipped) in ways that's not easy to detect (there are a lot of type alias shenanigans going on). This causes an error when using the derive macro.

Would it be possible to impl Drive for some more standard types? Or should this be solved differently?

Visit with closure

Hi,

I'm planning on using your crate for one of my projects, and I thought it would be great to have a default method on the Drive trait which accepts a closure instead of a full Visitor struct to reduce boilerplate.

Something along the lines of

let sum = 0;
my_complex_struct.visit_all(|node: &InnerStruct| { sum += node.value });

I'm pretty sure I'll implement this functionality in some kind of util.rs in the project, but it would be better to 'upstream' this functionality IMO. If you think too, I'd be happy to create a PR.

VisitorRef / DriveRef : storing references is the visitor

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
}