From 0154d39bf6e36612a00f418695699ec64900f269 Mon Sep 17 00:00:00 2001 From: Boris Egorov Date: Mon, 19 Sep 2016 11:13:09 +0700 Subject: [PATCH] Show struct member in unsignedLessThanZeroError warning Before: [/tmp/test.c:8]: (style) Checking if unsigned variable '.' is less than zero. [/tmp/test.c:12]: (style) Checking if unsigned variable '.' is less than zero. After: [/tmp/test.c:8]: (style) Checking if unsigned variable 'd.n' is less than zero. [/tmp/test.c:12]: (style) Checking if unsigned variable 'd.n' is less than zero. --- lib/checkother.cpp | 4 +-- lib/token.cpp | 80 +++++++++++++++++++++++++++------------------- test/testother.cpp | 23 +++++++++++++ 3 files changed, 73 insertions(+), 34 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 586d23e94..a7ce1c753 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2157,13 +2157,13 @@ void CheckOther::checkSignOfUnsignedVariable() if (vt && vt->pointer) pointerLessThanZeroError(tok, inconclusive); if (vt && vt->sign == ValueType::UNSIGNED) - unsignedLessThanZeroError(tok, tok->astOperand1()->str(), inconclusive); + unsignedLessThanZeroError(tok, tok->astOperand1()->expressionString(), inconclusive); } else if (Token::Match(tok->previous(), "0 >|>=") && tok->previous() == tok->astOperand1()) { const ValueType* vt = tok->astOperand2()->valueType(); if (vt && vt->pointer) pointerLessThanZeroError(tok, inconclusive); if (vt && vt->sign == ValueType::UNSIGNED) - unsignedLessThanZeroError(tok, tok->astOperand2()->str(), inconclusive); + unsignedLessThanZeroError(tok, tok->astOperand2()->expressionString(), inconclusive); } else if (Token::simpleMatch(tok, ">= 0") && tok->next() == tok->astOperand2()) { const ValueType* vt = tok->astOperand1()->valueType(); if (vt && vt->pointer) diff --git a/lib/token.cpp b/lib/token.cpp index 620b2110b..ec7f1f531 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -1146,6 +1146,51 @@ bool Token::isUnaryPreOp() const return false; // <- guess } +static const Token* goToLeftParenthesis(const Token* start, const Token* end) +{ + // move start to lpar in such expression: '(*it).x' + int par = 0; + for (const Token *tok = start; tok && tok != end; tok = tok->next()) { + if (tok->str() == "(") + ++par; + else if (tok->str() == ")") { + if (par == 0) + start = tok->link(); + else + --par; + } + } + return start; +} + +static const Token* goToRightParenthesis(const Token* start, const Token* end) +{ + // move end to rpar in such expression: '2>(x+1)' + int par = 0; + for (const Token *tok = end; tok && tok != start; tok = tok->previous()) { + if (tok->str() == ")") + ++par; + else if (tok->str() == "(") { + if (par == 0) + end = tok->link(); + else + --par; + } + } + return end; +} + +static std::string stringFromTokenRange(const Token* start, const Token* end) +{ + std::string ret; + for (const Token *tok = start; tok && tok != end; tok = tok->next()) { + ret += tok->str(); + if (Token::Match(tok, "%name%|%num% %name%|%num%")) + ret += " "; + } + return ret + end->str(); +} + std::string Token::expressionString() const { const Token * const top = this; @@ -1163,39 +1208,10 @@ std::string Token::expressionString() const end = end->astOperand2() ? end->astOperand2() : end->astOperand1(); } - // move start to lpar in such expression: '(*it).x' - int par = 0; - for (const Token *tok = start; tok && tok != end; tok = tok->next()) { - if (tok->str() == "(") - ++par; - else if (tok->str() == ")") { - if (par == 0) - start = tok->link(); - else - --par; - } - } + start = goToLeftParenthesis(start, end); + end = goToRightParenthesis(start, end); - // move end to rpar in such expression: '2>(x+1)' - par = 0; - for (const Token *tok = end; tok && tok != start; tok = tok->previous()) { - if (tok->str() == ")") - ++par; - else if (tok->str() == "(") { - if (par == 0) - end = tok->link(); - else - --par; - } - } - - std::string ret; - for (const Token *tok = start; tok && tok != end; tok = tok->next()) { - ret += tok->str(); - if (Token::Match(tok, "%name%|%num% %name%|%num%")) - ret += " "; - } - return ret + end->str(); + return stringFromTokenRange(start, end); } static void astStringXml(const Token *tok, std::size_t indent, std::ostream &out) diff --git a/test/testother.cpp b/test/testother.cpp index 4a4ba5dc0..edb46f0e0 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -170,6 +170,8 @@ private: TEST_CASE(testEvaluationOrderSequencePointsFunctionCall); TEST_CASE(testEvaluationOrderSequencePointsComma); TEST_CASE(testEvaluationOrderSizeof); + + TEST_CASE(testUnsignedLessThanZero); } void check(const char code[], const char *filename = nullptr, bool experimental = false, bool inconclusive = true, bool runSimpleChecks=true, Settings* settings = 0) { @@ -6053,6 +6055,27 @@ private: "}", "test.c"); ASSERT_EQUALS("", errout.str()); } + + void testUnsignedLessThanZero() { + check("struct d {\n" + " unsigned n;\n" + "};\n" + "void f(void) {\n" + " struct d d;\n" + " d.n = 3;\n" + "\n" + " if (d.n < 0) {\n" + " return;\n" + " }\n" + "\n" + " if (0 > d.n) {\n" + " return;\n" + " }\n" + "}", "test.c"); + ASSERT_EQUALS("[test.c:8]: (style) Checking if unsigned variable 'd.n' is less than zero.\n" + "[test.c:12]: (style) Checking if unsigned variable 'd.n' is less than zero.\n", + errout.str()); + } }; REGISTER_TEST(TestOther)