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