[PATCH] D132797: [clangd] Support renaming virtual methods

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

A few more NITs from me, but please wait for @sammccall for another round of reviews.



================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:550
+// method.
+void recursivelyInsertOverrides(SymbolID Base, llvm::DenseSet<SymbolID> &IDs,
+                                const SymbolIndex &Index) {
----------------
NIT: maybe rename to `insertTransitiveOverrides`?
This function is not recursive anymore and "transitive" seems to be a more precise term for describing the results of running this function.


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:561-562
+    Index.relations(Req, [&](const SymbolID &, const Symbol &Override) {
+      IDs.insert(Override.ID);
+      Pending.insert(Override.ID);
+    });
----------------
This should protect against infinite loop in case of broken/stale index.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132797/new/

https://reviews.llvm.org/D132797