arphaman marked an inline comment as done.
arphaman added inline comments.
================
Comment at: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp:398
Visitor.TraverseDecl(Decl);
- return Visitor.getLocationsFound();
+ return std::move(Visitor.getOccurrences());
}
----------------
hokein wrote:
> arphaman wrote:
> > hokein wrote:
> > > I think just returning `Visitor.getOccurrences()` is sufficient --
> > > compiler can handle it well, also using std::move would prevent copy
> > > elision.
> > I'm returning by non-const reference from `getOccurrences`, so the compiler
> > copies by default. I have to move either here or return by value from
> > `getOccurrences` and move there. We also have warnings for redundant
> > `std::move`, and they don't fire here.
> Ok, didn't notice that `getOccurance()` returns non-const reference.
>
> I think the method `getOccurances` may have potential effect -- the clients
> could change the Occurences member variable of USRLocFindingASTVisitor, after
> getting the non-const reference.
>
> Another option is to rename `getOccurances` to `takeOccurrences` and return
> by value:
>
> ```
> SymbolOccurrences takeOccurrences() { return std::move(Occurrences); }
> ```
>
> What do you think?
Yeah, `takeOccurrences` would be better I think. I'll update.
Repository:
rL LLVM
https://reviews.llvm.org/D36156
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
