From d6f6e68fa2b4f342a0e564a623fbed0d338723df Mon Sep 17 00:00:00 2001 From: Ken-Patrick Date: Wed, 3 Jul 2019 08:17:06 +0200 Subject: [PATCH] Fix false positive 9167 (#1904) Skip returns from local class/struct definition in FwdAnalysis. --- lib/astutils.cpp | 31 +++++++++++++++++++++---------- lib/astutils.h | 2 +- test/testunusedvar.cpp | 16 ++++++++++++++++ 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 2b6a6e246..d1008319f 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1146,7 +1146,7 @@ static bool hasFunctionCall(const Token *tok) return hasFunctionCall(tok->astOperand1()) || hasFunctionCall(tok->astOperand2()); } -struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const Token *startToken, const Token *endToken, const std::set &exprVarIds, bool local) +struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const Token *startToken, const Token *endToken, const std::set &exprVarIds, bool local, bool inInnerClass) { // Parse the given tokens for (const Token *tok = startToken; tok != endToken; tok = tok->next()) { @@ -1162,13 +1162,21 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const if (Token::simpleMatch(tok, "goto")) return Result(Result::Type::BAILOUT); + if (!inInnerClass && tok->str() == "{" && tok->scope()->isClassOrStruct()) { + // skip returns from local class definition + FwdAnalysis::Result result = checkRecursive(expr, tok, tok->link(), exprVarIds, local, true); + if (result.type != Result::Type::NONE) + return result; + tok=tok->link(); + } + if (tok->str() == "continue") // TODO return Result(Result::Type::BAILOUT); if (const Token *lambdaEndToken = findLambdaEndToken(tok)) { tok = lambdaEndToken; - const Result lambdaResult = checkRecursive(expr, lambdaEndToken->link()->next(), lambdaEndToken, exprVarIds, local); + const Result lambdaResult = checkRecursive(expr, lambdaEndToken->link()->next(), lambdaEndToken, exprVarIds, local, inInnerClass); if (lambdaResult.type == Result::Type::READ || lambdaResult.type == Result::Type::BAILOUT) return lambdaResult; } @@ -1184,10 +1192,13 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const return Result(Result::Type::READ); } - if (!local && mWhat == What::Reassign) - return Result(Result::Type::BAILOUT); + // #9167: if the return is inside an inner class, it does not tell us anything + if (!inInnerClass) { + if (!local && mWhat == What::Reassign) + return Result(Result::Type::BAILOUT); - return Result(Result::Type::RETURN); + return Result(Result::Type::RETURN); + } } if (tok->str() == "}") { @@ -1218,7 +1229,7 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const } // check loop body again.. - const struct FwdAnalysis::Result &result = checkRecursive(expr, tok->link(), tok, exprVarIds, local); + const struct FwdAnalysis::Result &result = checkRecursive(expr, tok->link(), tok, exprVarIds, local, inInnerClass); if (result.type == Result::Type::BAILOUT || result.type == Result::Type::READ) return result; } @@ -1297,14 +1308,14 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const if (Token::simpleMatch(tok->link()->previous(), "switch (")) // TODO: parse switch return Result(Result::Type::BAILOUT); - const Result &result1 = checkRecursive(expr, tok->tokAt(2), tok->linkAt(1), exprVarIds, local); + const Result &result1 = checkRecursive(expr, tok->tokAt(2), tok->linkAt(1), exprVarIds, local, inInnerClass); if (result1.type == Result::Type::READ || result1.type == Result::Type::BAILOUT) return result1; if (mWhat == What::ValueFlow && result1.type == Result::Type::WRITE) mValueFlowKnown = false; if (Token::simpleMatch(tok->linkAt(1), "} else {")) { const Token *elseStart = tok->linkAt(1)->tokAt(2); - const Result &result2 = checkRecursive(expr, elseStart, elseStart->link(), exprVarIds, local); + const Result &result2 = checkRecursive(expr, elseStart, elseStart->link(), exprVarIds, local, inInnerClass); if (mWhat == What::ValueFlow && result2.type == Result::Type::WRITE) mValueFlowKnown = false; if (result2.type == Result::Type::READ || result2.type == Result::Type::BAILOUT) @@ -1426,7 +1437,7 @@ FwdAnalysis::Result FwdAnalysis::check(const Token *expr, const Token *startToke if (mWhat == What::UnusedValue && isGlobalData(expr)) return Result(FwdAnalysis::Result::Type::BAILOUT); - Result result = checkRecursive(expr, startToken, endToken, exprVarIds, local); + Result result = checkRecursive(expr, startToken, endToken, exprVarIds, local, false); // Break => continue checking in outer scope while (mWhat!=What::ValueFlow && result.type == FwdAnalysis::Result::Type::BREAK) { @@ -1435,7 +1446,7 @@ FwdAnalysis::Result FwdAnalysis::check(const Token *expr, const Token *startToke s = s->nestedIn; if (s->type != Scope::eSwitch && s->type != Scope::eWhile && s->type != Scope::eFor) break; - result = checkRecursive(expr, s->bodyEnd->next(), endToken, exprVarIds, local); + result = checkRecursive(expr, s->bodyEnd->next(), endToken, exprVarIds, local, false); } return result; diff --git a/lib/astutils.h b/lib/astutils.h index 561184442..6c37b0779 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -219,7 +219,7 @@ private: }; struct Result check(const Token *expr, const Token *startToken, const Token *endToken); - struct Result checkRecursive(const Token *expr, const Token *startToken, const Token *endToken, const std::set &exprVarIds, bool local); + struct Result checkRecursive(const Token *expr, const Token *startToken, const Token *endToken, const std::set &exprVarIds, bool local, bool inInnerClass); // Is expression a l-value global data? bool isGlobalData(const Token *expr) const; diff --git a/test/testunusedvar.cpp b/test/testunusedvar.cpp index 42f2f8329..7219745c1 100644 --- a/test/testunusedvar.cpp +++ b/test/testunusedvar.cpp @@ -144,6 +144,7 @@ private: TEST_CASE(localvarstring2); // ticket #2929 TEST_CASE(localvarconst1); TEST_CASE(localvarconst2); + TEST_CASE(localvarreturn); // ticket #9167 TEST_CASE(localvarthrow); // ticket #3687 @@ -4086,6 +4087,21 @@ private: ASSERT_EQUALS("", errout.str()); } + void localvarreturn() { // ticket #9167 + functionVariableUsage("void foo() {\n" + " const int MyInt = 1;\n" + " class bar {\n" + " public:\n" + " bool operator()(const int &uIndexA, const int &uIndexB) const {\n" + " return true;\n" + " }\n" + " bar() {}\n" + " };\n" + " return MyInt;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } + void localvarthrow() { // ticket #3687 functionVariableUsage("void foo() {\n" " try {}"