Idea for minor performance improvement

[john borland]

When looking at the valueflow.cpp setVales function there is a while loop that most files have to run at least 3 iterations to complete the loop. I made a small rough prototype where information from the first pass of the loop can help the 2nd, 3rd and even 4th iteration of the loop. As an example if on the first pass of the loop if valueFlowSwitch doesn't detect any switch statements it returns right away on passes 2, 3, and 4. Here is my rough version https://github.com/amishscientist/cppcheck I don't think my version is near pull request ready. I thought it was at a point where I should ask to see if this kind of change would be wanted or if anyone has more thoughts along those lines.

[Paul Fultz]

As an example if on the first pass of the loop if valueFlowSwitch doesn't detect any switch statements it returns right away on passes 2, 3, and 4.

This probably works for valueFlowSwitch since part of the pass is disabled. However, for most other passes this is not the case, for example this snippet:

void f(std::vector<int> v, std::vector<int> p) {
    if (v.size() == 10 && p.size() == 9) {
        std::vector<int> a = v;
        for (int x = 0; x < a.size(); x++)
            p[x] = 0; // Out of bounds access
    }
}

So this needs valueFlowForLoop in the second pass even though the first pass doesnt produce any values. Of course, you could change the order of this so that it works in this example, but there would be other examples where it would fail(or possibly FPs), which is why we do a loop instead of trying to rely completely on order.

[john borland]

It would not be my intention to introduce an order dependency. My test cases my method only saw a 1% to 3% decreases in run time. Which is why I see it as only a minor performance increase. Is the issue in the placement of the of the orcaleInform function call. https://github.com/amishscientist/cppcheck/blob/d90b68bf58cdcc39e644ead0ad575df0ee25e0fa/lib/valueflow.cpp#L5743

If I move it higher up to here https://github.com/amishscientist/cppcheck/blob/d90b68bf58cdcc39e644ead0ad575df0ee25e0fa/lib/valueflow.cpp#L5735

Would that fix the issue or should I not do it at at in the valueFlowForLoop

So I guess I'm trying to figure out is the issue that because the for loop is under the if statement it isn't seen on the first pass or is it because I put my orcaleInform statement after the code snippet below?

        Token* tok = const_cast<Token*>(scope.classDef);
        Token* const bodyStart = const_cast<Token*>(scope.bodyStart);

        if (!Token::simpleMatch(tok->next()->astOperand2(), ";") ||
            !Token::simpleMatch(tok->next()->astOperand2()->astOperand2(), ";"))
            continue;

            orcaleInform(theOrcale);

[john borland]

I'm wondering if it would be safer if I just looped over the token list before for the while loop and checked for switch statements, for loops, abs, right shifr and smart pointers, I think one advantage of dealing with the for loops around token list is they look like this below

for (Token *tok = tokenlist->front(); tok; tok = tok->next()) {

In the case of valueFlowRightShift I could even save off the location of the first and last tok that a right shift was found and use that to start and stop the for loop.

Any thoughts?