Make `ASTGenVisitor` not conform to `SyntaxTransformVisitor`

[ahoppen]

The conformance of ASTGenVisitor to SyntaxTransformVisitor is holding us back at the moment because all visit methods need to return an ASTNode, which looses type information. The only thing that we get from SyntaxTransformVisitor is the dispatch to a node-specific visit method, which is actually also harmful because we fatalError in case we haven’t implemented the AST generation for that node yet.

What we should do instead is:

1. Remove the conformance of ASTGenVisitor to SyntaxTransformVisitor.
2. Implement a visit(Syntax) method similar to SytnaxTransformVisitor.visit(Syntax) but that only handles the node kinds that we do actually support in ASTGen
3. Once we have Remove usages of void * in CASTBridging.h #68346, change e.g. visit(TypeAliasDeclSyntax) to return a TypeAliasDecl instead of an ASTNode.
4. And if we are no longer tied to SyntaxTransformVisitor, we might as well rename the visit methods to generate since that’s a more descriptive name.

[saehejkang]

@ahoppen is anyone working on this? I would like to pick it up. I am new to working on the swift codebase 😁

[ahoppen]

No, nobody is working on it yet. If you want to work on this, I would base your work on top of #67111. We hope to get that PR merged soon-ish and because it’s such a big PR that has been open for a long time, I want to merge it before making any other significant changes to ASTGen. If you base your work on top of #67111, you can rebase when the PR is merged and then we have a clean PR to review.

[saehejkang]

i will take stab at it soon, just working on setting up the entire environment

[saehejkang]

@ahoppen Looking at ASTGen.swift I can see 5 node kinds that need to be handeled. Is that correct?

MemberBlockItemSyntax
InitializerClauseSyntax
ConditionElementSyntax
CodeBlockItemSyntax
ArrayElementSyntax

I updated those methods visit methods like so below

  public func generate(_ node: MemberBlockItemSyntax) -> ASTNode {
    visitAny(Syntax(node.decl))
  }

With regards to a new visit(Syntax), I added it to that ASTGenVisitor extension

    public func visit(_ node: Syntax) -> ASTNode {
        switch node.as(SyntaxEnum.self) {
        case .memberBlockItem(let node):
            return generate(node)
        case .initializerClause(let node):
            return generate(node)
        case .conditionElement(let node):
            return generate(node)
        case .codeBlockItem(let node):
            return generate(node)
        case .arrayElement(let node):
            return generate(node)
        }
    }

But, the switch-case statement needs to be exhaustive, so would I need to add the other cases and just add break statements? I just want to make sure that we are supposed to have a massive switch-case statement even if all other node types are not handled.

I just want to make sure I am on the right track 😄

[ahoppen]

Looking at ASTGen.swift I can see 5 node kinds that need to be handeled. Is that correct?

There are extensions of ASTGenVisitor in e.g. Exprs.swift. The visit functions in there also need to be implemented.

But, the switch-case statement needs to be exhaustive, so would I need to add the other cases and just add break statements? I just want to make sure that we are supposed to have a massive switch-case statement even if all other node types are not handled.

You can add a default case that just has fatalError("Not implemented."). Currently we fall back to calling visitAny, which fatal errors.

[saehejkang]

what does it mean by this error?

Value of optional type 'ASTNode?' must be unwrapped to refer to member 'rawValue' of wrapped base type 'ASTNode'

My thought process to solving this issue is to wrap the passed parameter to a Syntax type

let callee = visit(Syntax(node.calledExpression)).rawValue

[ahoppen]

In which line are you seeing the issue? The issue would be that you’re trying to refer to a member of an optional type and you thus probably need to use optional chaining by inserting ? before the ..

[saehejkang]

let base = visit(node.base!)?.rawValue on line 60 of Exprs.swift, is one of the examples.

Is using optional chaining even appropriate in this case? The visit function that I created expects a type of Syntax, so would type casting it be more appropriate?

let base = visit(Syntax(node.base!)).rawValue

Also going the way of optional chaining, causes issues line 63 Value of optional type 'UnsafeMutableRawPointer?' must be unwrapped to a value of type 'UnsafeMutableRawPointer

Value of optional type 'UnsafeMutableRawPointer?' must be unwrapped to a value of type 'UnsafeMutableRawPointer'

We can not pass a variable that could be optional.