Add Visitor Pass

[willemneal]

An abstract visitor method is added to each of the base abstract classes for a node in the ast and program element.

For example,

class IfStatement extends Statement {
  visit<T extends StatementVisitor>(visitor: T): void {
    visitor.visitIfStatement(this);
   } 
}

class BaseVisitor {
....
  visitIfStatement(stmt: IfStatement): void {
    stmt.condition.visit(this);
    stmt.trueBranch.visit(this);
    if (stmt.falseBranch) stmt.falseBranch.visit(this);
  }
}

Subclasses BaseVisitor can overwrite the implementation of visitIfStatement and continue to visit member nodes past it in the tree with super.visitIfStatement(stmt).

I also added an AbstractVisitor class, which defines a polymorphic visit function. This function expects an input of a type `Collection:

type Collection<T> = T | T[] | Map<string, T> | Iterable<T> | null;

This means it handles iterating through different types of collections.

The default visit function ignores null. So we can pass it possibly null references. This way the previous function can be rewritten:

visitIfStatement(stmt: IfStatement): void {
    this.visit(stmt.condition);
    this.visit(stmt.trueBranch);
    this.visit(stmt.falseBranch);
  }

Visitor classes must export their class as the default export. It is then constructed after the module in binaryen and passed the current parser and compiler.

The ultimate goal is to use the pass to add the interface/virtual table support.

fixes #702

[dcodeIO]

The first thing that comes to my mind here is that there's a concrete AST visitor in src/extra/ast.ts already that can be made abstract (or traverse by default if a method isn't explicitly overloaded) to become reusable. That one is also standalone and type-safe by design, and as such can serve as a template to implement an IR visitor taking a similar approach.

[willemneal]

I started with the visitor in src/extra/ast.ts, but instead of relying on the a big switch statement to determine which node is visited, the node's visit method is passed the visitor, which then calls the corresponding visit method for that node's type passing itself to the visit method.

This would be a good opportunity to split up the pipeline into separate passes of visitors. This way we can have a type checking phase that can be done before compiling to binaryen. Also we could create an internal IR that is closer to Binaryen so that the final compilation to it is simple, while also allowing for higher level abstractions like classes. This will make it easier to write the IR by hand, which I've experienced when trying to create the virtual method, which is here. Furthermore, having this new IR we could add compiler to Wasm directly at some point.

[dcodeIO]

instead of relying on the a big switch statement to determine which node is visited, the node's visit method is passed the visitor, which then calls the corresponding visit method for that node's type passing itself to the visit method.

Can you elaborate why you think that this is better? On a first glimpse, this looks like a less performant and less portable approach to do the same basic thing.

Also we could create an internal IR that is closer to Binaryen so that the final compilation to it is simple,

Currently we are reusing Binaryen's IR, which is about as close and simple as it can get. Adding another intermediate layer between the AST and Binaryen IR seems unnecessary to me, since one could directly use the Binaryen API instead. In a sense, the two representations we have are a) Stuff in TS and b) Stuff in Wasm, with the program elements sitting in between to keep the necessary state for high-level structures.

Furthermore, having this new IR we could add compiler to Wasm directly at some point.

Would say that we'd want to do either integration with Binaryen or roll our own, the latter strictly requiring additional IR, but not both. Adding more IR than necessary, that then might significantly slow-down compilation and increase memory footprint, doesn't appear worth it.

[willemneal]

I don't understand how it's less portable. Because it uses interfaces and abstract methods?

node.visit(visitor) -> (in ifnode.visit) visitor.visitIfnode(this), is the same as a visit with visitNode(n: node) -> switch (node instanceof ifnode) this.visitIfnode(node)

situations like,

// expressions

      case NodeKind.FALSE:
      case NodeKind.NULL:
      case NodeKind.SUPER:
      case NodeKind.THIS:
      case NodeKind.TRUE:
      case NodeKind.CONSTRUCTOR:
      case NodeKind.IDENTIFIER: {
        this.visitIdentifierExpression(<IdentifierExpression>node);
        break;
      }

becomes solved by having the IdentifierExpression have the visit method, which all the subclasses will inherit. Furthermore, throughout extras/ast.ts casts are used frequently, e.g.

visitLiteralExpression(node: LiteralExpression): void {
    switch (node.literalKind) {
      case LiteralKind.FLOAT: {
        this.visitFloatLiteralExpression(<FloatLiteralExpression>node);
        break;
      }
      case LiteralKind.INTEGER: {
        this.visitIntegerLiteralExpression(<IntegerLiteralExpression>node);
        break;
      }
......

Here each subclass of LiteralExpression would implement the visit function making this:

visitLiteralExpression(node: LiteralExpression): void {
    this.visit(node);
    super.visitLiteralExpression(node); 
    // This allows us to keep traversing since the base class traverses the ast.

My main reason for this switch is readability and a more standardized way of doing passes over the ast/program. So in that vain the only thing that really matters is that there is a base visitor that you can extend that allows you to overwrite just the methods you want and still have a way to call super.visit... I just found this an easier and more readable way to get there.

I also would be very shocked if this impacted performance significantly as it only adds a small constant cost. Again as long as everything hinges on the use of the base class we can swap out the implementation with another.

As for adding a new IR before binaryen; currently it's difficult to hand write IR with both higher level and lower level abstractions. For example, I'm currently trying to write the virtual method.

function virtual(methodID: usize, classID: usize): usize {
             //switch
}

I had trouble writing the switch statement in the current AST, because it needed to resolve methodID, which ideally could be represented in the AST as a local_get as it is in binaryen. I had the best success with just parsing new source code and was able to get a simple interface with one function working. Still tons left, but it is a good start.

[dcodeIO]

Closing this PR as part of 2020 vacuum as it seems to be outdated. Iirc there was a new suggestion to add a visitor to extra/ast meanwhile, which seems good to do as long as we can keep it reasonably simple.