diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 94307ab276f..274106fe248 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -2188,7 +2188,7 @@ T* getTokenArgumentFunctionImpl(T* tok, int& argn) argn = -1; { T* parent = tok->astParent(); - if (parent && parent->isUnaryOp("&")) + if (parent && (parent->isUnaryOp("&") || parent->isIncDecOp())) parent = parent->astParent(); while (parent && parent->isCast()) parent = parent->astParent(); @@ -2196,7 +2196,7 @@ T* getTokenArgumentFunctionImpl(T* tok, int& argn) parent = parent->astParent(); // passing variable to subfunction? - if (Token::Match(parent, "[[(,{]")) + if (Token::Match(parent, "[*[(,{]")) ; else if (Token::simpleMatch(parent, ":")) { while (Token::Match(parent, "[?:]")) @@ -2455,7 +2455,7 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings, (Token::simpleMatch(tok2->astParent(), ":") && Token::simpleMatch(tok2->astParent()->astParent(), "?"))) tok2 = tok2->astParent(); - if (tok2->astParent() && tok2->astParent()->tokType() == Token::eIncDecOp) + if (indirect == 0 && tok2->astParent() && tok2->astParent()->tokType() == Token::eIncDecOp) return true; auto skipRedundantPtrOp = [](const Token* tok, const Token* parent) { @@ -2576,9 +2576,7 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings, } const Token *parent = tok2->astParent(); - while (Token::Match(parent, ".|::")) - parent = parent->astParent(); - if (parent && parent->tokType() == Token::eIncDecOp) + if (parent && parent->tokType() == Token::eIncDecOp && (indirect == 0 || tok2 != tok)) return true; // structured binding, nonconst reference variable in lhs @@ -2615,7 +2613,7 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings, if (indirect > 0) { // check for `*(ptr + 1) = new_value` case parent = tok2->astParent(); - while (parent && parent->isArithmeticalOp() && parent->isBinaryOp()) { + while (parent && ((parent->isArithmeticalOp() && parent->isBinaryOp()) || parent->isIncDecOp())) { parent = parent->astParent(); } if (Token::simpleMatch(parent, "*")) { diff --git a/lib/checkother.cpp b/lib/checkother.cpp index a0d2677ad58..0970ab313d9 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1587,9 +1587,10 @@ void CheckOther::checkConstPointer() if (std::find(nonConstPointers.cbegin(), nonConstPointers.cend(), var) != nonConstPointers.cend()) continue; pointers.emplace_back(var); - const Token* const parent = tok->astParent(); + const Token* parent = tok->astParent(); enum Deref { NONE, DEREF, MEMBER } deref = NONE; - if (parent && parent->isUnaryOp("*")) + bool hasIncDec = false; + if (parent && (parent->isUnaryOp("*") || (hasIncDec = parent->isIncDecOp() && parent->astParent() && parent->astParent()->isUnaryOp("*")))) deref = DEREF; else if (Token::simpleMatch(parent, "[") && parent->astOperand1() == tok && tok != nameTok) deref = DEREF; @@ -1600,7 +1601,7 @@ void CheckOther::checkConstPointer() else if (astIsRangeBasedForDecl(tok)) continue; if (deref != NONE) { - const Token* const gparent = parent->astParent(); + const Token* gparent = parent->astParent(); if (deref == MEMBER) { if (!gparent) continue; @@ -1613,6 +1614,10 @@ void CheckOther::checkConstPointer() } if (Token::Match(gparent, "%cop%") && !gparent->isUnaryOp("&") && !gparent->isUnaryOp("*")) continue; + if (hasIncDec) { + parent = gparent; + gparent = gparent ? gparent->astParent() : nullptr; + } int argn = -1; if (Token::simpleMatch(gparent, "return")) { const Function* function = gparent->scope()->function; @@ -1633,7 +1638,7 @@ void CheckOther::checkConstPointer() continue; else if (const Token* ftok = getTokenArgumentFunction(parent, argn)) { bool inconclusive{}; - if (!isVariableChangedByFunctionCall(ftok, vt->pointer, var->declarationId(), mSettings, &inconclusive) && !inconclusive) + if (!isVariableChangedByFunctionCall(ftok->next(), vt->pointer, var->declarationId(), mSettings, &inconclusive) && !inconclusive) continue; } } else { diff --git a/lib/checkunusedvar.cpp b/lib/checkunusedvar.cpp index bd42a61824c..9f0ea5dc35c 100644 --- a/lib/checkunusedvar.cpp +++ b/lib/checkunusedvar.cpp @@ -1691,8 +1691,8 @@ bool CheckUnusedVar::isFunctionWithoutSideEffects(const Function& func, const To } // check if global variable is changed if (bodyVariable->isGlobal() || (pointersToGlobals.find(bodyVariable) != pointersToGlobals.end())) { - const int depth = 20; - if (isVariableChanged(bodyToken, depth, mSettings, mTokenizer->isCPP())) { + const int indirect = bodyVariable->isArray() ? bodyVariable->dimensions().size() : bodyVariable->isPointer(); + if (isVariableChanged(bodyToken, indirect, mSettings, mTokenizer->isCPP())) { return false; } // check if pointer to global variable assigned to another variable (another_var = &global_var) diff --git a/test/testother.cpp b/test/testother.cpp index 8c54d541feb..623daae545f 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -8120,7 +8120,7 @@ class TestOther : public TestFixture { "void packed_object_info(struct object_info *oi) {\n" " if (*oi->typep < 0);\n" "}"); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'oi' can be declared as pointer to const\n", errout.str()); } void checkSuspiciousSemicolon1() { diff --git a/test/teststl.cpp b/test/teststl.cpp index b25a4443108..64acd7027c6 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -2819,27 +2819,20 @@ class TestStl : public TestFixture { } void eraseOnVector() { - check("void f(const std::vector& m_ImplementationMap) {\n" - " std::vector::iterator aIt = m_ImplementationMap.begin();\n" - " m_ImplementationMap.erase(something(unknown));\n" // All iterators become invalidated when erasing from std::vector - " m_ImplementationMap.erase(aIt);\n" - "}"); - ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:4]: (error) Using iterator to local container 'm_ImplementationMap' that may be invalid.\n", errout.str()); - - check("void f(const std::vector& m_ImplementationMap) {\n" - " std::vector::iterator aIt = m_ImplementationMap.begin();\n" - " m_ImplementationMap.erase(*aIt);\n" // All iterators become invalidated when erasing from std::vector - " m_ImplementationMap.erase(aIt);\n" + check("void f(std::vector& v) {\n" + " std::vector::iterator aIt = v.begin();\n" + " v.erase(something(unknown));\n" // All iterators become invalidated when erasing from std::vector + " v.erase(aIt);\n" "}"); - ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:4]: (error) Using iterator to local container 'm_ImplementationMap' that may be invalid.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:4]: (error) Using iterator to local container 'v' that may be invalid.\n", errout.str()); - check("void f(const std::vector& m_ImplementationMap) {\n" - " std::vector::iterator aIt = m_ImplementationMap.begin();\n" - " std::vector::iterator bIt = m_ImplementationMap.begin();\n" - " m_ImplementationMap.erase(*bIt);\n" // All iterators become invalidated when erasing from std::vector - " aIt = m_ImplementationMap.erase(aIt);\n" + check("void f(std::vector& v) {\n" + " std::vector::iterator aIt = v.begin();\n" + " std::vector::iterator bIt = v.begin();\n" + " v.erase(bIt);\n" // All iterators become invalidated when erasing from std::vector + " aIt = v.erase(aIt);\n" "}"); - ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:2] -> [test.cpp:4] -> [test.cpp:1] -> [test.cpp:5]: (error) Using iterator to local container 'm_ImplementationMap' that may be invalid.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:2] -> [test.cpp:4] -> [test.cpp:1] -> [test.cpp:5]: (error) Using iterator to local container 'v' that may be invalid.\n", errout.str()); } void pushback1() { @@ -3040,7 +3033,8 @@ class TestStl : public TestFixture { " }\n" "}"); ASSERT_EQUALS( - "[test.cpp:4] -> [test.cpp:6] -> [test.cpp:3] -> [test.cpp:9]: (error, inconclusive) Using iterator to local container 'vec' that may be invalid.\n", + "[test.cpp:4] -> [test.cpp:6] -> [test.cpp:3] -> [test.cpp:9]: (error, inconclusive) Using iterator to local container 'vec' that may be invalid.\n" + "[test.cpp:4] -> [test.cpp:9]: (warning) Either the condition 'it!=vec.end()' is redundant or there is possible dereference of an invalid iterator: it.\n", errout.str()); }