Move TokenSyntax to SyntaxNodes.swift.gyb

[ahoppen]

All the other syntax nodes live in SyntaxNodes.swift.gyb, we should not make an exception for TokenSyntax.

[ahoppen]

@swift-ci Please test

[allevato]

From a discoverability point of view, might it not be better if types that are entirely non-generated stay in separate files instead of being lumped in with the generated content? (Unless they need fileprivate access for some reason, I suppose.)

Especially considering how large the SyntaxNodes.swift file already is...

[ahoppen]

It’s only affecting TokenSyntax and UnknownSyntax. UnknownSyntax has always lived in SyntaxNodes.swift.gyb and I’m not really sure if there’s a benefit in making a file for those two types.

[harlanhaskins]

I dunno, I think I'd prefer going the other direction, moving UnknownSyntax to its own file. I don't see a benefit for moving TokenSyntax...

[ahoppen]

The issue came up because I was looking through code in the generated SyntaxNodes.swift, wanted to jump to TokenSyntax and couldn’t find its definition there.

I think it makes sense to have all node declarations in one file if you think that we are shipping the generated files as a SwiftPM dependency. Whoever depends on SwiftSyntax should not have to worry about why some nodes were generated and others weren’t. I think it’s conceptually easier if you can tell someone that all syntax nodes are declared in SyntaxNodes.swift.

[akyrtzi]

I'm leaning with with @allevato and @harlanhaskins, I prefer that we only put in gyb files what needs to be generated, and move to normal Swift files what does not need gyb to be written out.

[allevato]

Yeah, I probably would have moved UnknownSyntax to its own file too under those circumstances.

IMO, it feels a little odd to have large units of code that doesn't change via generation in a generator file when the language doesn't require that organization. In my own experience, it's helpful to keep the generated surface as small as possible, because at a minimum, if you're editing/viewing a .swift.gyb file in your favorite editor/diff viewer/online browser, you potentially lose whatever UI affordances or additional features you would have for .swift files (syntax highlighting, semantic navigation, etc.). Similarly, if UnknownSyntax or TokenSyntax changes and I look through the repository history, it's easier to see that if they're in their own files vs. if the changes are just considered part of a larger template file.

I think it makes sense to have all node declarations in one file if you think that we are shipping the generated files as a SwiftPM dependency. Whoever depends on SwiftSyntax should not have to worry about why some nodes were generated and others weren’t. I think it’s conceptually easier if you can tell someone that all syntax nodes are declared in SyntaxNodes.swift.

The times that I've had to jump into SyntaxNodes.swift (the generated one) to look something up on a particular node type, its size sometimes gets in the way. So what you say above makes sense and leads to a question I was going to ask for a different reason:

The gyb_syntax_support files in apple/swift are nicely split up based on the "type" of node (Decls, Exprs, Stmts, etc.), and I think that makes it a lot easier to find what I'm looking for and also keeps those files from becoming too unmanageable. I don't know if it's feasible with the way the scripts work, but could we generate separate files (DeclNodes.swift, ExprNodes.swift, StmtNodes.swift), or is that breakdown lost by the time the Python code has loaded everything into memory?

[ahoppen]

I don’t have too strong feelings about this as well. I just felt like the status quo is definitely not optimal.

The gyb_syntax_support files in apple/swift are nicely split up based on the "type" of node (Decls, Exprs, Stmts, etc.), and I think that makes it a lot easier to find what I'm looking for and also keeps those files from becoming too unmanageable. I don't know if it's feasible with the way the scripts work, but could we generate separate files (DeclNodes.swift, ExprNodes.swift, StmtNodes.swift), or is that breakdown lost by the time the Python code has loaded everything into memory?

That’s an interesting thought. We may be able to drive gyb in a way that it only generates the code for a subset of the nodes and then invoke it several times for the different node types. I’ll look into it.

[ahoppen]

Thanks for your feedback. I am now pursuing splitting SyntaxNodes.swift.gyb into multiple files here #165.

Closing this PR.