Skip to content

Fix #15145 FN knownArgumentHiddenVariableExpression in if scope #8974

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chrchr-github wants to merge 7 commits into
base: main
Choose a base branch
from
+18 −6
Open

Fix #15145 FN knownArgumentHiddenVariableExpression in if scope #8974

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a2c9ccf
Update checkother.cpp
chrchr-github Oct 28, 2026
246b6e6
Update testother.cpp
chrchr-github Oct 28, 2026
e04ce1d
Update checkother.cpp
chrchr-github Oct 28, 2026
fc4beba
Update testother.cpp [skip ci]
chrchr-github Nov 12, 2026
d8ddb9b
Update checkother.cpp [skip ci]
chrchr-github Nov 12, 2026
b6a8a32
Merge branch 'cppcheck-opensource:main' into chr_15145
chrchr-github Nov 12, 2026
2f2568c
Update checkother.cpp
chrchr-github Nov 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions lib/checkother.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4383,7 +4383,7 @@ void CheckOtherImpl::checkKnownArgument()
for (const Token *tok = functionScope->bodyStart; tok != functionScope->bodyEnd; tok = tok->next()) {
if (!tok->hasKnownIntValue() || tok->isExpandedMacro())
continue;
if (Token::Match(tok, "++|--|%assign%"))
if (Token::Match(tok, "++|--|%assign%|%comp%"))

@dmcppcheck dmcppcheck Nov 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review

Skipping every %comp% token here drops correct warnings that base reports. For example, g(x * 0 == 0), g((x & 0) != 0), h(x - x == 0) and g(x * 0 > -1) were all reported before (knownArgument or knownArgumentHiddenVariableExpression), and now none of them is. The removed isSameExpression check only skipped comparisons like x == x.

The skip also doesn't stop the impossible-value false positive once the comparison is nested: if (x != 0) g((x != 0) * 1); and unsigned g((x >= 0) + 0) are still reported with "It does not matter what value 'x' has".

It may be better to restore the old comparison handling and limit the relaxed values() test in the lambda below, so that impossible values are only accepted when a literal operand hides the variable (*/& with 0, &&/|| with a constant). Please also add tests for these cases.

continue;
if (!Token::Match(tok->astParent(), "(|{|,"))
continue;
Expand All @@ -4408,17 +4408,16 @@ void CheckOtherImpl::checkKnownArgument()
tok2 = tok2->astOperand2();
if (isVariableExpression(tok2))
continue;
if (tok->isComparisonOp() &&
isSameExpression(
true, tok->astOperand1(), tok->astOperand2(), mSettings, true, true))
continue;
// ensure that there is a integer variable in expression with unknown value
const Token* vartok = nullptr;
visitAstNodes(tok, [&](const Token* child) {
if (Token::Match(child, "%var%|.|[")) {
if (child->hasKnownIntValue())
return ChildrenToVisit::none;
if (astIsIntegral(child, false) && !astIsPointer(child) && child->values().empty()) {
if (astIsIntegral(child, false) && !astIsPointer(child) &&
std::none_of(child->values().begin(), child->values().end(), [](const ValueFlow::Value& v) {
return v.isIntValue() && !v.isImpossible();
})) {
vartok = child;
return ChildrenToVisit::done;
}
Expand Down
13 changes: 13 additions & 0 deletions test/testother.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13888,6 +13888,19 @@ class TestOther : public TestFixture {
"[test.cpp:5:18]: (style) Argument 'true||x' to function dostuff is always 1. Constant literal calculation disable/hide variable expression 'x'. [knownArgumentHiddenVariableExpression]\n"
"[test.cpp:6:15]: (style) Argument 'x*0' to function dostuff is always 0. Constant literal calculation disable/hide variable expression 'x'. [knownArgumentHiddenVariableExpression]\n"
"[test.cpp:7:15]: (style) Argument '0*x' to function dostuff is always 0. Constant literal calculation disable/hide variable expression 'x'. [knownArgumentHiddenVariableExpression]\n", errout_str());

check("void f1(int x) {\n" // #15145
" if (x != 0)\n"
" g(0 * x);\n"
"}\n"
"void f2(unsigned x) {\n"
" g(x >= 0);\n"
" if (x != 5)\n"
" g(x == 5);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:6:9]: (style) Unsigned expression 'x' can't be negative so it is unnecessary to test it. [unsignedPositive]\n"
"[test.cpp:3:13]: (style) Argument '0*x' to function g is always 0. Constant literal calculation disable/hide variable expression 'x'. [knownArgumentHiddenVariableExpression]\n",
errout_str());
}

void knownArgumentTernaryOperator() { // #10374
Expand Down
Loading