alexfh added a comment.
Cool! A few nits.
================
Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:56
+ast_matchers::internal::Matcher<Expr> supportedContainerTypesMatcher() {
+ const auto types = cxxRecordDecl(hasAnyName(
+ "::std::vector", "::std::set", "::std::unordered_set", "::std::map",
----------------
I'd just inline the expression and remove the local variable, since it doesn't
seem to bring any benefits.
================
Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:103
+ // Matchers for the loop whose body has only 1 push_back calling statement.
+ const auto HasInterestedLoopBody = hasBody(anyOf(
+ compoundStmt(statementCountIs(1), has(PushBackCall)), PushBackCall));
----------------
s/Interested/Interesting/
================
Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:129
+ cxxForRangeStmt(
+ hasRangeInit(declRefExpr(supportedContainerTypesMatcher())),
+ HasInterestedLoopBody, InInterestedCompoundStmt)
----------------
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?
================
Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:152
+
+ const Stmt * LoopStmt = nullptr;
+ if (ForLoop)
----------------
const Stmt *LoopStmt = ForLoop ? ForLoop : RangeLoop;
================
Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:185
+ // `for (range-declarator: range-expression)`.
+ llvm::StringRef RangeInitExpName = clang::Lexer::getSourceText(
+ CharSourceRange::getTokenRange(
----------------
No need to qualify `StringRef` and `Lexer`. Here and a couple of other
instances in this function.
================
Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:198
+ Context->getLangOpts());
+ ReserveStmt = (VectorVarName + ".reserve(" + LoopEndSource + ");\n").str();
+ }
----------------
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.
https://reviews.llvm.org/D32436
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
