[SwiftSyntax] Make Syntax nodes structs instead of classes

[harlanhaskins]

This patch, more in line with how I hoped we could structure the API, moves from a class-based model with Syntax as a superclass to a protocol-based model where each struct conforms to Syntax.

This has benefits and drawbacks:

Benefits

• Syntax nodes, which are just thin wrappers around SyntaxData, no longer need to be ARC managed.
• This moves it from a 4-word ARC box to a 2-word struct
• Creating a Syntax node turns from a heap allocation into a retain on the root

(I noticed allocations of these things was a problem when I profiled a run of Silt's parser and noticed a huuuuge number of Syntax node allocations)

Drawbacks

• Syntax/ExprSyntax/DeclSyntax, etc. nodes can no longer be created directly.
• Syntax can no longer be Hashable because that precludes its use as an existential. Instead of comparing two Syntax nodes with an Equatable conformance, one must conditionally cast them down to the same type. There is an overload for ==, so most users won't feel the pain of this. But it does mean you can no longer make a Set<Syntax>, only Sets of concrete Syntax nodes.

I'm leaning towards the drawbacks outweighing the benefits, but I personally think it's unreasonable for each of these nodes to have to be an allocation.

[harlanhaskins]

@swift-ci please smoke test

[rintaro]

+1 I believe this is the right way to go!

[nkcsgexi]

This is an interesting experiment, @harlanhaskins . The second drawback is the one I'm not a big fan of. We do see our clients using Set/Dictionary to track Syntax nodes in multi-pass analysis (this patch may break their codebase). Can we reconcile the hashable convenience and the awesome memory efficiency?

[harlanhaskins]

Well, hope is not completely lost. You can still have a set of, say, FunctionDeclSyntax, but a set of ExprSyntax or Syntax wouldn't work. I could add a .uniqueIdentifier property that you could use in a Set.

[harlanhaskins]

@swift-ci please smoke test

[harlanhaskins]

Another thing: I think Any<Kind>Syntax is actually much worse, as it really only fits in with a C++-style cast<>()-oriented Syntax tree, not one with structs and type metadata. Any<Kind>Syntax was really only necessary to appease the generic SyntaxCollection, because we couldn't say SyntaxCollection<ExprSyntax> if ExprSyntax is a protocol. Instead, the approach I've settled on is using gyb to generate all Syntax collections instead of generic typealiases. It'll lead to more generated code, but will expose a model where things like DeclListSyntax are a list of anything conforming to DeclSyntax.

[harlanhaskins]

@nkcsgexi So, this seems to be the best I can get the branch. All the existing CollectionSyntax nodes work the same way, and their Element type is still the upper bound of whatever type they wrap. Unfortunately, we lose the ability to directly compare Syntax nodes, requiring you to check
if they both have the same type. There is still an overload for ==, which helps when users are writing code, but doesn't allow e.g., [ExprSyntax] to conform to Equatable.

Also, we lose the ability to use Syntax and ExprSyntax nodes in Sets, directly, without making it a Set of some concrete Syntax type.

[harlanhaskins]

@swift-ci please smoke test

[harlanhaskins]

To measure the performance impact of this change, I adjusted Silt's parser to use struct-based Syntax nodes and had it parse a 100,000-line file. Our AST, prior to this change, allocated 400MB, then kept allocating more memory as we dumped the AST until we reached 2.2GB while printing the 20,000th line. I killed the program after there because memory was climbing linearly.

After making this change, we got through all 100,000 lines allocating 1.75GB, which (though excessive) is a significant improvement.

[nkcsgexi]

@harlanhaskins I think you're right. Judging from the experiment results, this is a great performance improvement that worths the dropping of hashability. Feel free to merge if when you feel ready. Thanks! 👍

[harlanhaskins]

@swift-ci please smoke test

[harlanhaskins]

Thanks for reviewing!