Reusing valueFlowAfterCondition for valueFlowContainerCondition

[Paul Fultz]

Hi,

So the valueFlowAfterCondition takes care of a lot of the cases for forwarding the conditional values. We could refactor this to a more generic function that takes two functions:

{{{
template<class forward="" condition,="" class="">
static void valueFlowCondition(TokenList tokenlist, SymbolDatabase symboldatabase, ErrorLogger errorLogger, const Settings settings, Condition condition, Forward forward);
}}}</class>

I use templates here, but function pointers could be used instead if you dont like the templates. So the Condition function would return ConditionValue struct like this:

{{{
struct ConditionValue
{
std::list<valueflow::value> true_values = {};
std::list<valueflow::value> false_values = {};
const Token * vartok = nullptr;
};
}}}</valueflow::value></valueflow::value>

The Forward function would call valueFlowForward or the valueFlowContainerForward. This way we can reuse the conditional logic for both container sizes and for regular values. Would you guys be interested in such refactoring in cppcheck?

Here is a rough implementation of such a function:

{{{
template<class forward="" condition,="" class="">
static void valueFlowCondition(TokenList tokenlist, SymbolDatabase symboldatabase, ErrorLogger errorLogger, const Settings settings, Condition condition, Forward forward)
{
for (const Scope * scope : symboldatabase->functionScopes) {
std::set<unsigned> aliased;
for (Token* tok = const_cast<token*>(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) {
if (Token::Match(tok, "= & %var% ;"))
aliased.insert(tok->tokAt(2)->varId());</token*></unsigned></class>

       ConditionValue cv = condition(tok);
       if(!vc.vartok)
           continue;

       const unsigned int varid = cv.vartok->varId();
       if (varid == 0U)
           continue;
       const Variable *var = cv.vartok->variable();
       if (!var || !(var->isLocal() || var->isGlobal() || var->isArgument()))
           continue;
       if (aliased.find(varid) != aliased.end()) {
           if (settings->debugwarnings)
               bailout(tokenlist, errorLogger, cv.vartok, "variable is aliased so we just skip all valueflow after condition");
           continue;
       }

       if (Token::Match(tok->astParent(), "%oror%|&&")) {
           Token *parent = const_cast<Token*>(tok->astParent());
           const std::string &op(parent->str());

           if (parent->astOperand1() == tok &&
               ((op == "&&" && Token::Match(tok, "==|>=|<=|!")) ||
                (op == "||" && Token::Match(tok, "%name%|!=")))) {
               for (; parent && parent->str() == op; parent = const_cast<Token*>(parent->astParent())) {
                   std::stack<Token *> tokens;
                   tokens.push(const_cast<Token*>(parent->astOperand2()));
                   bool assign = false;
                   while (!tokens.empty()) {
                       Token *rhstok = tokens.top();
                       tokens.pop();
                       if (!rhstok)
                           continue;
                       tokens.push(const_cast<Token*>(rhstok->astOperand1()));
                       tokens.push(const_cast<Token*>(rhstok->astOperand2()));
                       if (rhstok->varId() == varid)
                           setTokenValue(rhstok, cv.true_values.front(), settings);
                       else if (Token::Match(rhstok, "++|--|=") && Token::Match(rhstok->astOperand1(), "%varid%", varid)) {
                           assign = true;
                           break;
                       }
                   }
                   if (assign)
                       break;
                   while (parent->astParent() && parent == parent->astParent()->astOperand2())
                       parent = const_cast<Token*>(parent->astParent());
               }
           }
       }

       const Token *top = tok->astTop();
       if (top && Token::Match(top->previous(), "if|while (") && !top->previous()->isExpandedMacro()) {
           // does condition reassign variable?
           if (tok != top->astOperand2() &&
               Token::Match(top->astOperand2(), "%oror%|&&") &&
               isVariableChanged(top, top->link(), varid, var->isGlobal(), settings, tokenlist->isCPP())) {
               if (settings->debugwarnings)
                   bailout(tokenlist, errorLogger, tok, "assignment in condition");
               continue;
           }

           // start token of conditional code
           Token *startTokens[] = { nullptr, nullptr };

           // based on the comparison, should we check the if or while?
           bool check_if = false;
           bool check_else = false;
           if (Token::Match(tok, "==|>=|<=|!|>|<"))
               check_if = true;
           if (Token::Match(tok, "%name%|!=|>|<"))
               check_else = true;

           if (!check_if && !check_else)
               continue;

           // if astParent is "!" we need to invert codeblock
           {
               const Token *parent = tok->astParent();
               while (parent && parent->str() == "&&")
                   parent = parent->astParent();
               if (parent && parent->str() == "!") {
                   check_if = !check_if;
                   check_else = !check_else;
               }
           }

           // determine startToken(s)
           if (check_if && Token::simpleMatch(top->link(), ") {"))
               startTokens[0] = top->link()->next();
           if (check_else && Token::simpleMatch(top->link()->linkAt(1), "} else {"))
               startTokens[1] = top->link()->linkAt(1)->tokAt(2);

           bool bail = false;

           for (int i=0; i<2; i++) {
               Token * startToken = startTokens[i];
               if (!startToken)
                   continue;
               std::list<ValueFlow::Value> & values = (i==0 ? cv.true_values : cv.false_values);
               if (values.size() == 1U && Token::Match(tok, "==|!")) {
                   const Token *parent = tok->astParent();
                   while (parent && parent->str() == "&&")
                       parent = parent->astParent();
                   if (parent && parent->str() == "(")
                       values.front().setKnown();
               }

               forward(startTokens[i]->next(), startTokens[i]->link(), var, varid, values, true, false, tokenlist, errorLogger, settings);
               values.front().setPossible();
               if (isVariableChanged(startTokens[i], startTokens[i]->link(), varid, var->isGlobal(), settings, tokenlist->isCPP())) {
                   // TODO: The endToken should not be startTokens[i]->link() in the forward call
                   if (settings->debugwarnings)
                       bailout(tokenlist, errorLogger, startTokens[i]->link(), "valueFlowCondition: " + var->name() + " is changed in conditional block");
                   bail = true;
                   break;
               }
           }
           if (bail)
               continue;

           // After conditional code..
           if (Token::simpleMatch(top->link(), ") {")) {
               Token *after = top->link()->linkAt(1);
               std::string unknownFunction;
               if (settings->library.isScopeNoReturn(after, &unknownFunction)) {
                   if (settings->debugwarnings && !unknownFunction.empty())
                       bailout(tokenlist, errorLogger, after, "possible noreturn scope");
                   continue;
               }

               const bool dead_if = isReturnScope(after);
               bool dead_else = false;

               if (Token::simpleMatch(after, "} else {")) {
                   after = after->linkAt(2);
                   if (Token::simpleMatch(after->tokAt(-2), ") ; }")) {
                       if (settings->debugwarnings)
                           bailout(tokenlist, errorLogger, after, "possible noreturn scope");
                       continue;
                   }
                   dead_else = isReturnScope(after);
               }

               std::list<ValueFlow::Value> * values = nullptr;
               if (!dead_if && check_if)
                   values = &cv.true_values;
               else if (!dead_else && check_else)
                   values = &cv.false_values;

               if (values) {
                   // TODO: constValue could be true if there are no assignments in the conditional blocks and
                   //       perhaps if there are no && and no || in the condition
                   bool constValue = false;
                   forward(after->next(), top->scope()->bodyEnd, var, varid, *values, constValue, false, tokenlist, errorLogger, settings);
               }
           }
       }
   }

}
}
}}}