kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:466
auto DeclsUnderCursor = locateDeclAt(AST, IdentifierToken->location());
if (DeclsUnderCursor.empty())
----------------
hokein wrote:
> kadircet wrote:
> > hokein wrote:
> > > kadircet wrote:
> > > > `locateDeclAt` is already working on `NamedDecl`s but returning a set
> > > > of `Decl`s could you rather update that helper to return a set of
> > > > `NamedDecl`s instead?
> > > I think the main problem is that `NamedDecl->getCanonicalDecl()` returns
> > > a `Decl*`, which we need to do a `dyn_cast`.
> > ah right, but still it should be safe to perform just an `llvm:cast` here,
> > as a `NamedDecl` shouldn't have an `unnamed` decl as its canonical
> > declaration.
> > a NamedDecl shouldn't have an unnamed decl as its canonical declaration.
> yeah, this is true for most cases, but it is not safe, we have corn cases,
> see the comment in SymbolCollector about ObjCPropertyDecl.
I don't think that's relevant in here though, it is performing the cast on a
`Decl`. It is the `ASTNode.OrigD` that has been populated by `libIndex`.
So it doesn't need to be related with whatever nameddecl is being processed.
Am I missing something?
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:148
+ RenameDecl, RenameDecl.getASTContext(), SymbolCollector::Options(),
+ IsMainFileOnly)) // If the symbol is not indexable, we disallow rename.
return ReasonToReject::NonIndexable;
----------------
kadircet wrote:
> maybe move comment to be above if statement, and clang-format
looks like it is still not clang-formatted
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73450/new/
https://reviews.llvm.org/D73450
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
