[PATCH] D91930: [clangd] Implement textDocument/codeLens

[WangWei via Phabricator via cfe-commits]

lightmelodies added a comment.

I have also considered these problems, and here is some points of my view. 
**Latency**
Consider llvm-project itself, **textDocument/codeLens** can be done in <500ms generally and <100ms if collect hierarchy is disabled in my notebook. Latency is significant in some rare case like //Type.h// which has too complex hierarchy relations. Meanwhile, vscode is smart to remember previous codelens's positions so the text rendering will not look too weird during long-time request. In fact, the biggest performance issue I find is that building AST when open a  new file costs too much time, which makes **textDocument/codeLens** quite slow, as well as other action such as  **textDocument/documentSymbols**. That's why I modified //ClangdLSPServer.cpp// to keep AST for recent closed files.

| file        | line count | cost    | cost (without hierarchy) |
| ASTVector.h | 411        | 4ms     | 1ms                      |
| APValue.h   | 693        | 16 ms   | 8ms                      |
| Decl.h      | 4599       | 213 ms  | 40 ms                    |
| Expr.h      | 6383       | 447 ms  | 61 ms                    |
| Type.h      | 7222       | 3765 ms | 47 ms                    |
|

**Batch**
That's definitely a problem with remote index since vscode can only do resolution per symbol. Maybe it will be better to resolve directly in  **textDocument/codeLens** requests when deal with remote index. Just need add an option to allow users switch the strategy.

**Useless Lens**
Yes, we can see the base directly in class declaration. However c++ has forward declaration and type alias, in such case the base can not be seen directly. So someone may prefer to have these codelens. Typescript in vscode has options to allow users disable specific type of codelens. We can use similar strategy, e.g. **-codelens=references,base,derived** instead of **-codelens=true**.

I'm currently working on an implemention by traversing AST directly instead of using **textDocument/documentSymbols**, since symbols defined by macro are often reported a wrong range. It should save much time of **findNamedDeclAt** comparing to current implemention, and make it easier to add additional filters on symbols.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91930