kadircet added inline comments.

================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:620
+      if (llvm::dyn_cast<CXXDestructorDecl>(E->getMemberDecl()))
+        NameLocation = E->getEndLoc();
       Refs.push_back(ReferenceLoc{E->getQualifierLoc(),
----------------
instead of patching the source location for destructors. we should probably not 
report anything for them in here, as they will be reported correctly as 
typelocs.

So you can check for `E->getMemberNameInfo().getNamedTypoInfo()` and bail out 
if its non-null, which means this is a constructor/destructor/operator with a 
typeloc that will be visited separately and result in the same ref. That way we 
would also get rid of duplicate refs being reported by 
`findExplicitReferences`, API doesn't mention anything about those but most of 
the callers would fail in the presence of duplicates, therefore I think we 
shouldn't be reporting duplicates. (for example the main visitor actually makes 
sure typelocs are not visited twice).

also could you add unittests for constructor and operator cases as well to make 
sure we get exactly one reference for those as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72638



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

Reply via email to