walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

incorrectLogicOperator: fixed FP when comparing char values. improved handling of float comparisons.

This commit is contained in:
Daniel Marjamäki 2013-11-10 18:06:51 +01:00
parent 807f62520c
commit 4027848761
2 changed files with 54 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
lib/checkother.cpp
View File

@ -1349,7 +1349,10 @@ void CheckOther::checkIncorrectLogicOperator()
continue;
}
if (MathLib::isInt(value1) != MathLib::isInt(value2))
// Only float and int values are currently handled
if (!MathLib::isInt(value1) && !MathLib::isFloat(value1))
continue;
if (!MathLib::isInt(value2) && !MathLib::isFloat(value2))
continue;
const std::set<std::string> constStandardFunctions;
@ -1360,6 +1363,11 @@ void CheckOther::checkIncorrectLogicOperator()
const bool isfloat = MathLib::isFloat(value1) || MathLib::isFloat(value2);
// don't check floating point equality comparisons. that is bad
// and deserves different warnings.
if (isfloat && (op1=="==" || op1=="!=" || op2=="==" || op2=="!="))
continue;
// evaluate if expression is always true/false
bool alwaysTrue = true, alwaysFalse = true;
bool firstTrue = true, secondTrue = true;
@ -1367,7 +1375,28 @@ void CheckOther::checkIncorrectLogicOperator()
for (int selvalue = 1; selvalue <= 2; selvalue++) {
bool res1,res2;
if (isfloat) {
const double varvalue = MathLib::toDoubleNumber(selvalue==1 ? value1 : value2) + (double)offset;
const double d1 = MathLib::toDoubleNumber(value1);
const double d2 = MathLib::toDoubleNumber(value2);
double varvalue = (selvalue==1) ? d1 : d2;
switch (offset) {
case -3:
varvalue /= 2.0;
break;
case -2:
varvalue *= 2.0;
break;
case -1:
varvalue -= 1.0;
break;
case 1:
varvalue += 1.0;
break;
case 2:
varvalue = (d1 + d2) / 2.0;
break;
default:
break;
};
res1 = checkFloatRelation(op1, varvalue, MathLib::toDoubleNumber(value1));
res2 = checkFloatRelation(op2, varvalue, MathLib::toDoubleNumber(value2));
} else {

25
test/testother.cpp
View File

@ -129,6 +129,7 @@ private:
TEST_CASE(incorrectLogicOperator3);
TEST_CASE(incorrectLogicOperator4);
TEST_CASE(incorrectLogicOperator5);
TEST_CASE(incorrectLogicOperator6);
TEST_CASE(secondAlwaysTrueFalseWhenFirstTrueError);
TEST_CASE(incorrectLogicOp_condSwapping);
TEST_CASE(sameExpression);
@ -3835,7 +3836,8 @@ private:
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 1.0 && x == 3.0.\n", errout.str());
//ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 1.0 && x == 3.0.\n", errout.str());
ASSERT_EQUALS("", errout.str()); // float comparisons with == and != are not checked right now - such comparison is a bad idea
check("void f(float x) {\n"
" if (x == 1 && x == 1.0)\n"
@ -3863,6 +3865,13 @@ private:
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 1.0.\n", errout.str());
check("void f(int x) {\n"
" if (x >= 1.0 && x <= 1.001)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("", errout.str());
check("void f(int x) {\n"
@ -4047,13 +4056,25 @@ private:
ASSERT_EQUALS("", errout.str());
}
void incorrectLogicOperator5() {
void incorrectLogicOperator5() { // complex expressions
check("void f(int x) {\n"
" if (x+3 > 2 || x+3 < 10) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: EXPR > 2 || EXPR < 10.\n", errout.str());
}
void incorrectLogicOperator6() { // char literals
check("void f(char x) {\n"
" if (x == '1' || x == '2') {}\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f(char x) {\n"
" if (x == '1' && x == '2') {}\n"
"}");
TODO_ASSERT_EQUALS("error", "", errout.str());
}
void secondAlwaysTrueFalseWhenFirstTrueError() {
check("void f(int x) {\n"
" if (x > 5 && x != 1)\n"