[PATCH] D65936: [clangd] Use raw rename functions to implement the rename.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 8 04:00:29 PDT 2019


sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:141
+  for (Decl *TopLevelDecl : AST.getLocalTopLevelDecls()) {
+    auto RenameInDecl =
+        tooling::getOccurrencesOfUSRs(RenameUSRs, OldName, TopLevelDecl);
----------------
nit: most of the new uses of `auto` in this patch don't have a type that's obvious from the RHS, nor one that's hard to read, and should probably be expanded


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:175
+    // Currently, we only support normal rename (one range) for C/C++.
+    // FIXME: support multiple-range rename for objective-c methods.
+    if (Rename.getNameRanges().size() > 1)
----------------
is this a regression in this patch, or did the limitation already exist?


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:178
+      continue;
+    cantFail(FilteredChanges.add(tooling::Replacement(
+        AST.getASTContext().getSourceManager(),
----------------
why can't this fail?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65936





More information about the cfe-commits mailing list