Skip to content

Fix #14734 FP invalidFunctionArg with ternary expression #8538

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 3 commits into
base: main
Choose a base branch
from
+28 −20
Open

Fix #14734 FP invalidFunctionArg with ternary expression #8538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
66c6902
Fix #14734 FP invalidFunctionArg with ternary expression
web-flow May 7, 2026
2ab0a7b
Fix
web-flow May 7, 2026
eeb1086
Avoid string comparison
web-flow May 7, 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
25 changes: 11 additions & 14 deletions lib/programmemory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,10 +632,6 @@ static MathLib::bigint asInt(const ValueFlow::Value& value)
return value.isFloatValue() ? static_cast<MathLib::bigint>(value.floatValue) : value.intvalue;
}

static std::string removeAssign(const std::string& assign) {
return std::string{assign.cbegin(), assign.cend() - 1};
}

namespace {
struct assign {
template<class T, class U>
Expand All @@ -651,22 +647,23 @@ static bool isIntegralValue(const ValueFlow::Value& value)
return value.isIntValue() || value.isIteratorValue() || value.isSymbolicValue();
}

static ValueFlow::Value evaluate(const std::string& op, const ValueFlow::Value& lhs, const ValueFlow::Value& rhs)
static ValueFlow::Value evaluate(const Token* op, const ValueFlow::Value& lhs, const ValueFlow::Value& rhs, bool removeAssign = false)
{
const std::string opStr = removeAssign ? op->str().substr(0, op->str().size() - 1) : op->str();
ValueFlow::Value result;
if (lhs.isImpossible() && rhs.isImpossible())
return ValueFlow::Value::unknown();
if (lhs.isImpossible() || rhs.isImpossible()) {
// noninvertible
if (contains({"%", "/", "&", "|"}, op))
if (contains({"%", "/", "&", "|"}, opStr))
return ValueFlow::Value::unknown();
result.setImpossible();
}
if (isNumericValue(lhs) && isNumericValue(rhs)) {
if (lhs.isFloatValue() || rhs.isFloatValue()) {
result.valueType = ValueFlow::Value::ValueType::FLOAT;
result.valueType = op->isArithmeticalOp() ? ValueFlow::Value::ValueType::FLOAT : ValueFlow::Value::ValueType::INT;
bool error = false;
result.floatValue = calculate(op, asFloat(lhs), asFloat(rhs), &error);
result.floatValue = calculate(opStr, asFloat(lhs), asFloat(rhs), &error);
if (error)
return ValueFlow::Value::unknown();
return result;
Expand All @@ -678,12 +675,12 @@ static ValueFlow::Value evaluate(const std::string& op, const ValueFlow::Value&
// If not the same type then one must be int
if (lhs.valueType != rhs.valueType && !lhs.isIntValue() && !rhs.isIntValue())
return ValueFlow::Value::unknown();
const bool compareOp = contains({"==", "!=", "<", ">", ">=", "<="}, op);
const bool compareOp = op->isComparisonOp();
// Comparison must be the same type
if (compareOp && lhs.valueType != rhs.valueType)
return ValueFlow::Value::unknown();
// Only add, subtract, and compare for non-integers
if (!compareOp && !contains({"+", "-"}, op) && !lhs.isIntValue() && !rhs.isIntValue())
if (!compareOp && !contains({"+", "-"}, opStr) && !lhs.isIntValue() && !rhs.isIntValue())
return ValueFlow::Value::unknown();
// Both can't be iterators for non-compare
if (!compareOp && lhs.isIteratorValue() && rhs.isIteratorValue())
Expand All @@ -701,10 +698,10 @@ static ValueFlow::Value evaluate(const std::string& op, const ValueFlow::Value&
result.valueType = ValueFlow::Value::ValueType::INT;
}
bool error = false;
result.intvalue = calculate(op, lhs.intvalue, rhs.intvalue, &error);
result.intvalue = calculate(opStr, lhs.intvalue, rhs.intvalue, &error);
if (error)
return ValueFlow::Value::unknown();
if (result.isImpossible() && op == "!=") {
if (result.isImpossible() && opStr == "!=") {
if (isTrue(result)) {
result.intvalue = 1;
} else if (isFalse(result)) {
Expand Down Expand Up @@ -1501,7 +1498,7 @@ namespace {
if (!pm->hasValue(expr->astOperand1()->exprId()))
return unknown();
ValueFlow::Value& lhs = pm->at(expr->astOperand1()->exprId());
rhs = evaluate(removeAssign(expr->str()), lhs, rhs);
rhs = evaluate(expr, lhs, rhs, /*removeAssign*/ true);
if (lhs.isIntValue())
ValueFlow::Value::visitValue(rhs, std::bind(assign{}, std::ref(lhs.intvalue), std::placeholders::_1));
else if (lhs.isFloatValue())
Expand Down Expand Up @@ -1565,7 +1562,7 @@ namespace {
ValueFlow::Value rhs = execute(expr->astOperand2());
if (rhs.isUninitValue())
return unknown();
ValueFlow::Value r = evaluate(expr->str(), lhs, rhs);
ValueFlow::Value r = evaluate(expr, lhs, rhs);
if (expr->isComparisonOp() && (r.isUninitValue() || r.isImpossible())) {
if (rhs.isIntValue() && !expr->astOperand1()->values().empty()) {
std::vector<ValueFlow::Value> result = infer(makeIntegralInferModel(),
Expand Down
14 changes: 8 additions & 6 deletions test/cfg/std.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1058,31 +1058,31 @@ void returnValue_std_isgreater(void)
{
// cppcheck-suppress knownConditionTrueFalse
if (std::isgreater(4,2) == 0) {}
// @todo support floats
// cppcheck-suppress knownConditionTrueFalse
if (std::isgreater(4.0f,2.0f) == 0) {}
}

void returnValue_std_isgreaterequal(void)
{
// cppcheck-suppress knownConditionTrueFalse
if (std::isgreaterequal(4,2) == 0) {}
// @todo support floats
// cppcheck-suppress knownConditionTrueFalse
if (std::isgreaterequal(4.0f,2.0f) == 0) {}
}

void returnValue_std_isless(void)
{
// cppcheck-suppress knownConditionTrueFalse
if (std::isless(4,2) == 0) {}
// @todo support floats
// cppcheck-suppress knownConditionTrueFalse
if (std::isless(4.0f,2.0f) == 0) {}
}

void returnValue_std_islessequal(void)
{
// cppcheck-suppress knownConditionTrueFalse
if (std::islessequal(4,2) == 0) {}
// @todo support floats
// cppcheck-suppress knownConditionTrueFalse
if (std::islessequal(4.0f,2.0f) == 0) {}
}

Expand All @@ -1093,8 +1093,10 @@ void returnValue_std_islessgreater(void)
// cppcheck-suppress knownConditionTrueFalse
if (std::islessgreater(2,4) == 0) {}

if (std::islessgreater(4.0f,2.0f) == 0) {} // @todo support floats
if (std::islessgreater(2.0f,4.0f) == 0) {} // @todo support floats
// cppcheck-suppress knownConditionTrueFalse
if (std::islessgreater(4.0f,2.0f) == 0) {}
// cppcheck-suppress knownConditionTrueFalse
if (std::islessgreater(2.0f,4.0f) == 0) {}
}

void bufferAccessOutOfBounds(void)
Expand Down
9 changes: 9 additions & 0 deletions test/testvalueflow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4119,6 +4119,15 @@ class TestValueFlow : public TestFixture {
"}\n";
ASSERT_EQUALS(true, testValueOfX(code, 4U, 0));
ASSERT_EQUALS(false, testValueOfXImpossible(code, 4U, 0));

code = "double f(double d, bool b) {\n" // #14734
" double s = 0.0;\n"
" if (b)\n"
" s += d;\n"
" return s > 0.0 ? s : 0.0;\n"
"}\n";
auto values = tokenValues(code, "s :", ValueFlow::Value::ValueType::FLOAT);
ASSERT_EQUALS(0, values.size());
}

void valueFlowForwardLambda() {
Expand Down
Loading