diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 6c8258623..1088d8b42 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -227,6 +227,44 @@ void CheckOther::clarifyConditionError(const Token *tok, bool assign, bool boolo errmsg); } +//--------------------------------------------------------------------------- +// if (bool & bool) -> if (bool && bool) +// if (bool | bool) -> if (bool || bool) +//--------------------------------------------------------------------------- +void CheckOther::checkBitwiseOnBoolean() +{ + if (!_settings->isEnabled("style")) + return; + + // danmar: this is inconclusive because I don't like that there are + // warnings for calculations. Example: set_flag(a & b); + if (!_settings->inconclusive) + return; + + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + if (Token::Match(tok, "(|.|return %var% [&|]")) + { + if (tok->next()->varId()) + { + const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(tok->next()->varId()); + if (var && (var->typeStartToken() == var->typeEndToken()) && + var->typeStartToken()->str() == "bool") + { + bitwiseOnBooleanError(tok->next(), tok->next()->str(), tok->strAt(2) == "&" ? "&&" : "||"); + } + } + } + } +} + +void CheckOther::bitwiseOnBooleanError(const Token *tok, const std::string &varname, const std::string &op) +{ + reportInconclusiveError(tok, Severity::style, "bitwiseOnBoolean", + "Boolean variable '" + varname + "' is used in bitwise operation. Did you mean " + op + " ?"); +} + +//--------------------------------------------------------------------------- //--------------------------------------------------------------------------- void CheckOther::warningOldStylePointerCast() { diff --git a/lib/checkother.h b/lib/checkother.h index c1ab0f1e3..0af0d96c9 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -102,6 +102,7 @@ public: checkOther.checkAssignBoolToPointer(); checkOther.checkSignOfUnsignedVariable(); + checkOther.checkBitwiseOnBoolean(); } /** @brief Clarify calculation for ".. a * b ? .." */ @@ -225,6 +226,9 @@ public: /** @brief %Check for testing sign of unsigned variable */ void checkSignOfUnsignedVariable(); + /** @brief %Check for using bool in bitwise expression */ + void checkBitwiseOnBoolean(); + // Error messages.. void cstyleCastError(const Token *tok); void dangerousUsageStrtolError(const Token *tok); @@ -261,6 +265,7 @@ public: void assignBoolToPointerError(const Token *tok); void unsignedLessThanZeroError(const Token *tok, const std::string &varname); void unsignedPositiveError(const Token *tok, const std::string &varname); + void bitwiseOnBooleanError(const Token *tok, const std::string &varname, const std::string &op); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) { @@ -309,6 +314,7 @@ public: c.duplicateBreakError(0); c.unsignedLessThanZeroError(0, "varname"); c.unsignedPositiveError(0, "varname"); + c.bitwiseOnBooleanError(0, "varname", "&&"); } std::string myName() const @@ -357,6 +363,7 @@ public: "* duplicate break statement\n" "* testing if unsigned variable is negative\n" "* testing is unsigned variable is positive\n" + "* using bool in bitwise expression\n" // optimisations "* optimisation: detect post increment/decrement\n"; diff --git a/test/testother.cpp b/test/testother.cpp index a6ba70f93..cfe62ff8b 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -121,6 +121,7 @@ private: TEST_CASE(clarifyCondition2); // if (a & b == c) TEST_CASE(clarifyCondition3); // if (! a & b) TEST_CASE(clarifyCondition4); // ticket #3110 + TEST_CASE(bitwiseOnBoolean); // if (bool & bool) TEST_CASE(incorrectStringCompare); @@ -165,6 +166,7 @@ private: checkOther.checkDuplicateIf(); checkOther.checkDuplicateBranch(); checkOther.checkDuplicateExpression(); + checkOther.checkBitwiseOnBoolean(); // Simplify token list.. tokenizer.simplifyTokenList(); @@ -2855,6 +2857,63 @@ private: ASSERT_EQUALS("", errout.str()); } + void bitwiseOnBoolean() // 3062 + { + check("void f(_Bool a, _Bool b) {\n" + " if(a & b) {}\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Boolean variable 'a' is used in bitwise operation. Did you mean && ?\n", errout.str()); + + check("void f(_Bool a, _Bool b) {\n" + " if(a | b) {}\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Boolean variable 'a' is used in bitwise operation. Did you mean || ?\n", errout.str()); + + check("void f(bool a, bool b) {\n" + " if(a & b) {}\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Boolean variable 'a' is used in bitwise operation. Did you mean && ?\n", errout.str()); + + check("void f(bool a, bool b) {\n" + " if(a & !b) {}\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Boolean variable 'a' is used in bitwise operation. Did you mean && ?\n", errout.str()); + + check("void f(bool a, bool b) {\n" + " if(a | b) {}\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Boolean variable 'a' is used in bitwise operation. Did you mean || ?\n", errout.str()); + + check("void f(bool a, bool b) {\n" + " if(a | !b) {}\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Boolean variable 'a' is used in bitwise operation. Did you mean || ?\n", errout.str()); + + check("bool a, b;\n" + "void f() {\n" + " if(a & b) {}\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (style) Boolean variable 'a' is used in bitwise operation. Did you mean && ?\n", errout.str()); + + check("bool a, b;\n" + "void f() {\n" + " if(a & !b) {}\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (style) Boolean variable 'a' is used in bitwise operation. Did you mean && ?\n", errout.str()); + + check("bool a, b;\n" + "void f() {\n" + " if(a | b) {}\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (style) Boolean variable 'a' is used in bitwise operation. Did you mean || ?\n", errout.str()); + + check("bool a, b;\n" + "void f() {\n" + " if(a | !b) {}\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (style) Boolean variable 'a' is used in bitwise operation. Did you mean || ?\n", errout.str()); + } + void incorrectStringCompare() { check("int f() {\n"