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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to