[PATCH] D66751: [clangd] Add targetDecl(), which determines what declaration an AST node refers to.

Thu Aug 29 02:38:26 PDT 2019

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.

In D66751#1646991 <https://reviews.llvm.org/D66751#1646991>, @sammccall wrote:

> Yes. I actually ripped this out of the patch, because it wasn't related enough and the patch is large.

Makes sense and thanks for summing it up. Just wanted to make sure we are thinking about this use-case.

My comment was specifically referring to the lack of a "flat" model in our codebase.
I believe hierarchical model is very well handled by the selection tree (there might be bugs, obviously, but conceptually it's doing the right thing).

>> E.g. finding a source location of field for DeclRefExpr produced for MyBase::field seems to be the same amount of work (in general case) as finding the Decl* of field.
> 
> For the hierarchical model it's easy, this is one of the few methods DynTypeNode actually has :-)
>  For the flat model: it's not trivial, but is less work because while you do have to dispatch over all node types, you don't have general multi-hop/graph cases (I think).
>  Usually the "handle" tokens are well-supported by the AST because they're precisely the ones that diagnostics want to report. So in the DeclRefExpr case, DeclRefExpr::getLoc() returns the right thing.

To be clear, the concern is **not** that it's hard to find the corresponding locations for each particular AST node.
The concern is that it's hard to cover **all** possible nodes and we should do it once rather than repeat this multiple times.

`DeclRefExpr` was a bad example for that purpose. E.g. one could make the same claim about `targetDecl` function we are introducing: it's very easy to get a target decl of `DeclRefExpr`,
but it's hard to enumerate all nodes that can have target decls and ensure they are handled properly.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66751