From 82a372a380cd5ef8ddf813d38ddd2fd6f7c69cc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 20 May 2017 08:47:35 +0200 Subject: [PATCH] Try to clarify ErrorPath texts --- lib/valueflow.cpp | 52 ++++++++++++++++++++++++++++-------------- test/testvalueflow.cpp | 12 +++++----- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index e074f2737..4ad796348 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -1972,7 +1972,7 @@ static void valueFlowAfterAssign(TokenList *tokenlist, SymbolDatabase* symboldat std::list values = tok->astOperand2()->values(); for (std::list::iterator it = values.begin(); it != values.end(); ++it) { - std::string info = "Assignment, " + it->infoString(); + const std::string info = "Assignment '" + tok->expressionString() + "', assigned value is " + it->infoString(); it->errorPath.push_back(ErrorPathItem(tok->astOperand2(), info)); } const bool constValue = tok->astOperand2()->isNumber(); @@ -2536,7 +2536,7 @@ static void valueFlowForLoopSimplifyAfter(Token *fortok, unsigned int varid, con std::list values; values.push_back(ValueFlow::Value(num)); - values.back().errorPath.push_back(ErrorPathItem(fortok,"After for loop, " + values.back().infoString())); + values.back().errorPath.push_back(ErrorPathItem(fortok,"After for loop, " + var->name() + " has value " + values.back().infoString())); valueFlowForward(fortok->linkAt(1)->linkAt(1)->next(), endToken, @@ -2731,8 +2731,8 @@ static void valueFlowSubFunction(TokenList *tokenlist, ErrorLogger *errorLogger, if (!Token::Match(tok, "%name% (")) continue; - const Function * const currentFunction = tok->function(); - if (!currentFunction) { + const Function * const calledFunction = tok->function(); + if (!calledFunction) { // library function? const std::string& returnValue(settings->library.returnValue(tok)); if (!returnValue.empty()) @@ -2740,16 +2740,15 @@ static void valueFlowSubFunction(TokenList *tokenlist, ErrorLogger *errorLogger, continue; } - // Function scope.. - const Scope * const functionScope = currentFunction->functionScope; - if (!functionScope) + const Scope * const calledFunctionScope = calledFunction->functionScope; + if (!calledFunctionScope) continue; const std::vector &callArguments = getArguments(tok); for (unsigned int argnr = 0U; argnr < callArguments.size(); ++argnr) { const Token *argtok = callArguments[argnr]; // Get function argument - const Variable * const argvar = currentFunction->getArgumentVar(argnr); + const Variable * const argvar = calledFunction->getArgumentVar(argnr); if (!argvar) break; @@ -2766,15 +2765,34 @@ static void valueFlowSubFunction(TokenList *tokenlist, ErrorLogger *errorLogger, continue; // Error path.. - for (std::list::iterator it = argvalues.begin(); it != argvalues.end(); ++it) - it->errorPath.push_back(ErrorPathItem(argtok, "Function argument, " + it->infoString())); + for (std::list::iterator it = argvalues.begin(); it != argvalues.end(); ++it) { + std::string nr = MathLib::toString(argnr + 1); + if (argnr==0) + nr += "st"; + else if (argnr==1) + nr += "nd"; + else if (argnr==2) + nr += "rd"; + else + nr += "th"; + + it->errorPath.push_back(ErrorPathItem(argtok, + "Calling function '" + + calledFunction->name() + + "', " + + nr + + " argument '" + + argvar->name() + + "' value is " + + it->infoString())); +} // passed values are not "known".. for (std::list::iterator it = argvalues.begin(); it != argvalues.end(); ++it) { it->changeKnownToPossible(); } - valueFlowInjectParameter(tokenlist, errorLogger, settings, argvar, functionScope, argvalues); + valueFlowInjectParameter(tokenlist, errorLogger, settings, argvar, calledFunctionScope, argvalues); } } } @@ -2934,22 +2952,22 @@ ValueFlow::Value::Value(const Token *c, long long val) defaultArg(false), valueKind(ValueKind::Possible) { - errorPath.push_back(ErrorPathItem(c, "Condition '" + c->expressionString() + "'")); + errorPath.push_back(ErrorPathItem(c, "Assuming that condition '" + c->expressionString() + "' is not redundant")); } std::string ValueFlow::Value::infoString() const { switch (valueType) { case INT: - return "integer value " + MathLib::toString(intvalue); + return MathLib::toString(intvalue); case TOK: - return "value " + tokvalue->str(); + return tokvalue->str(); case FLOAT: - return "float value " + MathLib::toString(floatValue); + return MathLib::toString(floatValue); case MOVED: - return "value "; + return ""; case UNINIT: - return "value "; + return ""; }; throw InternalError(nullptr, "Invalid ValueFlow Value type"); } diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index f9a382b96..77ecaded4 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -595,7 +595,7 @@ private: " int x = 53;\n" " a = x;\n" "}\n"; - ASSERT_EQUALS("2,Assignment, integer value 53\n", + ASSERT_EQUALS("2,Assignment 'x=53', assigned value is 53\n", getErrorPathForX(code, 3U)); code = "void f(int y) {\n" @@ -604,8 +604,8 @@ private: " y += 12;\n" " if (y == 32) {}" "}\n"; - ASSERT_EQUALS("5,Condition 'y==32'\n" - "2,Assignment, integer value 20\n", + ASSERT_EQUALS("5,Assuming that condition 'y==32' is not redundant\n" + "2,Assignment 'x=y', assigned value is 20\n", getErrorPathForX(code, 3U)); code = "void f1(int x) {\n" @@ -615,8 +615,8 @@ private: " int x = 3;\n" " f1(x+1);\n" "}\n"; - ASSERT_EQUALS("5,Assignment, integer value 3\n" - "6,Function argument, integer value 4\n", + ASSERT_EQUALS("5,Assignment 'x=3', assigned value is 3\n" + "6,Calling function 'f1', 1st argument 'x' value is 4\n", getErrorPathForX(code, 2U)); code = "void f(int a) {\n" @@ -624,7 +624,7 @@ private: " for (x = a; x < 50; x++) {}\n" " b = x;\n" "}\n"; - ASSERT_EQUALS("3,After for loop, integer value 50\n", + ASSERT_EQUALS("3,After for loop, x has value 50\n", getErrorPathForX(code, 4U)); }