Fix issue 9904: False positive: duplicateCondition when modifying variable in lambda (#2811)

This commit is contained in:
Paul Fultz II 2020-09-18 00:44:26 -05:00 committed by GitHub
parent e5d0ffdbe7
commit c2e8051196
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 69 additions and 1 deletions

View File

@ -29,9 +29,11 @@
#include "valueflow.h"
#include <algorithm>
#include <functional>
#include <iterator>
#include <list>
#include <stack>
#include <utility>
template<class T, REQUIRES("T must be a Token class", std::is_convertible<T*, const Token*>)>
void visitAstNodesGeneric(T *ast, std::function<ChildrenToVisit(T *)> visitor)
@ -1605,17 +1607,66 @@ bool isVariableChanged(const Token *start, const Token *end, int indirect, const
return findVariableChanged(start, end, indirect, exprid, globalvar, settings, cpp, depth) != nullptr;
}
static const Token* findExpression(const Token* start, const nonneg int exprid)
{
Function * f = Scope::nestedInFunction(start->scope());
if (!f)
return nullptr;
const Scope* scope = f->functionScope;
if (!scope)
return nullptr;
for (const Token *tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) {
if (tok->exprId() != exprid)
continue;
return tok;
}
return nullptr;
}
// Thread-unsafe memoization
template<class F, class R=decltype(std::declval<F>()())>
static std::function<R()> memoize(F f)
{
bool init = false;
R result{};
return [=]() mutable -> R {
if (init)
return result;
result = f();
init = true;
return result;
};
}
Token* findVariableChanged(Token *start, const Token *end, int indirect, const nonneg int exprid, bool globalvar, const Settings *settings, bool cpp, int depth)
{
if (!precedes(start, end))
return nullptr;
if (depth < 0)
return start;
auto getExprTok = memoize([&]{ return findExpression(start, exprid); });
for (Token *tok = start; tok != end; tok = tok->next()) {
if (tok->exprId() != exprid) {
if (globalvar && Token::Match(tok, "%name% ("))
// TODO: Is global variable really changed by function call?
return tok;
// Is aliased function call
if (Token::Match(tok, "%var% (") && std::any_of(tok->values().begin(), tok->values().end(), std::mem_fn(&ValueFlow::Value::isLifetimeValue))) {
bool aliased = false;
// If we cant find the expression then assume it was modified
if (!getExprTok())
return tok;
visitAstNodes(getExprTok(), [&](const Token* childTok) {
if (childTok->varId() > 0 && isAliasOf(tok, childTok->varId())) {
aliased = true;
return ChildrenToVisit::done;
}
return ChildrenToVisit::op1_and_op2;
});
// TODO: Try to traverse the lambda function
if (aliased)
return tok;
}
continue;
}
if (isVariableChanged(tok, indirect, settings, cpp, depth))

View File

@ -3585,7 +3585,7 @@ static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLog
LifetimeStore{argtok, "Passed to '" + tok->str() + "'.", ValueFlow::Value::LifetimeKind::Object} .byRef(
tok->next(), tokenlist, errorLogger, settings);
}
} else if (Token::Match(tok->tokAt(-2), "std :: make_tuple|tuple_cat|make_pair|make_reverse_iterator|next|prev|move")) {
} else if (Token::Match(tok->tokAt(-2), "std :: make_tuple|tuple_cat|make_pair|make_reverse_iterator|next|prev|move|bind")) {
for (const Token *argtok : getArguments(tok)) {
LifetimeStore{argtok, "Passed to '" + tok->str() + "'.", ValueFlow::Value::LifetimeKind::Object} .byVal(
tok->next(), tokenlist, errorLogger, settings);

View File

@ -3800,6 +3800,23 @@ private:
"}\n");
ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:9]: (style) The if condition is the same as the previous if condition\n",
errout.str());
check("void f(bool a, bool b) {\n"
" auto g = [&] { b = !a; };\n"
" if (b)\n"
" g();\n"
" if (b) {}\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void g(bool& a);\n"
"void f(bool b) {\n"
" auto h = std::bind(&g, std::ref(b));\n"
" if (b)\n"
" h();\n"
" if (b) {}\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
void checkInvalidTestForOverflow() {