Fix #15031 (False positive: knownConditionTrueFalse reported when calling a function with a known result)

[danmar]

No description provided.

[danmar]

I don't want to do anything drastic now.. but it feels to me like we should split out some checking after the release.

In your example code.. what is the motivation to warn about the assignment and what is the fix supposed to be?

bool f(S s) {
    if (s.g()) {
        bool b = s.g();
        return b;
    }
    return false;
}

We don't check how the assigned variable is used after the assignment right?

[chrchr-github]

I don't want to do anything drastic now.. but it feels to me like we should split out some checking after the release.

In your example code.. what is the motivation to warn about the assignment and what is the fix supposed to be?

bool f(S s) {
    if (s.g()) {
        bool b = s.g();
        return b;
    }
    return false;
}

We don't check how the assigned variable is used after the assignment right?

The motivation is in https://trac.cppcheck.net/ticket/14392
My idea above would move the warning from the assignment to the return statement.

[danmar]

My idea above would move the warning from the assignment to the return statement.

ok I am still not sure what is the motivation to warn about the return statement? It's not bad to return true from a function. It might be more explicit to write s.g() in the code than true to indicate what the returned value comes from?

I would say that my motivation and description in the doc I wrote does not mean that there should be a warning for the return statement and somehow we should write a motivation and description that match the return statement warnings.

[danmar]

currently the checker does not care about what happens after the assignment. We get a warning here too:

struct S {
        bool g() const { return m; }
        bool m{};
    };

bool f(S s) {
    if (s.g()) {
        bool b = s.g(); // <- Assigned value 's.g()' is always true [knownConditionTrueFalse]
        foo(s,&b);
    }
    return false;
}

I assume your suggestion is that we do not warn here?

[danmar]

I propose that we will split up the checker. We will create a separate id where such code:

    if (expr)
        flag = true;

can be simplified into:

    flag |= expr;

And we will create one other id where such code:

    if (expr)
        return true;
    return false;

can be simplified to:

    return expr;

with separate ids we can write distinct and "sharp" descriptions and motivations for each..

I propose that we split it up after the release.

[chrchr-github]

currently the checker does not care about what happens after the assignment. We get a warning here too:

struct S {
        bool g() const { return m; }
        bool m{};
    };

bool f(S s) {
    if (s.g()) {
        bool b = s.g(); // <- Assigned value 's.g()' is always true [knownConditionTrueFalse]
        foo(s,&b);
    }
    return false;
}

I assume your suggestion is that we do not warn here?

No, your change prevents that. My suggestion was to warn about the return statement instead. But I can open a PR implementing that later.

[chrchr-github]

I propose that we will split up the checker. We will create a separate id where such code:

    if (expr)
        flag = true;

can be simplified into:

    flag |= expr;

And we will create one other id where such code:

    if (expr)
        return true;
    return false;

can be simplified to:

    return expr;

with separate ids we can write distinct and "sharp" descriptions and motivations for each..

I propose that we split it up after the release.

Those seem like ideas for new checkers, not quite related to the current behavior.