On Thu, Feb 23, 2017 at 10:40 PM Sam McCall <sammcc...@google.com> wrote:
>
>
> On Feb 23, 2017 8:48 PM, "Haojian Wu via Phabricator" <
> revi...@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
>
>
>
>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
