diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 953a1ef82..cb95750f5 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2558,6 +2558,10 @@ void CheckOther::checkSignOfUnsignedVariable() if (!_settings->isEnabled("style")) return; + const bool inconclusive = _tokenizer->codeWithTemplates(); + if (inconclusive && !_settings->inconclusive) + return; + const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); std::list<Scope>::const_iterator scope; @@ -2572,36 +2576,54 @@ void CheckOther::checkSignOfUnsignedVariable() if (Token::Match(tok, ";|(|&&|%oror% %var% <|<= 0 ;|)|&&|%oror%") && tok->next()->varId()) { const Variable * var = symbolDatabase->getVariableFromVarId(tok->next()->varId()); if (var && var->typeEndToken()->isUnsigned()) - unsignedLessThanZeroError(tok->next(), tok->next()->str()); + unsignedLessThanZeroError(tok->next(), tok->next()->str(), inconclusive); } else if (Token::Match(tok, ";|(|&&|%oror% 0 > %var% ;|)|&&|%oror%") && tok->tokAt(3)->varId()) { const Variable * var = symbolDatabase->getVariableFromVarId(tok->tokAt(3)->varId()); if (var && var->typeEndToken()->isUnsigned()) - unsignedLessThanZeroError(tok->tokAt(3), tok->strAt(3)); + unsignedLessThanZeroError(tok->tokAt(3), tok->strAt(3), inconclusive); } else if (Token::Match(tok, ";|(|&&|%oror% 0 <= %var% ;|)|&&|%oror%") && tok->tokAt(3)->varId()) { const Variable * var = symbolDatabase->getVariableFromVarId(tok->tokAt(3)->varId()); if (var && var->typeEndToken()->isUnsigned()) - unsignedPositiveError(tok->tokAt(3), tok->strAt(3)); + unsignedPositiveError(tok->tokAt(3), tok->strAt(3), inconclusive); } else if (Token::Match(tok, ";|(|&&|%oror% %var% >= 0 ;|)|&&|%oror%") && tok->next()->varId()) { const Variable * var = symbolDatabase->getVariableFromVarId(tok->next()->varId()); if (var && var->typeEndToken()->isUnsigned()) - unsignedPositiveError(tok->next(), tok->next()->str()); + unsignedPositiveError(tok->next(), tok->next()->str(), inconclusive); } } } } -void CheckOther::unsignedLessThanZeroError(const Token *tok, const std::string &varname) +void CheckOther::unsignedLessThanZeroError(const Token *tok, const std::string &varname, bool inconclusive) { - reportError(tok, Severity::style, "unsignedLessThanZero", - "Checking if unsigned variable '" + varname + "' is less than zero.\n" - "An unsigned variable will never be negative so it is either pointless or " - "an error to check if it is."); + if (inconclusive) { + reportInconclusiveError(tok, Severity::style, "unsignedLessThanZero", + "Checking if unsigned variable '" + varname + "' is less than zero. This might be a false warning.\n" + "Checking if unsigned variable '" + varname + "' is less than zero. An unsigned " + "variable will never be negative so it is either pointless or an error to check if it is. " + "It's not known if the used constant is a template parameter or not and therefore " + "this message might be a false warning"); + } else { + reportError(tok, Severity::style, "unsignedLessThanZero", + "Checking if unsigned variable '" + varname + "' is less than zero.\n" + "An unsigned variable will never be negative so it is either pointless or " + "an error to check if it is."); + } } -void CheckOther::unsignedPositiveError(const Token *tok, const std::string &varname) +void CheckOther::unsignedPositiveError(const Token *tok, const std::string &varname, bool inconclusive) { - reportError(tok, Severity::style, "unsignedPositive", - "Checking if unsigned variable '" + varname + "' is positive is always true.\n" - "An unsigned variable will always be positive so it is either pointless or " - "an error to check if it is."); + if (inconclusive) { + reportInconclusiveError(tok, Severity::style, "unsignedPositive", + "Checking if unsigned variable '" + varname + "' is positive is always true. This might be a false warning.\n" + "Checking if unsigned variable '" + varname + "' is positive is always true. " + "An unsigned variable will always be positive so it is either pointless or " + "an error to check if it is. It's not known if the used constant is a " + "template parameter or not and therefore this message might be a false warning"); + } else { + reportError(tok, Severity::style, "unsignedPositive", + "Checking if unsigned variable '" + varname + "' is positive is always true.\n" + "An unsigned variable will always be positive so it is either pointless or " + "an error to check if it is."); + } } diff --git a/lib/checkother.h b/lib/checkother.h index 5858a3549..11b99f106 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -289,8 +289,8 @@ public: void alwaysTrueStringVariableCompareError(const Token *tok, const std::string& str1, const std::string& str2); void duplicateBreakError(const Token *tok); void assignBoolToPointerError(const Token *tok); - void unsignedLessThanZeroError(const Token *tok, const std::string &varname); - void unsignedPositiveError(const Token *tok, const std::string &varname); + void unsignedLessThanZeroError(const Token *tok, const std::string &varname, bool inconclusive); + void unsignedPositiveError(const Token *tok, const std::string &varname, bool inconclusive); void bitwiseOnBooleanError(const Token *tok, const std::string &varname, const std::string &op); void comparisonOfBoolExpressionWithIntError(const Token *tok); void SuspiciousSemicolonError(const Token *tok); @@ -343,8 +343,8 @@ public: c.alwaysTrueFalseStringCompareError(0, "str1", "str2"); c.alwaysTrueStringVariableCompareError(0, "varname1", "varname2"); c.duplicateBreakError(0); - c.unsignedLessThanZeroError(0, "varname"); - c.unsignedPositiveError(0, "varname"); + c.unsignedLessThanZeroError(0, "varname", false); + c.unsignedPositiveError(0, "varname", false); c.bitwiseOnBooleanError(0, "varname", "&&"); c.comparisonOfBoolExpressionWithIntError(0); c.SuspiciousSemicolonError(0); diff --git a/test/testother.cpp b/test/testother.cpp index 5fe2f708c..99337c828 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -3527,12 +3527,13 @@ private: ASSERT_EQUALS("[test.cpp:4]: (warning) Passing sizeof(pointer) as the last argument to strncmp.\n", errout.str()); } - void check_signOfUnsignedVariable(const char code[]) { + void check_signOfUnsignedVariable(const char code[], bool inconclusive=false) { // Clear the error buffer.. errout.str(""); Settings settings; settings.addEnabled("style"); + settings.inconclusive = inconclusive; // Tokenize.. Tokenizer tokenizer(&settings, this); @@ -3753,6 +3754,19 @@ private: " return false;\n" "}"); ASSERT_EQUALS("", errout.str()); + + // #3233 - FP when template is used (template parameter is numeric constant) + { + const char code[] = + "template<int n> void foo(unsigned int x) {\n" + " if (x <= n);\n" + "}\n" + "foo<0>();"; + check_signOfUnsignedVariable(code, false); + ASSERT_EQUALS("", errout.str()); + check_signOfUnsignedVariable(code, true); + ASSERT_EQUALS("[test.cpp:2]: (style) Checking if unsigned variable 'x' is less than zero. This might be a false warning.\n", errout.str()); + } } void checkForSuspiciousSemicolon1() {