hokein added inline comments.
================
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:
> hokein wrote:
> > ioeric wrote:
> > > I think these are using shadows only?
> > These are interested `UsingDecl`s which contain `UsingShadowDecl` of the
> > symbol declarations being renamed.
> Sure. But maybe also document it?
I have documented it at the "UsingDecls" member definition.
================
Comment at: clang-rename/USRLocFinder.h:36
+/// \return Replacement for renaming.
+std::vector<tooling::Replacement>
+createRenameReplacement(llvm::ArrayRef<std::string> USRs,
----------------
ioeric wrote:
> hokein wrote:
> > 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).
> I think what we really want is `AtomicChange`. We shouldn't be using
> `std::vector` or `std::set` replacements anymore.
Good idea. Done. I think we might still need to refine the interface in the
future.
https://reviews.llvm.org/D31176
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
