[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

Manuel Klimek via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 24 00:54:43 PST 2017


On Thu, Feb 23, 2017 at 10:40 PM Sam McCall <sammccall at google.com> wrote:

>
>
> On Feb 23, 2017 8:48 PM, "Haojian Wu via Phabricator" <
> reviews at reviews.llvm.org> wrote:
>
> hokein added inline comments.
>
>
> ================
> Comment at:
> unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp:40
> +  void reportSymbols(llvm::StringRef FileName,
> +                     SymbolInfo::SignalMap NewSymbols) override {
> +    for (const auto &Entry : NewSymbols)
> ----------------
> A new catch: `NewSymbols` should be passed by reference, otherwise a copy
> will be generated.
>
> I did actually intend by-value here, FindAllSymbols no longer needs the
> map once it's reported, so it is moved rather than copied (see the end of
> FindAllSymbols.cpp)
>
> If this is too surprising, I can change it.
>

I'd say that that couples the call site and the function too much.


>
>
>
>
>
> https://reviews.llvm.org/D30210
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170224/877e49dc/attachment.html>


More information about the cfe-commits mailing list