[PATCH] D72446: [Syntax] Build mapping from AST to syntax tree nodes

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 accepted this revision.
gribozavr2 added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:147
+  void add(ASTPtr From, syntax::Tree *To) {
+    assert(To != nullptr);
+
----------------
Also assert that From is not null either?


================
Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:160
+  // Keys are either Stmt* or Decl*.
+  llvm::DenseMap<void *, syntax::Tree *> Nodes;
+};
----------------
I think it is possible to use a PointerUnion as a DenseMap key (there's a DenseMapInfo for it).


================
Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:491
     /// FIXME: storing the end tokens is redundant.
     /// FIXME: the key of a map is redundant, it is also stored in NodeForRange.
+    std::map<const syntax::Token *, syntax::Node *> Trees;
----------------
I don't understand the first fixme (maybe it is stale).

The second fixme is not applicable after this patch.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72446