diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index eb0b290ac6c..3dca55e5117 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -632,10 +632,6 @@ static MathLib::bigint asInt(const ValueFlow::Value& value) return value.isFloatValue() ? static_cast(value.floatValue) : value.intvalue; } -static std::string removeAssign(const std::string& assign) { - return std::string{assign.cbegin(), assign.cend() - 1}; -} - namespace { struct assign { template @@ -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; @@ -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()) @@ -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)) { @@ -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()) @@ -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 result = infer(makeIntegralInferModel(), diff --git a/test/cfg/std.cpp b/test/cfg/std.cpp index abe952de075..0344b98455c 100644 --- a/test/cfg/std.cpp +++ b/test/cfg/std.cpp @@ -1058,7 +1058,7 @@ 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) {} } @@ -1066,7 +1066,7 @@ 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) {} } @@ -1074,7 +1074,7 @@ 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) {} } @@ -1082,7 +1082,7 @@ 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) {} } @@ -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) diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 51bb78aa40a..dc5345ab3e3 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -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() {