From 4149617978c6d9c20ac369a89b5827701f2661cc Mon Sep 17 00:00:00 2001 From: Robert Reif Date: Sat, 16 Jul 2011 23:05:35 -0400 Subject: [PATCH] fix #2827 to use numeric comparisons --- lib/checkother.cpp | 28 ++++++++++----------- lib/mathlib.cpp | 25 ++++++++++++++++++ lib/mathlib.h | 5 ++++ test/testother.cpp | 63 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 107 insertions(+), 14 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 7bef92619..901d85636 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -745,14 +745,14 @@ void CheckOther::checkIncorrectLogicOperator() { "(", true, Second, "<", "&&", First, "<", ")", true, MoreEqual, false }, // (3 < x) && (x < 1) <- always false { "(", true, Second, ">", "&&", Second, "<", ")", true, LessEqual, false }, // (1 > x) && (3 < x) <- always false { "(", true, Second, "<", "&&", Second, ">", ")", true, MoreEqual, false }, // (3 < x) && (1 > x) <- always false - { "(", true, First , ">", "||", First, "<", ")", true, More, true }, // (x > 3) || (x < 10) <- always true - { "(", true, First , "<", "||", First, ">", ")", true, Less, true }, // (x < 10) || (x > 3) <- always true - { "(", true, Second, "<", "||", First, "<", ")", true, More, true }, // (3 < x) || (x < 10) <- always true - { "(", true, First, "<", "||", Second, "<", ")", true, Less, true }, // (x < 10) || (3 < x) <- always true - { "(", true, First, ">", "||", Second, ">", ")", true, More, true }, // (x > 3) || (10 > x) <- always true - { "(", true, Second, ">", "||", First, ">", ")", true, Less, true }, // (10 > x) || (x > 3) <- always true - { "(", true, Second, "<", "||", Second, ">", ")", true, More, true }, // (3 < x) || (10 > x) <- always true - { "(", true, Second, ">", "||", Second, "<", ")", true, Less, true }, // (10 > x) || (3 < x) <- always true + { "(", true, First , ">", "||", First, "<", ")", true, Less, true }, // (x > 3) || (x < 10) <- always true + { "(", true, First , "<", "||", First, ">", ")", true, More, true }, // (x < 10) || (x > 3) <- always true + { "(", true, Second, "<", "||", First, "<", ")", true, Less, true }, // (3 < x) || (x < 10) <- always true + { "(", true, First, "<", "||", Second, "<", ")", true, More, true }, // (x < 10) || (3 < x) <- always true + { "(", true, First, ">", "||", Second, ">", ")", true, Less, true }, // (x > 3) || (10 > x) <- always true + { "(", true, Second, ">", "||", First, ">", ")", true, More, true }, // (10 > x) || (x > 3) <- always true + { "(", true, Second, "<", "||", Second, ">", ")", true, Less, true }, // (3 < x) || (10 > x) <- always true + { "(", true, Second, ">", "||", Second, "<", ")", true, More, true }, // (10 > x) || (3 < x) <- always true }; for (unsigned int i = 0; i < (sizeof(conditions) / sizeof(conditions[0])); i++) @@ -778,12 +778,12 @@ void CheckOther::checkIncorrectLogicOperator() if (!((conditions[i].afterEqual && (conditions[i].after == nextTok->str())) || (!conditions[i].afterEqual && (conditions[i].after != nextTok->str())))) continue; - if ((conditions[i].relation == Equal && firstConstant == secondConstant) || - (conditions[i].relation == NotEqual && firstConstant != secondConstant) || - (conditions[i].relation == Less && firstConstant < secondConstant) || - (conditions[i].relation == LessEqual && firstConstant <= secondConstant) || - (conditions[i].relation == More && firstConstant > secondConstant) || - (conditions[i].relation == MoreEqual && firstConstant >= secondConstant)) + if ((conditions[i].relation == Equal && MathLib::isEqual(firstConstant, secondConstant)) || + (conditions[i].relation == NotEqual && MathLib::isNotEqual(firstConstant, secondConstant)) || + (conditions[i].relation == Less && MathLib::isLess(firstConstant, secondConstant)) || + (conditions[i].relation == LessEqual && MathLib::isLessEqual(firstConstant, secondConstant)) || + (conditions[i].relation == More && MathLib::isGreater(firstConstant, secondConstant)) || + (conditions[i].relation == MoreEqual && MathLib::isGreaterEqual(firstConstant, secondConstant))) incorrectLogicOperatorError(term1Tok, conditions[i].state); } } diff --git a/lib/mathlib.cpp b/lib/mathlib.cpp index a047718da..b99c3a648 100644 --- a/lib/mathlib.cpp +++ b/lib/mathlib.cpp @@ -297,11 +297,36 @@ std::string MathLib::abs(const std::string &tok) return toString(std::abs(toDoubleNumber(tok))); } +bool MathLib::isEqual(const std::string &first, const std::string &second) +{ + return toDoubleNumber(first) == toDoubleNumber(second); +} + +bool MathLib::isNotEqual(const std::string &first, const std::string &second) +{ + return toDoubleNumber(first) != toDoubleNumber(second); +} + bool MathLib::isGreater(const std::string &first, const std::string &second) { return toDoubleNumber(first) > toDoubleNumber(second); } +bool MathLib::isGreaterEqual(const std::string &first, const std::string &second) +{ + return toDoubleNumber(first) >= toDoubleNumber(second); +} + +bool MathLib::isLess(const std::string &first, const std::string &second) +{ + return toDoubleNumber(first) < toDoubleNumber(second); +} + +bool MathLib::isLessEqual(const std::string &first, const std::string &second) +{ + return toDoubleNumber(first) <= toDoubleNumber(second); +} + bool MathLib::isNullValue(const std::string &str) { return (str == "-0" || str == "-0.0" diff --git a/lib/mathlib.h b/lib/mathlib.h index 9c7f414fa..4c01eff2a 100644 --- a/lib/mathlib.h +++ b/lib/mathlib.h @@ -66,7 +66,12 @@ public: static std::string cos(const std::string & tok); static std::string tan(const std::string & tok); static std::string abs(const std::string & tok); + static bool isEqual(const std::string & first, const std::string & second); + static bool isNotEqual(const std::string & first, const std::string & second); static bool isGreater(const std::string & first, const std::string & second); + static bool isGreaterEqual(const std::string & first, const std::string & second); + static bool isLess(const std::string & first, const std::string & second); + static bool isLessEqual(const std::string & first, const std::string & second); static bool isNullValue(const std::string &tok); /** * Return true if given character is 0,1,2,3,4,5,6 or 7. diff --git a/test/testother.cpp b/test/testother.cpp index be52d8340..d6c399040 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -2138,6 +2138,27 @@ private: ); ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str()); + check("void f(int x) {\n" + " if ((x == 1.0) && (x == 3.0))\n" + " a++;\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str()); + + check("void f(float x) {\n" + " if ((x == 1) && (x == 1.0))\n" + " a++;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + check("void f(int x) {\n" + " if ((x == 1) && (x == 0x00000001))\n" + " a++;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + check("void f(int x) {\n" " if (x == 1 && x == 3)\n" " a++;\n" @@ -2145,6 +2166,41 @@ private: ); ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str()); + check("void f(int x) {\n" + " if (x == 1.0 && x == 3.0)\n" + " a++;\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str()); + + check("void f(float x) {\n" + " if (x == 1 && x == 1.0)\n" + " a++;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + check("void f(int x) {\n" + " if (x < 1 && x > 1)\n" + " a++;\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str()); + + check("void f(int x) {\n" + " if (x < 1.0 && x > 1.0)\n" + " a++;\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str()); + + check("void f(int x) {\n" + " if (x < 1 && x > 1.0)\n" + " a++;\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str()); + check("void f(int x) {\n" " if (x < 1 && x > 3)\n" " a++;\n" @@ -2152,6 +2208,13 @@ private: ); ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str()); + check("void f(float x) {\n" + " if (x < 1.0 && x > 3.0)\n" + " a++;\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str()); + check("void f(int x) {\n" " if (1 > x && 3 < x)\n" " a++;\n"