Fix #10792 FP knownConditionTrueFalse with double to int cast#3964
Fix #10792 FP knownConditionTrueFalse with double to int cast#3964danmar merged 7 commits intocppcheck-opensource:mainfrom
Conversation
| ValueFlow::getSizeOf(*tok->valueType(), settings) >= ValueFlow::getSizeOf(valueType, settings)) | ||
| return; | ||
| if (tok->valueType() && tok->valueType()->isFloat() && valueType.isIntegral()) | ||
| value.valueType = ValueFlow::Value::ValueType::FLOAT; |
There was a problem hiding this comment.
Maybe this should be in setTokenValueCast?
There was a problem hiding this comment.
I think the type of !='s value should not be INT, so that
void f(double d) {
if (d != 0) {
int i = d;
if (i == 0) {}
}
}
is also handled correctly.
There was a problem hiding this comment.
The example above gives an oppositeInnerCondition error, which seems to be a different issue.
There was a problem hiding this comment.
Moving the code to setTokenValueCast() does not work, since tok is not available. I have now moved it to setConditionalValues(), seems to work so far.
| if (Token::Match(tok, "==|!=|>=|<=")) { | ||
| true_value = ValueFlow::Value{tok, value}; | ||
| const Variable* var1 = tok->astOperand1()->variable(); | ||
| const Variable* var2 = tok->astOperand2()->variable(); |
There was a problem hiding this comment.
This wont work if either side is not a variable like in:
struct A { double d; };
void f(A a) {
if (a.d != 0) {
int i = a.d;
if (i == 0) {}
}
}
| if ((var1 && var1->isFloatingType()) || (var2 && var2->isFloatingType())) { | ||
| true_value.valueType = ValueFlow::Value::ValueType::FLOAT; | ||
| false_value.valueType = ValueFlow::Value::ValueType::FLOAT; | ||
| } |
There was a problem hiding this comment.
The code below doesnt work for floating point. This function never sets floatValue which is what the ValueType::FLOAT uses, and it uses incorrect logic for the bounds(ie value + 1 instead of using float epsilon). It probably works because floatValue is zero by default, and you are only testing with equality.
Actually, this function is mainly used in parseCompareInt(and also for symbolic conditions) which are for integral-like comparisons. I think it would be better to return null in parseCompareInt if the return token is a float. So you could add a check in addition to isSaturated check:
const Token *parseCompareInt(const Token *tok, ValueFlow::Value &true_value, ValueFlow::Value &false_value, const std::function<std::vector<MathLib::bigint>(const Token*)>& evaluate)
{
...
if (tok->isComparisonOp()) {
...
if (!value1.empty()) {
if (isSaturated(value1.front()) || astIsFloat(tok->astOperand2()))
return nullptr;
setConditionalValues(tok, true, value1.front(), true_value, false_value);
return tok->astOperand2();
} else if (!value2.empty()) {
if (isSaturated(value2.front())|| astIsFloat(tok->astOperand1()))
return nullptr;
setConditionalValues(tok, false, value2.front(), true_value, false_value);
return tok->astOperand1();
}
}
...
}We should probably add better support for float comparisons in the future, but they are little difficult due to not being totally ordered(ie d < 0.0 != 0.0 > d).
3 participants
No description provided.