Fix #14392 FN knownConditionTrueFalse (assigning function call result)

[chrchr-github]

No description provided.

[chrchr-github]

This looks like a true positive:

cppcheck/externals/tinyxml2/tinyxml2.cpp

Line 1133 in bf76975

FirstChild() &&

We don't catch this instance yet:

cppcheck/lib/checkother.cpp

Line 2927 in bf76975

const bool pointerDereference = (tok->astOperand1() && tok->astOperand1()->isUnaryOp("*")) ||

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[danmar]

I think that the original purpose of knownConditionTrueFalse was only to warn about redundant inner conditions meaning that it would be possible to remove code.

For assignment and return statements it's not possible to remove code anymore. So it feels like the purpose is a bit different. I assume the intended fix will be to replace the expression with true or false.

How does it sound to change the id also so ids are different for assignments, conditions, returns? please give me your honest opinion..

for personal reasons it would be better with separate ids; I have mapped knownConditionTrueFalse to Misra rules about invariant conditions so for that mapping to be more proper it would be better with separate ids.

[chrchr-github]

It is still possible to remove code in case of assignments and returns:

if (alwaysTrue())
    x = alwaysTrue() && doStuff(); // x = doStuff();

if (alwaysTrue())
    return alwaysTrue() || doStuff(); // return true;

Without this change, we already warn for

struct S {
	bool g() const { return m; }
	bool m{};
};
bool f(S s, int i) {
	if (s.g()) {
		const bool b = s.g() == true && i;
		return b;
	}	
	return false;
}

and call s.g() == true a condition, although it is part of an assignment.

Re: different IDs: I'm not opposed to it, if it , but that's out of scope for this PR.

[danmar]

Re: different IDs: I'm not opposed to it, if it , but that's out of scope for this PR.

ok sure.