[PATCH] D63872: [clangd] Fix a case where we fail to detect a header-declared symbol in rename.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:157
   assert(RenameDecl && "symbol must be found at this point");
+  RenameDecl = cast<NamedDecl>(RenameDecl->getCanonicalDecl());
   if (auto Reject = renamableWithinFile(*RenameDecl, File, Index)) {
----------------
sammccall wrote:
> This cast isn't safe. I can't remember the exact case, but if you really need a NamedDecl you need to decide what to do if the canonical one isn't.
Thanks, it is a little surprising this is not safe (I don't have a corner case in mind either).

The `shouldCollectSymbol` requires a NamedDecl, I think when the CanonicalDecl is not a NamedDecl, we think this symbol is not indexable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63872