[PATCH] D72638: [clangd] Fix rename for explicit destructor calls

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 15 05:15:43 PST 2020


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





More information about the cfe-commits mailing list