[PATCH] D68137: [clangd] Handle template arguments in findExplicitReferences

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked 6 inline comments as done.
ilya-biryukov added inline comments.


================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:556
 
+  // We re-define Traverse*, since there's no corresponding Visit*.
+  bool TraverseTemplateArgumentLoc(TemplateArgumentLoc A) {
----------------
kadircet wrote:
> ... and we need it because, template template decls are visited through it ?
Yes, exactly. Amended this to the comment.


================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:567
+      break;
+    case TemplateArgument::Declaration:
+      break; // FIXME: can this actually happen in TemplateArgumentLoc?
----------------
kadircet wrote:
> I suppose these corresponds to non-type template paramters, don't they? (which should make it similar to integral case)
Yes, e.g. see the `wrapper<func>` example from the tests.
However, these arguments seem to be stored and visited as expressions in the `TemplateArgumentLoc`.

IIUC, `Declaration` can only happen in template arguments outside `TemplateArgumentLoc`.


================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:618
-    if (Ref->NameLoc.isInvalid()) {
-      dlog("invalid location at node {0}", nodeToString(N));
       return;
----------------
kadircet wrote:
> can we keep dlog ?
I've initially removed it to avoid passing `DynTypedNode` to `reportReference`, but I also agree this `dlog` is useful.

So making the function signature a bit clunky for the sake of debug logging


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68137