[clang] 42f6fec - Propose naming principle for NodeRole and apply it

Dmitri Gribenko via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 4 11:10:51 PDT 2020


Author: Eduardo Caldas
Date: 2020-06-04T20:08:35+02:00
New Revision: 42f6fec3878d708f2791ab0be3a060b07dac9d76

URL: https://github.com/llvm/llvm-project/commit/42f6fec3878d708f2791ab0be3a060b07dac9d76
DIFF: https://github.com/llvm/llvm-project/commit/42f6fec3878d708f2791ab0be3a060b07dac9d76.diff

LOG: Propose naming principle for NodeRole and apply it

Reviewers: gribozavr2

Reviewed By: gribozavr2

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D81157

Added: 
    

Modified: 
    clang/include/clang/Tooling/Syntax/Nodes.h
    clang/lib/Tooling/Syntax/BuildTree.cpp
    clang/lib/Tooling/Syntax/Nodes.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Tooling/Syntax/Nodes.h b/clang/include/clang/Tooling/Syntax/Nodes.h
index 7d0c4439e279..7e05aa968f02 100644
--- a/clang/include/clang/Tooling/Syntax/Nodes.h
+++ b/clang/include/clang/Tooling/Syntax/Nodes.h
@@ -91,6 +91,23 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, NodeKind K);
 
 /// A relation between a parent and child node, e.g. 'left-hand-side of
 /// a binary expression'. Used for implementing accessors.
+///
+/// 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.
 enum class NodeRole : uint8_t {
   // Roles common to multiple node kinds.
   /// A node without a parent
@@ -103,19 +120,21 @@ enum class NodeRole : uint8_t {
   CloseParen,
   /// A keywords that introduces some grammar construct, e.g. 'if', 'try', etc.
   IntroducerKeyword,
+  /// A token that represents a literal, e.g. 'nullptr', '1', 'true', etc.
+  LiteralToken,
+  /// Tokens or Keywords
+  ArrowToken,
+  ExternKeyword,
   /// An inner statement for those that have only a single child of kind
   /// statement, e.g. loop body for while, for, etc; inner statement for case,
   /// default, etc.
   BodyStatement,
 
   // Roles specific to particular node kinds.
-  UnaryOperatorExpression_operatorToken,
+  OperatorExpression_operatorToken,
   UnaryOperatorExpression_operand,
   BinaryOperatorExpression_leftHandSide,
-  BinaryOperatorExpression_operatorToken,
   BinaryOperatorExpression_rightHandSide,
-  CxxNullPtrExpression_keyword,
-  IntegerLiteralExpression_literalToken,
   CaseStatement_value,
   IfStatement_thenStatement,
   IfStatement_elseKeyword,
@@ -127,10 +146,8 @@ enum class NodeRole : uint8_t {
   StaticAssertDeclaration_message,
   SimpleDeclaration_declarator,
   TemplateDeclaration_declaration,
-  ExplicitTemplateInstantiation_externKeyword,
   ExplicitTemplateInstantiation_declaration,
   ArraySubscript_sizeExpression,
-  TrailingReturnType_arrow,
   TrailingReturnType_declarator,
   ParametersAndQualifiers_parameter,
   ParametersAndQualifiers_trailingReturn

diff  --git a/clang/lib/Tooling/Syntax/BuildTree.cpp b/clang/lib/Tooling/Syntax/BuildTree.cpp
index 1f81cb4a12b0..9ce12ac21f8b 100644
--- a/clang/lib/Tooling/Syntax/BuildTree.cpp
+++ b/clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -609,26 +609,22 @@ class BuildTreeVisitor : public RecursiveASTVisitor<BuildTreeVisitor> {
   }
 
   bool WalkUpFromIntegerLiteral(IntegerLiteral *S) {
-    Builder.markChildToken(
-        S->getLocation(),
-        syntax::NodeRole::IntegerLiteralExpression_literalToken);
+    Builder.markChildToken(S->getLocation(), syntax::NodeRole::LiteralToken);
     Builder.foldNode(Builder.getExprRange(S),
                      new (allocator()) syntax::IntegerLiteralExpression, S);
     return true;
   }
 
   bool WalkUpFromCXXNullPtrLiteralExpr(CXXNullPtrLiteralExpr *S) {
-    Builder.markChildToken(S->getLocation(),
-                           syntax::NodeRole::CxxNullPtrExpression_keyword);
+    Builder.markChildToken(S->getLocation(), syntax::NodeRole::LiteralToken);
     Builder.foldNode(Builder.getExprRange(S),
                      new (allocator()) syntax::CxxNullPtrExpression, S);
     return true;
   }
 
   bool WalkUpFromUnaryOperator(UnaryOperator *S) {
-    Builder.markChildToken(
-        S->getOperatorLoc(),
-        syntax::NodeRole::UnaryOperatorExpression_operatorToken);
+    Builder.markChildToken(S->getOperatorLoc(),
+                           syntax::NodeRole::OperatorExpression_operatorToken);
     Builder.markExprChild(S->getSubExpr(),
                           syntax::NodeRole::UnaryOperatorExpression_operand);
 
@@ -647,9 +643,8 @@ class BuildTreeVisitor : public RecursiveASTVisitor<BuildTreeVisitor> {
   bool WalkUpFromBinaryOperator(BinaryOperator *S) {
     Builder.markExprChild(
         S->getLHS(), syntax::NodeRole::BinaryOperatorExpression_leftHandSide);
-    Builder.markChildToken(
-        S->getOperatorLoc(),
-        syntax::NodeRole::BinaryOperatorExpression_operatorToken);
+    Builder.markChildToken(S->getOperatorLoc(),
+                           syntax::NodeRole::OperatorExpression_operatorToken);
     Builder.markExprChild(
         S->getRHS(), syntax::NodeRole::BinaryOperatorExpression_rightHandSide);
     Builder.foldNode(Builder.getExprRange(S),
@@ -664,7 +659,7 @@ class BuildTreeVisitor : public RecursiveASTVisitor<BuildTreeVisitor> {
           syntax::NodeRole::BinaryOperatorExpression_leftHandSide);
       Builder.markChildToken(
           S->getOperatorLoc(),
-          syntax::NodeRole::BinaryOperatorExpression_operatorToken);
+          syntax::NodeRole::OperatorExpression_operatorToken);
       Builder.markExprChild(
           S->getArg(1),
           syntax::NodeRole::BinaryOperatorExpression_rightHandSide);
@@ -984,7 +979,7 @@ class BuildTreeVisitor : public RecursiveASTVisitor<BuildTreeVisitor> {
     const auto *Arrow = Return.begin() - 1;
     assert(Arrow->kind() == tok::arrow);
     auto Tokens = llvm::makeArrayRef(Arrow, Return.end());
-    Builder.markChildToken(Arrow, syntax::NodeRole::TrailingReturnType_arrow);
+    Builder.markChildToken(Arrow, syntax::NodeRole::ArrowToken);
     if (ReturnDeclarator)
       Builder.markChild(ReturnDeclarator,
                         syntax::NodeRole::TrailingReturnType_declarator);
@@ -999,9 +994,7 @@ class BuildTreeVisitor : public RecursiveASTVisitor<BuildTreeVisitor> {
       syntax::SimpleDeclaration *InnerDeclaration, Decl *From) {
     assert(!ExternKW || ExternKW->kind() == tok::kw_extern);
     assert(TemplateKW && TemplateKW->kind() == tok::kw_template);
-    Builder.markChildToken(
-        ExternKW,
-        syntax::NodeRole::ExplicitTemplateInstantiation_externKeyword);
+    Builder.markChildToken(ExternKW, syntax::NodeRole::ExternKeyword);
     Builder.markChildToken(TemplateKW, syntax::NodeRole::IntroducerKeyword);
     Builder.markChild(
         InnerDeclaration,

diff  --git a/clang/lib/Tooling/Syntax/Nodes.cpp b/clang/lib/Tooling/Syntax/Nodes.cpp
index bad902b8c6d7..7498ddceeb14 100644
--- a/clang/lib/Tooling/Syntax/Nodes.cpp
+++ b/clang/lib/Tooling/Syntax/Nodes.cpp
@@ -110,6 +110,12 @@ llvm::raw_ostream &syntax::operator<<(llvm::raw_ostream &OS, NodeRole R) {
     return OS << "CloseParen";
   case syntax::NodeRole::IntroducerKeyword:
     return OS << "IntroducerKeyword";
+  case syntax::NodeRole::LiteralToken:
+    return OS << "LiteralToken";
+  case syntax::NodeRole::ArrowToken:
+    return OS << "ArrowToken";
+  case syntax::NodeRole::ExternKeyword:
+    return OS << "ExternKeyword";
   case syntax::NodeRole::BodyStatement:
     return OS << "BodyStatement";
   case syntax::NodeRole::CaseStatement_value:
@@ -120,18 +126,12 @@ llvm::raw_ostream &syntax::operator<<(llvm::raw_ostream &OS, NodeRole R) {
     return OS << "IfStatement_elseKeyword";
   case syntax::NodeRole::IfStatement_elseStatement:
     return OS << "IfStatement_elseStatement";
-  case syntax::NodeRole::IntegerLiteralExpression_literalToken:
-    return OS << "IntegerLiteralExpression_literalToken";
-  case syntax::NodeRole::CxxNullPtrExpression_keyword:
-    return OS << "CxxNullPtrExpression_keyword";
-  case syntax::NodeRole::UnaryOperatorExpression_operatorToken:
-    return OS << "UnaryOperatorExpression_operatorToken";
+  case syntax::NodeRole::OperatorExpression_operatorToken:
+    return OS << "OperatorExpression_operatorToken";
   case syntax::NodeRole::UnaryOperatorExpression_operand:
     return OS << "UnaryOperatorExpression_operand";
   case syntax::NodeRole::BinaryOperatorExpression_leftHandSide:
     return OS << "BinaryOperatorExpression_leftHandSide";
-  case syntax::NodeRole::BinaryOperatorExpression_operatorToken:
-    return OS << "BinaryOperatorExpression_operatorToken";
   case syntax::NodeRole::BinaryOperatorExpression_rightHandSide:
     return OS << "BinaryOperatorExpression_rightHandSide";
   case syntax::NodeRole::ReturnStatement_value:
@@ -148,14 +148,10 @@ llvm::raw_ostream &syntax::operator<<(llvm::raw_ostream &OS, NodeRole R) {
     return OS << "SimpleDeclaration_declarator";
   case syntax::NodeRole::TemplateDeclaration_declaration:
     return OS << "TemplateDeclaration_declaration";
-  case syntax::NodeRole::ExplicitTemplateInstantiation_externKeyword:
-    return OS << "ExplicitTemplateInstantiation_externKeyword";
   case syntax::NodeRole::ExplicitTemplateInstantiation_declaration:
     return OS << "ExplicitTemplateInstantiation_declaration";
   case syntax::NodeRole::ArraySubscript_sizeExpression:
     return OS << "ArraySubscript_sizeExpression";
-  case syntax::NodeRole::TrailingReturnType_arrow:
-    return OS << "TrailingReturnType_arrow";
   case syntax::NodeRole::TrailingReturnType_declarator:
     return OS << "TrailingReturnType_declarator";
   case syntax::NodeRole::ParametersAndQualifiers_parameter:
@@ -168,12 +164,12 @@ llvm::raw_ostream &syntax::operator<<(llvm::raw_ostream &OS, NodeRole R) {
 
 syntax::Leaf *syntax::IntegerLiteralExpression::literalToken() {
   return llvm::cast_or_null<syntax::Leaf>(
-      findChild(syntax::NodeRole::IntegerLiteralExpression_literalToken));
+      findChild(syntax::NodeRole::LiteralToken));
 }
 
 syntax::Leaf *syntax::CxxNullPtrExpression::nullPtrKeyword() {
   return llvm::cast_or_null<syntax::Leaf>(
-      findChild(syntax::NodeRole::CxxNullPtrExpression_keyword));
+      findChild(syntax::NodeRole::LiteralToken));
 }
 
 syntax::Expression *syntax::BinaryOperatorExpression::lhs() {
@@ -183,7 +179,7 @@ syntax::Expression *syntax::BinaryOperatorExpression::lhs() {
 
 syntax::Leaf *syntax::UnaryOperatorExpression::operatorToken() {
   return llvm::cast_or_null<syntax::Leaf>(
-      findChild(syntax::NodeRole::UnaryOperatorExpression_operatorToken));
+      findChild(syntax::NodeRole::OperatorExpression_operatorToken));
 }
 
 syntax::Expression *syntax::UnaryOperatorExpression::operand() {
@@ -193,7 +189,7 @@ syntax::Expression *syntax::UnaryOperatorExpression::operand() {
 
 syntax::Leaf *syntax::BinaryOperatorExpression::operatorToken() {
   return llvm::cast_or_null<syntax::Leaf>(
-      findChild(syntax::NodeRole::BinaryOperatorExpression_operatorToken));
+      findChild(syntax::NodeRole::OperatorExpression_operatorToken));
 }
 
 syntax::Expression *syntax::BinaryOperatorExpression::rhs() {
@@ -367,7 +363,7 @@ syntax::Leaf *syntax::ExplicitTemplateInstantiation::templateKeyword() {
 
 syntax::Leaf *syntax::ExplicitTemplateInstantiation::externKeyword() {
   return llvm::cast_or_null<syntax::Leaf>(
-      findChild(syntax::NodeRole::ExplicitTemplateInstantiation_externKeyword));
+      findChild(syntax::NodeRole::ExternKeyword));
 }
 
 syntax::Declaration *syntax::ExplicitTemplateInstantiation::declaration() {
@@ -402,7 +398,7 @@ syntax::Leaf *syntax::ArraySubscript::rbracket() {
 
 syntax::Leaf *syntax::TrailingReturnType::arrowToken() {
   return llvm::cast_or_null<syntax::Leaf>(
-      findChild(syntax::NodeRole::TrailingReturnType_arrow));
+      findChild(syntax::NodeRole::ArrowToken));
 }
 
 syntax::SimpleDeclarator *syntax::TrailingReturnType::declarator() {


        


More information about the cfe-commits mailing list