Fix #15005: FP uninitdata with compound assignment or increment

[aadanen]

isVariableUsage() currently returns nullptr for the lhs of assignment operators, because unless it's a pointer that gets dereferenced, it is just getting overwritten and its potentially uninitialized value is not read. I extend this idea to compound assignment and increment/decrement operators.

While working on this, I found some issues with CTU analysis.

        valueFlowUninit("void increment(int& i) { ++i; }\n" // #6475
                        "int f() {\n"
                        "    int n;\n"
                        "    increment(n);\n"
                        "    return n;\n"
                        "}\n");
        ASSERT_EQUALS("[test.cpp:4:15] -> [test.cpp:1:28]: (warning) Uninitialized variable: i [uninitvar]\n", errout_str());

this test only works because of the false positive "usage" from the increment operator. For example, this test breaks if we use regular assignment because of the current code properly handling "="

this code

void increment(int& i) { i = i + 1; }
int f() {
    int n;
    increment(n);
    return n;
}

misses the ctu error, and only throws

Checking examples/ctu3.cpp ...
examples/ctu3.cpp:1:30: warning: Uninitialized variable: i [uninitvar]
void increment(int& i) { i = i + 1; }
                             ^
examples/ctu3.cpp:4:15: note: Calling function 'increment', 1st argument 'n' value is <Uninit>
    increment(n);
              ^
examples/ctu3.cpp:1:30: note: Uninitialized variable: i
void increment(int& i) { i = i + 1; }
                             ^

I think that the problem has to do with

// NOLINTNEXTLINE(readability-non-const-parameter) - used as callback so we need to preserve the signature
static bool isVariableUsage(const Settings &settings, const Token *vartok, MathLib::bigint *value)
{
    (void)value;
    return !!CheckUninitVarImpl::isVariableUsage(vartok, settings.library, true, CheckUninitVarImpl::Alloc::ARRAY);
}

the hardcoded pointer=true and alloc=ARRAY assignments tell isVariableUsage to return nullptr if it i isn't dereferenced, which it isn't. If we are going to hardcode these values to be this conservative then this test should fail because it currently only passes due to the FP bug I want to fix. I currently have a workaround where I use vartok->variable() to determine whether something is actually a pointer/array.

Maybe there can be another PR where we rework the ctu/uninitvar connection. Notably since I don't touch the "=" branch of the code the false negative I mentioned above remains unfixed. Let me know if I should apply my vartok workaround to that code in this PR, if it should be another PR, or if its just the wrong idea overall.

[danmar]

it looks good to me. just have some nits to simplify the code a little.

[aadanen]

If the simplifications were the last changes to make, I think this is ready to merge! Thanks for the review :)

[danmar]

This is an AI review take it with a grain of salt. Please feel free to reject it by clicking on Resolve button

The change looks safe to me from the false-positive point of view. The new isVariableUsage() branch only returns "not a usage" for a pointer/array that is not dereferenced. The %assign% change in checkScopeForVariable() only adds a check of the right-hand side; the "assume that variable is assigned" path after it is unchanged. CI is green.

Small nit: the // FP here reads as if a false positive is expected, but the test asserts no warning. Maybe drop it, or write // #15005 as the reference? There's also a double blank line before the first new test.