[PATCH] D84781: [SyntaxTree] Use PointerUnion instead of inheritance for alternative clauses in NNS

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas added inline comments.


================
Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:788-789
-
-    // Remove "::" from the `SourceRange`
-    SR.setEnd(SR.getEnd().getLocWithOffset(-1));
 
----------------
Newbie mistake causing the crash


================
Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:815
+      Builder.markChild(new (allocator()) syntax::EmptyNode,
+                        syntax::NodeRole::Unknown);
+      return new (allocator()) syntax::NameSpecifier;
----------------
I mark `NodeRole` as `Unknown` here, this would need fixing in the future. But should we make roles that  mimic the `NodeKind` of the alternative?


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1004-1009
+    | | | | | `-SimpleTemplateSpecifier
+    | | | | |   |-template
+    | | | | |   |-ST
+    | | | | |   |-<
+    | | | | |   |-int
+    | | | | |   `->
----------------
Here notice the additional level of nesting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84781