Fixed #3800 (False negative: Self-assignement of variable declared as 'extern')

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;'

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();
     }
 

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

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() {