hokein added a comment.
Thanks for the comments.
================
Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:81
+ hasLoopInit(LoopVarInit),
+ hasCondition(binaryOperator(hasOperatorName("<"),
+ hasLHS(RefersToLoopVar),
----------------
aaron.ballman wrote:
> Perhaps you could support other comparisons, but not attempt to generate a
> fix-it for them? It seems odd that this would trigger for `<` but not `<=`,
> but I can see why you'd not want to figure out the reserve call for `!(foo >=
> 10)`.
We could support other comparisons in the future. I think it is a good start to
just support `<` in the first version as `<` usage happens more frequently than
other comparisons.
Added a fixme.
================
Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:83
+ hasLHS(RefersToLoopVar),
+ hasRHS(expr().bind(LoopEndExprName)))),
+ hasIncrement(unaryOperator(hasOperatorName("++"),
----------------
aaron.ballman wrote:
> Thinking out loud, but, what happens if the loop end expression has some
> hidden complexity to it? e.g.,
> ```
> for (int i = 0; i < i + 1; ++i) { // This is a "creative" loop.
> v.push_back(i);
> }
> ```
Good point. It will copy the expr "i+1" to "reserve" parameter. This kind of
case should not be existed, but it is safer to filter out this case.
================
Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:95-96
+ const MatchFinder::MatchResult &Result) {
+ if (Result.Context->getDiagnostics().hasUncompilableErrorOccurred())
+ return;
+
----------------
aaron.ballman wrote:
> We don't usually add this test in to our check calls; why are you adding it
> here?
It will prevent some unexpected things happened when the translation unit fails
to compile.
We had a few experiences before. We encountered some misbehavior of some checks
(e.g. https://reviews.llvm.org/rL294578) with a non-compilable TU, and then we
added this statement to avoid the misbehavior.
https://reviews.llvm.org/D31757
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
