Skip to content

wasmparser: Make VisitOperator more user friendly #713

Open
@Robbepop

Description

@Robbepop

At the moment the new VisitOperator trait requires users to implement each and every of the over 500 variants.
This quickly becomes a mundane task, especially if there are only a few of the visit methods that are of importance to a user.

Idea 1

One idea is to introduce another default implemented method visit_fallback or visit_default (name not final) which acts similar to a default arm in a match expression. All visit_* methods would be default implemented to forward to this new visit_fallback method and the visit_fallback itself would be default implemented with something generic such as unimplemented!().

pub trait VisitOperator {
    type Output;

    fn visit_fallback(&mut self, offset: usize) -> Self::Output {
        unimplemented!()
    }
    
    fn visit_unreachable(&mut self, offset: usize) -> Self::Output {
        self.visit_fallback(offset)
    }
    
    fn visit_nop(&mut self, offset: usize) -> Self::Output {
        self.visit_fallback(offset)
    }
    
    // etc...
}

Advantages

This design would allow users to incrementally implement the VisitOperator trait and also would allow to only define small subsets of the VisitOperator trait API surface. An example where this could be useful is the wasmprinter crate which simply prints newline for most of the operators and could make heavy use of the visit_fallback method there.

Downsides

The huge downside to this design is that it is no longer clear to users if they have forgotten to implement a variant. Also if wasmparser is updated an added new variants users are no longer informed about this. I think this is a rather big disadvantage and was wondering how we could maybe find a solution that still informs users that want to take care about each and every visit_* method themselves.


Idea 2

We could introduce yet another VisitOperator-like trait. Let's call it VisitOperatorFallback.
It is very similar to the original VisitOperator trait in that it houses a visit method for each Wasm operator.
The difference to the original VisitOperator trait is that it has this new visit_fallback method and all the aforementioned default implementations.
Users can decide to either implement the original VisitOperator or VisitOperatorFallback trait.
In case they implement the VisitOperatorFallback (or just subsets of it due to its default impls) then the type also receive an auto implementation of VisitOperator that simply forwards to the methods of the VisitOperatorFallback.

pub trait VisitOperatorFallback {
    type Output;

    fn visit_fallback(&mut self, offset: usize) -> Self::Output {
        unimplemented!()
    }
    
    fn visit_unreachable(&mut self, offset: usize) -> Self::Output {
        self.visit_fallback(offset)
    }
    
    fn visit_nop(&mut self, offset: usize) -> Self::Output {
        self.visit_fallback(offset)
    }
    
    // etc...
}

impl<T> VisitOperator for T
where
    T: VisitOperatorFallback,
{
    type Output = <T as VisitOperatorFallback>::Output;
    
    fn visit_unreachable(&mut self, offset: usize) -> Self::Output {
        <T as VisitOperatorFallback>::visit_unreachable(self, offset)
    }
    
    fn visit_nop(&mut self, offset: usize) -> Self::Output {
        <T as VisitOperatorFallback>::visit_nop(self, offset)
    }
    
    // etc...
}

Advantages

With this design we have the same set of advantages as with solution 1 if users decide to implement VisitOperatorFallback trait.
If users only care about a subset of the visit methods they can implement VisitOperatorFallback. If they care about all the visit methods they can implement VisitOperator trait and still be updated about changes properly as with the original design.

Disadvantages

We now require 2 massive traits instead of just 1. However, we could use macro_rules or proc. macros to generate them to keep them in sync.

Metadata

Metadata

Assignees

No one assigned

    Labels

    wasmparserRelated to the binary format of WebAssembly (wasmparser)

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions