[Syntax] Move roles into a separate enum

To align with reviewer's suggestions.

llvm-svn: 365479

Author: ilya-biryukov

clang/include/clang/Tooling/Syntax/Nodes.h

@@ -31,6 +31,18 @@ enum class NodeKind : uint16_t {
 /// For debugging purposes.
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, NodeKind K);
 
+/// A relation between a parent and child node. Used for implementing accessors.
+enum class NodeRole : uint8_t {
+  // A node without a parent.
+  Detached,
+  // Children of an unknown semantic nature, e.g. skipped tokens, comments.
+  Unknown,
+  // FIXME: should this be shared for all other nodes with braces, e.g. init
+  //        lists?
+  CompoundStatement_lbrace,
+  CompoundStatement_rbrace
+};
+
 /// A root node for a translation unit. Parent is always null.
 class TranslationUnit final : public Tree {
 public:
@@ -73,11 +85,6 @@ class CompoundStatement final : public Statement {
   }
   syntax::Leaf *lbrace();
   syntax::Leaf *rbrace();
-
-  struct Roles {
-    static constexpr NodeRole lbrace = 1;
-    static constexpr NodeRole rbrace = 2;
-  };
 };
 
 } // namespace syntax

clang/include/clang/Tooling/Syntax/Tree.h

@@ -66,30 +66,18 @@ class Arena {
 class Tree;
 class TreeBuilder;
 enum class NodeKind : uint16_t;
-
-/// Represents a relation of this node to its parent, e.g. 'lbrace inside a
-/// compound statement'.
-///
-/// Each node type defines a set of roles for its children.
-using NodeRole = uint8_t;
-
-/// Role for detached nodes, i.e. the ones that do not have parent nodes.
-constexpr NodeRole NodeRoleDetached = 0;
-/// Role for children of unknown semantic nature, e.g. skipped tokens, comments.
-constexpr NodeRole NodeRoleUnknown = 255;
+enum class NodeRole : uint8_t;
 
 /// A node in a syntax tree. Each node is either a Leaf (representing tokens) or
 /// a Tree (representing language constructrs).
 class Node {
 public:
   /// Newly created nodes are detached from a tree, parent and sibling links are
   /// set when the node is added as a child to another one.
-  Node(NodeKind Kind)
-      : Parent(nullptr), NextSibling(nullptr),
-        Kind(static_cast<unsigned>(Kind)), Role(NodeRoleDetached) {}
+  Node(NodeKind Kind);
 
   NodeKind kind() const { return static_cast<NodeKind>(Kind); }
-  NodeRole role() const { return Role; }
+  NodeRole role() const { return static_cast<NodeRole>(Role); }
 
   const Tree *parent() const { return Parent; }
   Tree *parent() { return Parent; }

clang/lib/Tooling/Syntax/BuildTree.cpp

@@ -170,7 +170,7 @@ class syntax::TreeBuilder {
     /// A with a role that should be assigned to it when adding to a parent.
     struct NodeAndRole {
       explicit NodeAndRole(syntax::Node *Node)
-          : Node(Node), Role(NodeRoleUnknown) {}
+          : Node(Node), Role(NodeRole::Unknown) {}
 
       syntax::Node *Node;
       NodeRole Role;
@@ -221,10 +221,12 @@ class BuildTreeVisitor : public RecursiveASTVisitor<BuildTreeVisitor> {
   }
 
   bool WalkUpFromCompoundStmt(CompoundStmt *S) {
-    using Roles = syntax::CompoundStatement::Roles;
+    using NodeRole = syntax::NodeRole;
 
-    Builder.markChildToken(S->getLBracLoc(), tok::l_brace, Roles::lbrace);
-    Builder.markChildToken(S->getRBracLoc(), tok::r_brace, Roles::rbrace);
+    Builder.markChildToken(S->getLBracLoc(), tok::l_brace,
+                           NodeRole::CompoundStatement_lbrace);
+    Builder.markChildToken(S->getRBracLoc(), tok::r_brace,
+                           NodeRole::CompoundStatement_rbrace);
 
     Builder.foldNode(Builder.getRange(S),
                      new (allocator()) syntax::CompoundStatement);

clang/lib/Tooling/Syntax/Nodes.cpp

@@ -25,9 +25,11 @@ llvm::raw_ostream &syntax::operator<<(llvm::raw_ostream &OS, NodeKind K) {
 }
 
 syntax::Leaf *syntax::CompoundStatement::lbrace() {
-  return llvm::cast_or_null<syntax::Leaf>(findChild(Roles::lbrace));
+  return llvm::cast_or_null<syntax::Leaf>(
+      findChild(NodeRole::CompoundStatement_lbrace));
 }
 
 syntax::Leaf *syntax::CompoundStatement::rbrace() {
-  return llvm::cast_or_null<syntax::Leaf>(findChild(Roles::rbrace));
+  return llvm::cast_or_null<syntax::Leaf>(
+      findChild(NodeRole::CompoundStatement_rbrace));
 }

clang/lib/Tooling/Syntax/Tree.cpp

@@ -38,17 +38,21 @@ bool syntax::Leaf::classof(const Node *N) {
   return N->kind() == NodeKind::Leaf;
 }
 
+syntax::Node::Node(NodeKind Kind)
+    : Parent(nullptr), NextSibling(nullptr), Kind(static_cast<unsigned>(Kind)),
+      Role(static_cast<unsigned>(NodeRole::Detached)) {}
+
 bool syntax::Tree::classof(const Node *N) { return N->kind() > NodeKind::Leaf; }
 
 void syntax::Tree::prependChildLowLevel(Node *Child, NodeRole Role) {
   assert(Child->Parent == nullptr);
   assert(Child->NextSibling == nullptr);
-  assert(Child->Role == NodeRoleDetached);
-  assert(Role != NodeRoleDetached);
+  assert(Child->role() == NodeRole::Detached);
+  assert(Role != NodeRole::Detached);
 
   Child->Parent = this;
   Child->NextSibling = this->FirstChild;
-  Child->Role = Role;
+  Child->Role = static_cast<unsigned>(Role);
   this->FirstChild = Child;
 }
 
@@ -81,9 +85,9 @@ static void dumpTokens(llvm::raw_ostream &OS, ArrayRef<syntax::Token> Tokens,
 
 static void dumpTree(llvm::raw_ostream &OS, const syntax::Node *N,
                      const syntax::Arena &A, std::vector<bool> IndentMask) {
-  if (N->role() != syntax::NodeRoleUnknown) {
+  if (N->role() != syntax::NodeRole::Unknown) {
     // FIXME: print the symbolic name of a role.
-    if (N->role() == syntax::NodeRoleDetached)
+    if (N->role() == syntax::NodeRole::Detached)
       OS << "*: ";
     else
       OS << static_cast<int>(N->role()) << ": ";
@@ -138,7 +142,7 @@ std::string syntax::Node::dumpTokens(const Arena &A) const {
 
 syntax::Node *syntax::Tree::findChild(NodeRole R) {
   for (auto *C = FirstChild; C; C = C->nextSibling()) {
-    if (C->Role == R)
+    if (C->role() == R)
       return C;
   }
   return nullptr;

clang/unittests/Tooling/Syntax/TreeTest.cpp

@@ -136,16 +136,16 @@ void foo() {}
 | |-(
 | |-)
 | `-CompoundStatement
-|   |-1: {
-|   `-2: }
+|   |-2: {
+|   `-3: }
 |-TopLevelDeclaration
 | |-void
 | |-foo
 | |-(
 | |-)
 | `-CompoundStatement
-|   |-1: {
-|   `-2: }
+|   |-2: {
+|   `-3: }
 `-<eof>
 )txt"},
   };