[PATCH] D82739: Improve heuristic resolution of dependent types in TargetFinder

Tue Jul 7 23:57:44 PDT 2020

nridge marked an inline comment as done.
nridge added a comment.

In D82739#2133155 <https://reviews.llvm.org/D82739#2133155>, @hokein wrote:

> looks like we use this heuristic for go-to-def, I think we might want to use it in more places, e.g.  libindex (for xrefs), sema code completion (so that `this->a.^` can suggest something).

Yeah, this is a great point. I would definitely like to reuse this heuristic resolution for other features like code completion (including in https://github.com/clangd/clangd/issues/443 where I hope to build on it to improve the original STR which involves completion).

I will explore relocating this code to a place where things like code completion can reuse it, in a follow-up patch.

================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:198
+    // analyzing it.
+    if (E && BT->getKind() == BuiltinType::Dependent) {
+      T = resolveDependentExprToType(E);
----------------
hokein wrote:
> I think this is the key point of the fix?
> 
> It would be nice if you could find a way to split it into two (one for refactoring, one for the fix). The diff of this patch contains large chunk of refactoring changes which make the fix less obvious.
Yeah, sorry about that. I've split the patch and cleaned it up further (to avoid unnecessary reordering of functions) to make the diff neater.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82739