sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
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)) {
----------------
hokein wrote:
> 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.
Sounds good.
See the comment in SymbolCollector about `ObjCPropertyDecl`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63872/new/
https://reviews.llvm.org/D63872
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
