Fix FP useStlAlgorithm (don't suggest std::accumulate when nothing is accumulated)#4647
Conversation
| else if (accumulateBoolLiteral(assignTok, assignVarId)) | ||
| algo = "std::any_of, std::all_of, std::none_of, or std::accumulate"; | ||
| else | ||
| else if (assignTok->str() != "=") |
There was a problem hiding this comment.
This should be moved up to the if above, so it is if (accumulateBoolLiteral(assignTok, assignVarId) || assignTok->str() == "=")
| " ret = 1;\n" | ||
| " return ret;\n" | ||
| "}\n"); | ||
| ASSERT_EQUALS("", errout.str()); |
There was a problem hiding this comment.
This should suggest std::any_of, std::all_of, or std::none_of.
There was a problem hiding this comment.
I don't think so, since those algorithms short-circuit. Presumably g() needs to be called for all elements.
There was a problem hiding this comment.
Then it should probably be std::any_of, std::all_of, std::none_of, or std::accumulate. Then you can use std::accumulate when g() has side effects.
There was a problem hiding this comment.
We could use isConstExpression to detect if the condition has side effects and then only suggest when there is no side effects.
| " int ret = 0;\n" | ||
| " for (const auto i : v)\n" | ||
| " if (!g(i))\n" | ||
| " ret = 1;\n" |
There was a problem hiding this comment.
will this still warn when doing ret = ret + 1 or ret = h(ret)? We should probably check that the variable appears on the RHS.
There was a problem hiding this comment.
We suggest std::count_if for ret = ret + 1. There is no warning for ret = h(ret) with or without this change.
No description provided.