hokein added a comment.
Thanks for the comments!
================
Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:129
+ cxxForRangeStmt(
+ hasRangeInit(declRefExpr(supportedContainerTypesMatcher())),
+ HasInterestedLoopBody, InInterestedCompoundStmt)
----------------
alexfh wrote:
> It might be valuable to support more complex expressions here, though it
> would require more involved fixes, e.g.
>
> for (const auto& e : *source)
> v.push_back(e);
>
> ->
>
> v.reserve(source->size());
> for (const auto& e : *source)
> v.push_back(e);
>
> or even like this:
>
> for (const auto& e : *some(complex)->container().expression())
> v.push_back(e);
>
> ->
>
> const auto& source_container = *some(complex)->container().expression();
> v.reserve(source_container.size());
> for (const auto& e : source_container)
> v.push_back(e);
>
> Maybe leave a FIXME for this?
Nice brainstorm, added a FIXME.
================
Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:152
+
+ const Stmt * LoopStmt = nullptr;
+ if (ForLoop)
----------------
alexfh wrote:
> const Stmt *LoopStmt = ForLoop ? ForLoop : RangeLoop;
We can't do it because `ForLoop` and `RangeLoop` are different types.
================
Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:198
+ Context->getLangOpts());
+ ReserveStmt = (VectorVarName + ".reserve(" + LoopEndSource + ");\n").str();
+ }
----------------
alexfh wrote:
> Does the check handle destination vector not being empty before the push_back
> loop? We'd need to reserve `destination.size() + source.size()` items in this
> case.
>
> If this is not handled yet, please add a FIXME.
This case should not happen, because we only find the push_back loop before
which there are no usages (defined as DeclRefExpr that refers to vector`v`) of
the vector `v`.
https://reviews.llvm.org/D32436
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
