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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
