Merge branch 'memset-improvements'

lib/checkclass.cpp

@@ -693,6 +693,103 @@ void CheckClass::unusedPrivateFunctionError(const Token *tok, const std::string
 // ClassCheck: Check that memset is not used on classes
 //---------------------------------------------------------------------------
 
+void CheckClass::checkMemsetType(const Token *tok, const std::string &type)
+{
+    // check for cached message for this type
+    std::map<std::string, std::string>::const_iterator msg = _memsetClassMessages.find(type);
+    if (msg != _memsetClassMessages.end())
+    {
+        memsetError(tok, type, msg->second);
+        return;
+    }
+
+    // Warn if type is a class or struct that contains any std::* variables
+    const std::string pattern2(std::string("struct|class ") + type + " :|{");
+    const Token *tstruct = Token::findmatch(_tokenizer->tokens(), pattern2.c_str());
+
+    if (!tstruct)
+        return;
+
+    // typeKind is either 'struct' or 'class'
+    const std::string &typeKind = tstruct->str();
+
+    if (tstruct->tokAt(2)->str() == ":")
+    {
+        tstruct = tstruct->tokAt(3);
+        for (; tstruct; tstruct = tstruct->next())
+        {
+            while (Token::Match(tstruct, "public|private|protected|virtual"))
+            {
+                tstruct = tstruct->next();
+            }
+
+            // recursively check all parent classes
+            checkMemsetType(tok, tstruct->str());
+
+            tstruct = tstruct->next();
+            if (tstruct->str() != ",")
+                break;
+        }
+    }
+
+    for (; tstruct; tstruct = tstruct->next())
+    {
+        if (tstruct->str() == "}")
+            break;
+
+        // struct with function? skip function body..
+        if (Token::simpleMatch(tstruct, ") {"))
+        {
+            tstruct = tstruct->next()->link();
+            if (!tstruct)
+                break;
+        }
+
+        // before a statement there must be either:
+        // * private:|protected:|public:
+        // * { } ;
+        if (Token::Match(tstruct, "[;{}]") ||
+            tstruct->str().find(":") != std::string::npos)
+        {
+            if (Token::Match(tstruct->next(), "std :: %type% %var% ;"))
+                memsetError(tok, type, tok->str(), "'std::" + tstruct->strAt(3) + "'", typeKind);
+
+            else if (Token::Match(tstruct->next(), "std :: %type% <"))
+            {
+                // backup the type
+                const std::string typestr(tstruct->strAt(3));
+
+                // check if it's a pointer variable..
+                unsigned int level = 0;
+                while (0 != (tstruct = tstruct->next()))
+                {
+                    if (tstruct->str() == "<")
+                        ++level;
+                    else if (tstruct->str() == ">")
+                    {
+                        if (level <= 1)
+                            break;
+                        --level;
+                    }
+                    else if (tstruct->str() == "(")
+                        tstruct = tstruct->link();
+                }
+
+                if (!tstruct)
+                    break;
+
+                // found error => report
+                if (Token::Match(tstruct, "> %var% ;"))
+                    memsetError(tok, type, tok->str(), "'std::" + typestr + "'", typeKind);
+            }
+            else if (Token::simpleMatch(tstruct->next(), "virtual"))
+                memsetError(tok, type, tok->str(), "virtual method", typeKind);
+            else if (!Token::Match(tstruct->next(), "static|}"))
+                checkMemsetType(tok, tstruct->next()->str());
+        }
+    }
+}
+
 void CheckClass::noMemset()
 {
     createSymbolDatabase();
@@ -726,73 +823,20 @@ void CheckClass::noMemset()
         if (type.empty())
             continue;
 
-        // Warn if type is a class or struct that contains any std::* variables
+        checkMemsetType(tok, type);
-        const std::string pattern2(std::string("struct|class ") + type + " {");
-        const Token *tstruct = Token::findmatch(_tokenizer->tokens(), pattern2.c_str());
-
-        if (!tstruct)
-            continue;
-
-        const std::string &typeName = tstruct->str();
-
-        for (; tstruct; tstruct = tstruct->next())
-        {
-            if (tstruct->str() == "}")
-                break;
-
-            // struct with function? skip function body..
-            if (Token::simpleMatch(tstruct, ") {"))
-            {
-                tstruct = tstruct->next()->link();
-                if (!tstruct)
-                    break;
-            }
-
-            // before a statement there must be either:
-            // * private:|protected:|public:
-            // * { } ;
-            if (Token::Match(tstruct, "[;{}]") ||
-                tstruct->str().find(":") != std::string::npos)
-            {
-                if (Token::Match(tstruct->next(), "std :: %type% %var% ;"))
-                    memsetError(tok, tok->str(), tstruct->strAt(3), typeName);
-
-                else if (Token::Match(tstruct->next(), "std :: %type% <"))
-                {
-                    // backup the type
-                    const std::string typestr(tstruct->strAt(3));
-
-                    // check if it's a pointer variable..
-                    unsigned int level = 0;
-                    while (0 != (tstruct = tstruct->next()))
-                    {
-                        if (tstruct->str() == "<")
-                            ++level;
-                        else if (tstruct->str() == ">")
-                        {
-                            if (level <= 1)
-                                break;
-                            --level;
-                        }
-                        else if (tstruct->str() == "(")
-                            tstruct = tstruct->link();
-                    }
-
-                    if (!tstruct)
-                        break;
-
-                    // found error => report
-                    if (Token::Match(tstruct, "> %var% ;"))
-                        memsetError(tok, tok->str(), typestr, typeName);
-                }
-            }
-        }
     }
 }
 
-void CheckClass::memsetError(const Token *tok, const std::string &memfunc, const std::string &classname, const std::string &type)
+void CheckClass::memsetError(const Token *tok, const std::string &type, const std::string &message)
 {
-    reportError(tok, Severity::error, "memsetClass", "Using '" + memfunc + "' on " + type + " that contains a 'std::" + classname + "'");
+    reportError(tok, Severity::error, "memsetClass", message);
+    // cache the message for this type so we don't have to look it up again
+    _memsetClassMessages[type] = message;
+}
+
+void CheckClass::memsetError(const Token *tok, const std::string &type, const std::string &memfunc, const std::string &classname, const std::string &typekind)
+{
+    memsetError(tok, type, "Using '" + memfunc + "' on " + typekind + " that contains a " + classname);
 }
 
 //---------------------------------------------------------------------------

lib/checkclass.h

@@ -117,7 +117,8 @@ private:
     void uninitVarError(const Token *tok, const std::string &classname, const std::string &varname);
     void operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname);
     void unusedPrivateFunctionError(const Token *tok, const std::string &classname, const std::string &funcname);
-    void memsetError(const Token *tok, const std::string &memfunc, const std::string &classname, const std::string &type);
+    void memsetError(const Token *tok, const std::string &type, const std::string &message);
+    void memsetError(const Token *tok, const std::string &type, const std::string &memfunc, const std::string &classname, const std::string &typekind);
     void operatorEqReturnError(const Token *tok);
     void virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived);
     void thisSubtractionError(const Token *tok);
@@ -133,7 +134,7 @@ private:
         c.uninitVarError(0, "classname", "varname");
         c.operatorEqVarError(0, "classname", "");
         c.unusedPrivateFunctionError(0, "classname", "funcname");
-        c.memsetError(0, "memfunc", "classname", "class");
+        c.memsetError(0, "type", "memfunc", "classname", "class");
         c.operatorEqReturnError(0);
         //c.virtualDestructorError(0, "Base", "Derived");
         c.thisSubtractionError(0);
@@ -227,6 +228,10 @@ private:
     void initializeVarList(const Function &func, std::list<std::string> &callstack, const Scope *scope, std::vector<Usage> &usage);
 
     bool canNotCopy(const Scope *scope) const;
+
+    // noMemset helpers
+    void checkMemsetType(const Token *tok, const std::string &type);
+    std::map<std::string, std::string> _memsetClassMessages;
 };
 /// @}
 //---------------------------------------------------------------------------

test/testclass.cpp

@@ -2949,6 +2949,56 @@ private:
                       "    memset(&fred, 0, sizeof(fred));\n"
                       "}\n");
         ASSERT_EQUALS("[test.cpp:8]: (error) Using 'memset' on class that contains a 'std::string'\n", errout.str());
+
+        checkNoMemset("class Fred\n"
+                      "{\n"
+                      "    std::string s;\n"
+                      "};\n"
+                      "class Pebbles: public Fred {};\n"
+                      "void f()\n"
+                      "{\n"
+                      "    Pebbles pebbles;\n"
+                      "    memset(&pebbles, 0, sizeof(pebbles));\n"
+                      "}\n");
+        ASSERT_EQUALS("[test.cpp:9]: (error) Using 'memset' on class that contains a 'std::string'\n", errout.str());
+
+        checkNoMemset("struct Stringy {\n"
+                      "    std::string inner;\n"
+                      "};\n"
+                      "struct Foo {\n"
+                      "    Stringy s;\n"
+                      "};\n"
+                      "int main() {\n"
+                      "    Foo foo;\n"
+                      "    memset(&foo, 0, sizeof(Foo));\n"
+                      "}\n");
+        ASSERT_EQUALS("[test.cpp:9]: (error) Using 'memset' on struct that contains a 'std::string'\n", errout.str());
+
+        checkNoMemset("class Fred\n"
+                      "{\n"
+                      "    virtual ~Fred();\n"
+                      "};\n"
+                      "void f()\n"
+                      "{\n"
+                      "    Fred fred;\n"
+                      "    memset(&fred, 0, sizeof(fred));\n"
+                      "}\n");
+        ASSERT_EQUALS("[test.cpp:8]: (error) Using 'memset' on class that contains a virtual method\n", errout.str());
+
+        checkNoMemset("class Fred\n"
+                      "{\n"
+                      "};\n"
+                      "class Wilma\n"
+                      "{\n"
+                      "    virtual ~Wilma();\n"
+                      "};\n"
+                      "class Pebbles: public Fred, Wilma {};\n"
+                      "void f()\n"
+                      "{\n"
+                      "    Pebbles pebbles;\n"
+                      "    memset(&pebbles, 0, sizeof(pebbles));\n"
+                      "}\n");
+        ASSERT_EQUALS("[test.cpp:12]: (error) Using 'memset' on class that contains a virtual method\n", errout.str());
     }
 
     void memsetOnStruct()