[PATCH] D90543: [Syntax] Start to move trivial Node class definitions to TableGen. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 3 inline comments as done.
sammccall added a comment.

In D90543#2373962 <https://reviews.llvm.org/D90543#2373962>, @eduucaldas wrote:

>> Compared to Dmitri's prototype, Nodes.td looks more like a class hierarchy and
>> less like a grammar. (E.g. variants list the Alternatives parent rather than
>> vice versa).
>
>
>
>> e.g. we may introduce abstract bases like "loop" that the grammar doesn't care about in order to model is-a concepts that might make refactorings more expressive. This is less natural in a grammar-like idiom.
>
> Do you have a concrete example of such abstract base -- loop is in the grammar <https://eel.is/c++draft/stmt.iter#nt:iteration-statement> ? And in the case such an example appear, in my mind we would just change "our grammar" to have an alternative for this "loop" abstract base.

Not really, I think you're right. This is a bad argument.

>> e.g. we're likely to have to model some alternatives as variants and others as class hierarchies, the choice will probably be based on natural is-a relationships.
>
> I agree, alternatives and the two ways to model them are a tricky subject...
>
>> it reduces the cognitive load of switching from editing *.td to working with code that uses the generated classes
>
> I think we should consider reading prior to editing.

Agree. I think the same load exists for reading though.

> A grammar is how we represent syntax, and syntax trees model syntax, as such I think we should rather consider the cognitive load of switching between the grammar and the definition of syntax trees.

I don't think people reason about concrete grammar as a collection of productions though, rather as a collection of kinds of nodes and what they're composed of.
And we model it that way in the C++ API! I think the tablegen should follow that too...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90543/new/

https://reviews.llvm.org/D90543