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

[Marcel Hlopko via Phabricator via cfe-commits]

hlopko added a comment.

Taking over the patch in https://reviews.llvm.org/D76355.



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


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


================
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;
----------------
gribozavr2 wrote:
> I don't understand the first fixme (maybe it is stale).
> 
> The second fixme is not applicable after this patch.
> 
Removed both FIXMEs now, asking Ilya offline for clarification.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72446