[PATCH] D61637: [Syntax] Introduce syntax trees

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Nice, let's land this!



================
Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:35
+/// A root node for a translation unit. Parent is always null.
+class TranslationUnitDeclaration final : public Tree {
+public:
----------------
I don't think TU is actually a declaration. Is there a reason to consider it one from a syntax POV?


================
Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:41
+  }
+  syntax::Leaf *eof();
+
----------------
I have a slight feeling the EOF token is going to be annoying, e.g. can't just splice stuff in at the end of the list. But not sure if it'll be a big deal, and whether the alternatives are better.


================
Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:43
+
+  struct Roles {
+    static constexpr NodeRole eof = 1;
----------------
we discussed offline - with 256 values, is it possible to come up with a single role enum that would cover all node types?

Advantage would be that certain logic could be generic (e.g. `Recovery` could be a role for leaves under any Tree, `LParen`/`RParen`/`MainKeyword` could apply to if, while, switch...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61637