hokein added inline comments.
================ Comment at: clang-rename/USRLocFinder.cpp:195 +// Find all locations identified by the given USRs. Traverse the AST and find +// every AST node whose USR is in the given USRs' set. +class RenameLocFinder ---------------- ioeric wrote: > I think this also does some renaming? No, this class is only responsible for finding rename locations and other information which are used for renaming. The renaming stuff is done by `USRSymbolRenamer`. ================ Comment at: clang-rename/USRLocFinder.cpp:217 + + // FIXME: For renaming declarations/definitions, prefix qualifiers should be + // filtered out. ---------------- ioeric wrote: > Could you be more specific in this FIXME? I don't quite get it. Maybe an > example? You can also see the `FIXME` in the test. ================ Comment at: clang-rename/USRLocFinder.cpp:359 + + // Returns a list of using declarations which are needed to update. + const std::vector<const UsingDecl *> &getUsingDecls() const { ---------------- ioeric wrote: > I think these are using shadows only? These are interested `UsingDecl`s which contain `UsingShadowDecl` of the symbol declarations being renamed. ================ Comment at: clang-rename/USRLocFinder.h:36 +/// \return Replacement for renaming. +std::vector<tooling::Replacement> +createRenameReplacement(llvm::ArrayRef<std::string> USRs, ---------------- ioeric wrote: > Why use `std::vector` instead of `tooling::Replacements`? Seems that we don't get many benefits from using `tooling::Replacements` here. This function could be called multiple times (for renaming multiple symbols), we need to merge/add all replacements in caller side. if using `tooling::Replacements`, we will merge twice (one is in the API implementation). https://reviews.llvm.org/D31176 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits