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:
> > `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.


================
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;
----------------
maybe move comment to be above if statement, and clang-format


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

Reply via email to