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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits