What is the motivation of C# ExpressionVisitor's implementation?

Question

I have to design a solution for a task, and I would like to use something theoretically similar to C#'s ExpressionVisitor.

For curiosity I opened the .NET sources for ExpressionVisitor to have a look at it. From that time I've been wondering why the .NET team implemented the visitor as they did.

For example MemberInitExpression.Accept looks like this:

protected internal override Expression Accept(ExpressionVisitor visitor) {
    return visitor.VisitMemberInit(this);
}

My - probably noob - question is: does it make any sense? I mean shouldn't the Accept method itself be responsible of how it implements the visiting within itself? I mean I've expected something like this (removing the internal visibility to be overridable from outside):

protected override Expression Accept(ExpressionVisitor visitor) {
    return this.Update(
            visitor.VisitAndConvert(this.NewExpression, "VisitMemberInit"),
            visitor.Visit(this.Bindings, VisitMemberBinding)
            );
}

But this code is in the base ExpressionVisitor's VisitMemberInit method, which gets called from MemberInitExpression.Accept. So seems like not any benefit of the Accept implementation here.

Why not just process the tree in the base ExpressionVisitor, and forget about all the Accept methods?

I hope you understand my points, and hope someone could shed some light on the motivation behind this implementation. Probably I don't understand the Visitor pattern at all?...

Answer 1

A visitor can override the way any expression is visited. If your proposal was implemented in all places the visitor never would be called. All the visitation logic would be in the overrides of Accept. Non-BCL code cannot override this method.

If you write visitor.Visit((Expression)this.SomeExpression) (like you do in the question) then how are you going to perform dynamic dispatch on the type of SomeExpression? Now the visitor has to perform the dynamic dispatch. Note, that your 2nd code snippet makes the simplifying assumption that all sub-expressions to be visited have known type. Try to write the code for BinaryExpression to see what I mean.

Maybe I did not understand something but this proposal does not make sense.

The purpose of the Accept method is a performance optimization. Each accept is a virtual call which is rather cheap. The alternative would be to have a huge switch in the visitor over the expression type (which is an enum). That's probably slower.