[SyntaxTree] Extend the syntax tree dump

[eduucaldas]

Summary

Functional changes in the dump:

• Surround Leaf tokens with '
• Append Node dumps with NodeRole information, except for unknown roles

Non-functional changes:

• ::dumpTokens(llvm::raw_ostream, ArrayRef<syntax::Token>, const SourceManager &SM) always received as parameter a syntax::Token * pointing to Leaf::token(). Changed the function to dumpLeaf(llvm::raw_ostream, syntax::Leaf *, const SourceManager&)
• dumpTree acted on a Node, rename to dumpNode
• use SourceManager in parameters for dump instead of Arena

TODO:
Adapt all the tree dumps in TreeTest.cpp to this new format.

[eduucaldas]

Rollback strOfLeaf

[gribozavr2]

clang/lib/Tooling/Syntax/Tree.cpp

149–150

Maybe the marks should be moved after the primary identifier of the node, WDYT? That would be more consistent with AST dump: first the node kind, then all its info.

[eduucaldas]

Answer comment.

Please suggest more descriptive markers.
Options I thought of:
I -> unmodifiable
M -> not backed by source code / synthesized

[eduucaldas]

Adapt Statement tests to new testing infrastructure

[eduucaldas]

clang/lib/Tooling/Syntax/Tree.cpp

149–150

I agree

clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

107–110 ↗

(On Diff #286983)

I just noticed that we didn't yet use annotations on statement tests.
I think that might help a bit, should I do a quick patch for adding those?

[gribozavr2]

Output format LGTM!

clang/lib/Tooling/Syntax/Tree.cpp

172–174

175

207–210

clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

107–110 ↗

(On Diff #286983)

Yes, that's a good idea!

[eduucaldas]

Update tests to dump role

[eduucaldas]

• Nits

[eduucaldas]

reorganize and nits

[eduucaldas]

.

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

public:
 
   const Tree *parent() const { return Parent; }
   Tree *parent() { return Parent; }
 
   const Node *nextSibling() const { return NextSibling; }
   Node *nextSibling() { return NextSibling; }
 
   /// Dumps the structure of a subtree. For debugging and testing purposes.
-  std::string dump(const Arena &A) const;
+  std::string dump(const SourceManager &SM) const;
   /// Dumps the tokens forming this subtree.
-  std::string dumpTokens(const Arena &A) const;
+  std::string dumpTokens(const SourceManager &SM) const;
 
   /// Asserts invariants on this node of the tree and its immediate children.
   /// Will not recurse into the subtree. No-op if NDEBUG is set.
   void assertInvariants() const;
   /// Runs checkInvariants on all nodes in the subtree. No-op if NDEBUG is set.
   void assertInvariantsRecursive() const;
 
 private:

clang/lib/Tooling/Syntax/BuildTree.cpp

std::string str(const syntax::Arena &A) const {
         unsigned CoveredTokens =
             It != Trees.end()
                 ? (std::next(It)->first - It->first)
                 : A.tokenBuffer().expandedTokens().end() - It->first;
 
         R += std::string(llvm::formatv(
             "- '{0}' covers '{1}'+{2} tokens\n", It->second->kind(),
             It->first->text(A.sourceManager()), CoveredTokens));
-        R += It->second->dump(A);
+        R += It->second->dump(A.sourceManager());
       }
       return R;
     }
 
   private:
     /// Maps from the start token to a subtree starting at that token.
     /// Keys in the map are pointers into the array of expanded tokens, so
     /// pointer order corresponds to the order of preprocessor tokens.

clang/lib/Tooling/Syntax/Tree.cpp

#endif
   }
 
   // Mark the node as modified.
   for (auto *T = this; T && T->Original; T = T->Parent)
     T->Original = false;
 }
 
 namespace {
-static void dumpTokens(llvm::raw_ostream &OS, ArrayRef<syntax::Token> Tokens,
-                       const SourceManager &SM) {
-  assert(!Tokens.empty());
-  bool First = true;
-  for (const auto &T : Tokens) {
-    if (!First)
-      OS << " ";
-    else
-      First = false;
-    // Handle 'eof' separately, calling text() on it produces an empty string.
-    if (T.kind() == tok::eof) {
-      OS << "<eof>";
-      continue;
-    }
-    OS << T.text(SM);
-  }
+static void dumpLeaf(llvm::raw_ostream &OS, const syntax::Leaf *L,
+  assert(L);
+  const auto *Token = L->token();
+  assert(Token);
+  if (Token->kind() == tok::eof)
+  else
+    OS << Token->text(SM);
 }
 
-static void dumpTree(llvm::raw_ostream &OS, const syntax::Node *N,
-                     const syntax::Arena &A, std::vector<bool> IndentMask) {
-  std::string Marks;
+static void dumpNode(llvm::raw_ostream &OS, const syntax::Node *N,
+                     const SourceManager &SM, std::vector<bool> IndentMask) {
+  auto dumpExtraInfo = [&OS](const syntax::Node *N) {
+    if (N->role() != syntax::NodeRole::Unknown)
+      OS << " " << N->role();
     if (!N->isOriginal())
-    Marks += "M";
-  if (N->role() == syntax::NodeRole::Detached)
-    Marks += "*"; // FIXME: find a nice way to print other roles.
+      OS << " M";
     if (!N->canModify())
-    Marks += "I";
-  if (!Marks.empty())
-    OS << Marks << ": ";
+      OS << " I";
+  };
 
+  assert(N);
   if (auto *L = llvm::dyn_cast<syntax::Leaf>(N)) {
-    dumpTokens(OS, *L->token(), A.sourceManager());
+    OS << "'";
+    dumpLeaf(OS, L, SM);
+    OS << "'";
+    dumpExtraInfo(N);
     OS << "\n";
     return;
   }
 
   auto *T = llvm::cast<syntax::Tree>(N);
-  OS << T->kind() << "\n";
+  OS << T->kind();
+  dumpExtraInfo(N);
+  OS << "\n";
 
-  for (auto It = T->firstChild(); It != nullptr; It = It->nextSibling()) {
+  for (const auto *It = T->firstChild(); It; It = It->nextSibling()) {
     for (bool Filled : IndentMask) {
       if (Filled)
         OS << "| ";
       else
         OS << "  ";
     }
     if (!It->nextSibling()) {
       OS << "`-";
       IndentMask.push_back(false);
     } else {
       OS << "|-";
       IndentMask.push_back(true);
     }
-    dumpTree(OS, It, A, IndentMask);
+    dumpNode(OS, It, SM, IndentMask);
     IndentMask.pop_back();
   }
 }
 } // namespace
 
-std::string syntax::Node::dump(const Arena &A) const {
+std::string syntax::Node::dump(const SourceManager &SM) const {
   std::string Str;
   llvm::raw_string_ostream OS(Str);
-  dumpTree(OS, this, A, /*IndentMask=*/{});
+  dumpNode(OS, this, SM, /*IndentMask=*/{});
   return std::move(OS.str());
 }
 
-std::string syntax::Node::dumpTokens(const Arena &A) const {
+std::string syntax::Node::dumpTokens(const SourceManager &SM) const {
   std::string Storage;
   llvm::raw_string_ostream OS(Storage);
   traverse(this, [&](const syntax::Node *N) {
-    auto *L = llvm::dyn_cast<syntax::Leaf>(N);
-    if (!L)
-      return;
-    ::dumpTokens(OS, *L->token(), A.sourceManager());
+    if (auto *L = llvm::dyn_cast<syntax::Leaf>(N)) {
+      dumpLeaf(OS, L, SM);
       OS << " ";
+    }
   });
   return OS.str();
 }
 
 void syntax::Node::assertInvariants() const {
 #ifndef NDEBUG
   if (isDetached())
     assert(parent() == nullptr);

clang/unittests/Tooling/Syntax/TreeTest.cpp

::testing::AssertionResult treeDumpEqual(StringRef Code, StringRef Tree) {
     SCOPED_TRACE(llvm::join(GetParam().getCommandLineArgs(), " "));
 
     auto *Root = buildTree(Code, GetParam());
     if (Diags->getClient()->getNumErrors() != 0) {
       return ::testing::AssertionFailure()
              << "Source file has syntax errors, they were printed to the test "
                 "log";
     }
-    std::string Actual = std::string(StringRef(Root->dump(*Arena)).trim());
+    std::string Actual =
+        std::string(StringRef(Root->dump(Arena->sourceManager())).trim());
     // EXPECT_EQ shows the diff between the two strings if they are different.
     EXPECT_EQ(Tree.trim().str(), Actual);
     if (Actual != Tree.trim().str()) {
       return ::testing::AssertionFailure();
     }
     return ::testing::AssertionSuccess();
   }