eduucaldas added inline comments.
================ Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:94 /// a binary expression'. Used for implementing accessors. +// How to name NodeRole: +// If the child node is a token/keyword, end its name with 'Token'/'Keyword' ---------------- gribozavr2 wrote: > I'd suggest to make this into a doc comment, but phrase it in a way that is > useful for users, so that they can understand the pattern too. For example: > > Some roles describe parent/child relations that occur multiple times in > language grammar. We define only one role to describe all instances of such > recurring relations. For example, grammar for both "if" and "while" > statements requires an opening paren and a closing paren. The opening paren > token is assigned the `OpenParen` role regardless of whether it appears as a > child of `IfStatement` or `WhileStatement` node. More generally, when grammar > requires a certain fixed token (like a specific keyword, or an opening > paren), we define a role for this token and use it across all grammar rules > with the same requirement. Names of such reusable roles end with a `~Token` > or a `~Keyword` suffix. > > Some roles are assigned only to child nodes of one specific parent syntax > node type. Names of such roles start with the name of the parent syntax tree > node type. For example, a syntax node with a role > `BinaryOperatorExpression_leftHandSide` can only appear as a child of a > `BinaryOperatorExpression` node. > > Thank you a lot. This is really well explained ================ Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:124 IfStatement_thenStatement, IfStatement_elseKeyword, IfStatement_elseStatement, ---------------- gribozavr2 wrote: > Shouldn't `elseKeyword` have no prefix? When a keyword can only be used by IfStatement, then I think it actually helps readability to have it prepended with the ParentKind. Here everything is nicely grouped, and if someone needs to change IfStatement, it is clear to see where to make the change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81157/new/ https://reviews.llvm.org/D81157 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
