hokein added inline comments.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:466
auto DeclsUnderCursor = locateDeclAt(AST, IdentifierToken->location());
if (DeclsUnderCursor.empty())
----------------
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`.
================
Comment at: clang-tools-extra/clangd/refactor/RenameBlacklist.cpp:14
+
+bool isRenameBlacklisted(const NamedDecl &RenameDecl) {
+ static const auto *StdSymbols = new llvm::DenseSet<llvm::StringRef>({
----------------
kadircet wrote:
> I don't think there's much value in having that in its own file, could you
> move it into the rename.cpp ?
I thought we will extend this in the near future, moved to rename.cpp now as it
is pretty short.
================
Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:453
rename({RenamePos, NewName, AST, testPath(TU.Filename)});
- ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
+ ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError() << T;
ASSERT_EQ(1u, RenameResult->size());
----------------
kadircet wrote:
> maybe `SCOPEDT_TRACE(T)` instead?
good point, reverted the change in this patch, and will do this in a separate
patch.
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
