From 8aba8013601f0f1b99d7e7dfe569ef7c3e32af23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Fri, 22 Jun 2012 11:23:50 +0200 Subject: [PATCH] Fixed #3800 (False negative: Self-assignement of variable declared as 'extern') --- lib/checkother.cpp | 7 +++---- lib/symboldatabase.cpp | 22 +++++----------------- lib/symboldatabase.h | 21 +++++++++++++++------ test/testother.cpp | 9 ++++++++- 4 files changed, 31 insertions(+), 28 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 1ae42fe2f..644bcd769 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -866,10 +866,9 @@ void CheckOther::switchCaseFallThrough(const Token *tok) // // int y = y; // <- redundant initialization to self //--------------------------------------------------------------------------- -static bool isPOD(const Variable* var) +static bool isPOD(const Tokenizer *tokenizer, const Variable* var) { - // TODO: Implement real support for POD definition - return(var && (var->isPointer() || var->nameToken()->previous()->isStandardType())); + return ((var && (!var->isClass() || var->isPointer())) || !tokenizer->isCPP()); } void CheckOther::checkSelfAssignment() @@ -884,7 +883,7 @@ void CheckOther::checkSelfAssignment() while (tok) { if (Token::Match(tok->previous(), "[;{}]") && tok->varId() && tok->varId() == tok->tokAt(2)->varId() && - isPOD(symbolDatabase->getVariableFromVarId(tok->varId()))) { + isPOD(_tokenizer, symbolDatabase->getVariableFromVarId(tok->varId()))) { bool err = true; // no false positive for 'x = x ? x : 1;' diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index ba90f23fe..81260f0bf 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -841,6 +841,8 @@ void Variable::evaluate() for (const Token* const end = _name?_name:_end; tok != end;) { if (tok->str() == "static") setFlag(fIsStatic, true); + else if (tok->str() == "extern") + setFlag(fIsExtern, true); else if (tok->str() == "mutable") setFlag(fIsMutable, true); else if (tok->str() == "const") @@ -1335,6 +1337,7 @@ void SymbolDatabase::printVariable(const Variable *var, const char *indent) cons std::cout << indent << "_flags: " << std::endl; std::cout << indent << " isMutable: " << (var->isMutable() ? "true" : "false") << std::endl; std::cout << indent << " isStatic: " << (var->isStatic() ? "true" : "false") << std::endl; + std::cout << indent << " isExtern: " << (var->isExtern() ? "true" : "false") << std::endl; std::cout << indent << " isConst: " << (var->isConst() ? "true" : "false") << std::endl; std::cout << indent << " isClass: " << (var->isClass() ? "true" : "false") << std::endl; std::cout << indent << " isArray: " << (var->isArray() ? "true" : "false") << std::endl; @@ -1914,23 +1917,8 @@ const Token *Scope::checkVariable(const Token *tok, AccessControl varaccess) return tok->linkAt(4); } - // Is it const..? - if (tok->str() == "const") { - tok = tok->next(); - } - - // Is it a static variable? - if (tok->str() == "static") { - tok = tok->next(); - } - - // Is it a mutable variable? - if (tok->str() == "mutable") { - tok = tok->next(); - } - - // Is it const..? - if (tok->str() == "const") { + // skip const|static|mutable|extern + while (Token::Match(tok, "const|static|mutable|extern")) { tok = tok->next(); } diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index bb5891c6f..b3fe7dce0 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -60,11 +60,12 @@ class CPPCHECKLIB Variable { fIsMutable = (1 << 0), /** @brief mutable variable */ fIsStatic = (1 << 1), /** @brief static variable */ fIsConst = (1 << 2), /** @brief const variable */ - fIsClass = (1 << 3), /** @brief user defined type */ - fIsArray = (1 << 4), /** @brief array variable */ - fIsPointer = (1 << 5), /** @brief pointer variable */ - fIsReference = (1 << 6), /** @brief reference variable */ - fHasDefault = (1 << 7) /** @brief function argument with default value */ + fIsExtern = (1 << 3), /** @brief extern variable */ + fIsClass = (1 << 4), /** @brief user defined type */ + fIsArray = (1 << 5), /** @brief array variable */ + fIsPointer = (1 << 6), /** @brief pointer variable */ + fIsReference = (1 << 7), /** @brief reference variable */ + fHasDefault = (1 << 8) /** @brief function argument with default value */ }; /** @@ -219,7 +220,7 @@ public: * @return true if local, false if not */ bool isLocal() const { - return _access == Local; + return (_access == Local) && !isExtern(); } /** @@ -238,6 +239,14 @@ public: return getFlag(fIsStatic); } + /** + * Is variable extern. + * @return true if extern, false if not + */ + bool isExtern() const { + return getFlag(fIsExtern); + } + /** * Is variable const. * @return true if const, false if not diff --git a/test/testother.cpp b/test/testother.cpp index 8894a7382..00e913929 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -2310,7 +2310,7 @@ private: "}"); ASSERT_EQUALS("[test.cpp:3]: (warning) Redundant assignment of \"x\" to itself\n", errout.str()); - // non-primitive type -> there might be some side effects + // #2502 - non-primitive type -> there might be some side effects check("void foo()\n" "{\n" " Fred fred; fred = fred;\n" @@ -2334,6 +2334,13 @@ private: " x = x ? x : 0;\n" "}"); ASSERT_EQUALS("", errout.str()); + + // #3800 - false negative when variable is extern + check("extern int i;\n" + "void f() {\n" + " i = i;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Redundant assignment of \"i\" to itself\n", errout.str()); } void trac1132() {