Generate grammar doc comments for layout nodes #1771

ahoppen · 2023-06-12T23:17:57Z

Generate a Grammar and a Children section as doc-comments for layout nodes.

The Grammar section is particularly useful for fairly simple syntax nodes to convey their structure. The Children section allows you to see the children of the syntax node at a glance in the source order.

ahoppen · 2023-06-12T23:18:02Z

@swift-ci Please test

bnbarham · 2023-06-16T23:25:41Z

Cool stuff, interesting idea!

I'm not convinced it's worth having both grammar + children though. In general children seems more useful to me. Do you have any examples where you'd consider the "grammar" as better than children? Right now they're basically duplicates, with children having the advantage of having the name as well.

WowbaggersLiquidLunch · 2023-06-18T23:39:16Z

CodeGeneration/Sources/SyntaxSupport/Node.swift

+        ### Children
+
+        \(GrammarGenerator.childrenList(for: children))


The list of children should already be visible through DocC, under the type/protocol's Instance Properties section. Does it really need to be repeated in the main description too?

I think it does because these are sorted in source order while the Instance Properties are sorted alphabetically.

OK I think this is a valid reason. Although, maybe in the long term we should have the ability to specify to DocC the ordering of properties.

Yes, if we could do that, I agree that we might be able to remove the children list.

WowbaggersLiquidLunch · 2023-06-18T23:42:06Z

Sources/SwiftSyntax/generated/syntaxNodes/SyntaxDeclNodes.swift

+/// (``AttributeSyntax`` | ``IfConfigDeclSyntax``) `*`
+/// ``DeclModifierSyntax``?
+/// (`'get'` | `'set'` | `'didSet'` | `'willSet'` | `'unsafeAddress'` | `'addressWithOwner'` | `'addressWithNativeOwner'` | `'unsafeMutableAddress'` | `'mutableAddressWithOwner'` | `'mutableAddressWithNativeOwner'` | `'_read'` | `'_modify'` | `'init'`)
+/// ``AccessorParameterSyntax``?
+/// ``AccessorEffectSpecifiersSyntax``?
+/// ``AccessorInitEffectsSyntax``?
+/// ``CodeBlockSyntax``?


Is there a way to generate the translation between the type/protocol names and the actual grammar terms/components that they represent? I think having such a translation table can help a lot to contributors.

What do you mean by “actual grammar terms/components that they represent”?

I might be misunderstanding it. What I mean is: it seems to me that not all syntax nodes in the parser correlate one-to-one with the syntax nodes as documented in TSPL.

No, there isn’t always a 1:1 correlation between the SwiftSyntax nodes and the grammar rules in TSPL.

WowbaggersLiquidLunch · 2023-06-18T23:42:59Z

CodeGeneration/Sources/SyntaxSupport/GrammarGenerator.swift

+  private func grammar(for tokenChoice: TokenChoice) -> String {
+    switch tokenChoice {
+    case .keyword(text: let text):
+      return "`'\(text)'`"


I feel having quotation marks within the code voice makes it a bit ambiguous what the token actually is.

My idea here was to optimize this for rendering in docc and I found that rendering the token name in code voice reads a little nicer in docc. Would you prefer to just use

return "'\(text)'"

Yes, I think I would prefer just return "'\(text)'" or return "`\(text)`". And if the latter is used, syntactic categories can be italicized like in TSPL, to differentiate from token texts.

The thing is that we use docc links for links to other syntactic categories, which are always monospaced.

The documentation for StructDeclSyntax shows all kinds of references. Which fonts would you use here?

WowbaggersLiquidLunch · 2023-06-18T23:44:00Z

CodeGeneration/Sources/SyntaxSupport/GrammarGenerator.swift

+      .joined(separator: "\n")
+  }
+
+  /// Generates a


Incomplete doc comment. Should be "/// Generates a list of child nodes"?

Good catch. Thank you

WowbaggersLiquidLunch

Thanks for this. I think this is a good approach in general. I have just some nitpicky-ish comments.

ahoppen · 2023-06-19T15:36:26Z

I'm not convinced it's worth having both grammar + children though. In general children seems more useful to me. Do you have any examples where you'd consider the "grammar" as better than children? Right now they're basically duplicates, with children having the advantage of having the name as well.

My idea was that in particular for simple nodes, the grammar shows the structure of the node in a very nice and concise form. But looking at other nodes as well, I agree that it very much breaks down for more complex nodes. @WowbaggersLiquidLunch Do you have opinions on just showing the Children section?

ahoppen · 2023-06-20T21:01:41Z

OK, I removed the Grammar section and we’re just left with Children now. That seems to be cleaner.

ahoppen · 2023-06-20T21:01:46Z

@swift-ci Please test

ahoppen · 2023-06-20T21:02:16Z

@swift-ci Please test Windows

ahoppen · 2023-06-22T20:44:56Z

@swift-ci Please test

bnbarham · 2023-06-15T16:49:25Z

CodeGeneration/Sources/SyntaxSupport/GrammarGenerator.swift

+      .joined(separator: "\n")
+  }
+
+  /// Generates a


Got half way through :)?

Generate a `Grammar` and a `Children` section as doc-comments for layout nodes. The `Grammar` section is particularly useful for fairly simple syntax nodes to convey their structure. The `Children` section allows you to see the children of the syntax node at a glance in the source order.

ahoppen · 2023-06-23T13:49:38Z

@swift-ci Please test

ahoppen · 2023-06-23T13:50:55Z

@swift-ci Please test Windows

WowbaggersLiquidLunch · 2023-06-23T16:05:52Z

My idea was that in particular for simple nodes, the grammar shows the structure of the node in a very nice and concise form. But looking at other nodes as well, I agree that it very much breaks down for more complex nodes. @WowbaggersLiquidLunch Do you have opinions on just showing the Children section?

OK, I removed the Grammar section and we’re just left with Children now. That seems to be cleaner.

Sorry for the late response. I still think it's better to have the Grammar section. It's probably a bit more clutter, but from my experience, having the grammar documented really helps new contributors find the relevant things to modify. Although, instead of "'await' ExprSyntax", I would prefer "await-operator" in the documentation, because that's probably what people will search for to find this syntax node.

Additionally something I just noticed: Currently the grammars are documented on the parse functions, not syntax node types. Are there plans to generated the grammar docs for those functions?

ahoppen · 2023-06-26T10:58:28Z

Sorry for the late response. I still think it's better to have the Grammar section. It's probably a bit more clutter, but from my experience, having the grammar documented really helps new contributors find the relevant things to modify.

Which information do you think would be valuable in the Grammar section that’s not present in the Children section?

Although, instead of "'await' ExprSyntax", I would prefer "await-operator" in the documentation, because that's probably what people will search for to find this syntax node.

To be clear: There is a difference between SwiftSyntax and TSPL, just by the way the trees are structured.

For example, if we wanted to model the following production rules in SwiftSyntax, prefix-operator would need to be an enum with two cases, each with associated values and that’s just not how the SwiftSyntax tree is structured.

prefix-expression → prefix-operator? postfix-expression
prefix-expression → in-out-expression

So, there is not equivalent of the

expression → try-operator? await-operator? prefix-expression infix-expressions?

production rule or await-operator in SwiftSyntax, so I don’t think there’s a natural way to place it.

Additionally something I just noticed: Currently the grammars are documented on the parse functions, not syntax node types. Are there plans to generated the grammar docs for those functions?

Since the grammar rules on the parse functions are mostly outdated, I want to remove them now that this PR is merged.

ahoppen requested review from rintaro, DougGregor and bnbarham June 12, 2023 23:17

ahoppen mentioned this pull request Jun 15, 2023

Remove Grammar doc comments #1420

Merged

WowbaggersLiquidLunch reviewed Jun 18, 2023

View reviewed changes

ahoppen force-pushed the ahoppen/generate-grammar branch from 1e8152f to dc058cd Compare June 19, 2023 15:31

ahoppen force-pushed the ahoppen/generate-grammar branch from dc058cd to 1669a5f Compare June 20, 2023 21:01

ahoppen force-pushed the ahoppen/generate-grammar branch from 1669a5f to d868b0d Compare June 22, 2023 20:44

bnbarham approved these changes Jun 22, 2023

View reviewed changes

ahoppen force-pushed the ahoppen/generate-grammar branch from d868b0d to 9b128ec Compare June 23, 2023 13:49

ahoppen enabled auto-merge June 23, 2023 13:49

ahoppen mentioned this pull request Jun 23, 2023

[5.9] Generate grammar doc comments for layout nodes #1836

Merged

ahoppen merged commit 0fbe758 into swiftlang:main Jun 23, 2023

ahoppen deleted the ahoppen/generate-grammar branch June 26, 2023 10:51

ahoppen mentioned this pull request Jun 27, 2023

Generate doc-comment for syntax node initializers #1849

Merged

Generate grammar doc comments for layout nodes #1771

Generate grammar doc comments for layout nodes #1771

Conversation

ahoppen commented Jun 12, 2023

Uh oh!

ahoppen commented Jun 12, 2023

Uh oh!

bnbarham commented Jun 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WowbaggersLiquidLunch Jun 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WowbaggersLiquidLunch left a comment

Choose a reason for hiding this comment

Uh oh!

ahoppen commented Jun 19, 2023

Uh oh!

ahoppen commented Jun 20, 2023

Uh oh!

ahoppen commented Jun 20, 2023

Uh oh!

ahoppen commented Jun 20, 2023

Uh oh!

ahoppen commented Jun 22, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahoppen commented Jun 23, 2023

Uh oh!

ahoppen commented Jun 23, 2023

Uh oh!

WowbaggersLiquidLunch commented Jun 23, 2023

Uh oh!

ahoppen commented Jun 26, 2023

Uh oh!

Uh oh!

bnbarham commented Jun 16, 2023 •

edited

Loading

WowbaggersLiquidLunch Jun 18, 2023 •

edited

Loading