Fixed #2172 (False positive: struct is not initialized in constructor)

lib/checkclass.cpp

@@ -122,16 +122,25 @@ void CheckClass::constructors()
             std::list<SymbolDatabase::Var>::const_iterator var;
             for (var = info->varlist.begin(); var != info->varlist.end(); ++var)
             {
-                // skip classes for regular constructor
-                if (var->isClass && func->type == SymbolDatabase::Func::Constructor)
-                    continue;
-
                 if (var->assign || var->init || var->isStatic)
                     continue;
 
                 if (var->isConst && var->token->previous()->str() != "*")
                     continue;
 
+                // Check if this is a class constructor
+                if (var->isClass && func->type == SymbolDatabase::Func::Constructor)
+                {
+                    // Unknown type so assume it is initialized
+                    if (!var->type)
+                        continue;
+
+                    // Known type that doesn't need initialization or
+                    // known type that has member variables of an unknown type
+                    else if (var->type->needInitialization != SymbolDatabase::SpaceInfo::True)
+                        continue;
+                }
+
                 // It's non-static and it's not initialized => error
                 if (func->type == SymbolDatabase::Func::OperatorEqual)
                 {

lib/symboldatabase.cpp

@@ -310,12 +310,13 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
 
     std::list<SpaceInfo *>::iterator it;
 
+    // fill in base class info
     for (it = spaceInfoList.begin(); it != spaceInfoList.end(); ++it)
     {
         info = *it;
 
         // skip namespaces and functions
-        if (info->type == SpaceInfo::Namespace || info->type == SpaceInfo::Function)
+        if (!info->isClassOrStruct())
             continue;
 
         // finish filling in base class info
@@ -346,12 +347,130 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
                 }
             }
         }
+    }
 
+    // fill in variable info
+    for (it = spaceInfoList.begin(); it != spaceInfoList.end(); ++it)
+    {
+        info = *it;
+
+        // skip functions
+        if (info->type != SpaceInfo::Function)
+        {
             // find variables
             info->getVarList();
         }
     }
 
+    // determine if user defined type needs initialization
+    unsigned int unknowns = 0; // stop checking when there are no unknowns
+    unsigned int retry = 0;    // bail if we don't resolve all the variable types for some reason
+
+    do
+    {
+        unknowns = 0;
+
+        for (it = spaceInfoList.begin(); it != spaceInfoList.end(); ++it)
+        {
+            info = *it;
+
+            if (info->isClassOrStruct() && info->needInitialization == SpaceInfo::Unknown)
+            {
+                // check for default constructor
+                bool hasDefaultConstructor = false;
+
+                std::list<SymbolDatabase::Func>::const_iterator func;
+
+                for (func = info->functionList.begin(); func != info->functionList.end(); ++func)
+                {
+                    if (func->type == SymbolDatabase::Func::Constructor)
+                    {
+                        // check for no arguments: func ( )
+                        /** @todo check for arguents with default values someday */
+                        if (func->argDef->next() == func->argDef->link())
+                        {
+                            hasDefaultConstructor = true;
+                            break;
+                        }
+                    }
+                }
+
+                // User defined types with user defined defaut constructor doesn't need initialization.
+                // We assume the default constructor initializes everything.
+                // Another check will figure out if the constructor actually initializes everything.
+                if (hasDefaultConstructor)
+                    info->needInitialization = SpaceInfo::False;
+
+                // check each member variable to see if it needs initialization
+                else
+                {
+                    bool needInitialization = false;
+                    bool unknown = false;
+
+                    std::list<Var>::const_iterator var;
+                    for (var = info->varlist.begin(); var != info->varlist.end(); ++var)
+                    {
+                        if (var->isClass)
+                        {
+                            if (var->type)
+                            {
+                                // does this type need initialization?
+                                if (var->type->needInitialization == SpaceInfo::True)
+                                    needInitialization = true;
+                                else if (var->type->needInitialization == SpaceInfo::Unknown)
+                                    unknown = true;
+                            }
+                        }
+                        else
+                            needInitialization = true;
+                    }
+
+                    if (!unknown)
+                    {
+                        if (needInitialization)
+                            info->needInitialization = SpaceInfo::True;
+                        else
+                            info->needInitialization = SpaceInfo::False;
+                    }
+
+                    if (info->needInitialization == SpaceInfo::Unknown)
+                        unknowns++;
+                }
+            }
+        }
+
+        retry++;
+    }
+    while (unknowns && retry < 100);
+
+    // this shouldn't happen so output a debug warning
+    if (retry == 100 && _settings->debugwarnings)
+    {
+        for (it = spaceInfoList.begin(); it != spaceInfoList.end(); ++it)
+        {
+            info = *it;
+
+            if (info->isClassOrStruct() && info->needInitialization == SpaceInfo::Unknown)
+            {
+                std::list<ErrorLogger::ErrorMessage::FileLocation> locationList;
+                ErrorLogger::ErrorMessage::FileLocation loc;
+                loc.line = info->classDef->linenr();
+                loc.setfile(_tokenizer->file(info->classDef));
+                locationList.push_back(loc);
+
+                const ErrorLogger::ErrorMessage errmsg(locationList,
+                                                       Severity::debug,
+                                                       "SymbolDatabase::SymbolDatabase couldn't resolve all user defined types.",
+                                                       "debug");
+                if (_errorLogger)
+                    _errorLogger->reportErr(errmsg);
+                else
+                    Check::reportError(errmsg);
+            }
+        }
+    }
+}
+
 SymbolDatabase::~SymbolDatabase()
 {
     std::list<SpaceInfo *>::iterator it;
@@ -745,7 +864,8 @@ SymbolDatabase::SpaceInfo::SpaceInfo(SymbolDatabase *check_, const Token *classD
     classStart(NULL),
     classEnd(NULL),
     nestedIn(nestedIn_),
-    numConstructors(0)
+    numConstructors(0),
+    needInitialization(SpaceInfo::Unknown)
 {
     if (!classDef)
     {
@@ -874,9 +994,12 @@ void SymbolDatabase::SpaceInfo::getVarList()
         // Search for start of statement..
         else if (!tok->previous() || !Token::Match(tok->previous(), ";|{|}|public:|protected:|private:"))
             continue;
+        else if (Token::Match(tok, ";|{|}"))
+            continue;
 
         // This is the start of a statement
-        const Token *vartok = 0;
+        const Token *vartok = NULL;
+        const Token *typetok = NULL;
 
         // Is it const..?
         bool isConst = false;
@@ -913,7 +1036,10 @@ void SymbolDatabase::SpaceInfo::getVarList()
         if (Token::Match(tok, "%type% %var% ;|:"))
         {
             if (!tok->isStandardType())
+            {
                 isClass = true;
+                typetok = tok;
+            }
 
             vartok = tok->next();
             tok = vartok->next();
@@ -922,31 +1048,37 @@ void SymbolDatabase::SpaceInfo::getVarList()
         {
             isClass = true;
             vartok = tok->tokAt(3);
+            typetok = vartok->previous();
             tok = vartok->next();
         }
         else if (Token::Match(tok, ":: %type% :: %type% %var% ;"))
         {
             isClass = true;
             vartok = tok->tokAt(4);
+            typetok = vartok->previous();
             tok = vartok->next();
         }
         else if (Token::Match(tok, "%type% :: %type% :: %type% %var% ;"))
         {
             isClass = true;
             vartok = tok->tokAt(5);
+            typetok = vartok->previous();
             tok = vartok->next();
         }
         else if (Token::Match(tok, ":: %type% :: %type% :: %type% %var% ;"))
         {
             isClass = true;
             vartok = tok->tokAt(6);
+            typetok = vartok->previous();
             tok = vartok->next();
         }
 
         // Structure?
         else if (Token::Match(tok, "struct|union %type% %var% ;"))
         {
+            isClass = true;
             vartok = tok->tokAt(2);
+            typetok = vartok->previous();
             tok = vartok->next();
         }
 
@@ -976,7 +1108,10 @@ void SymbolDatabase::SpaceInfo::getVarList()
         else if (Token::Match(tok, "%type% %var% [") && tok->next()->str() != "operator")
         {
             if (!tok->isStandardType())
+            {
                 isClass = true;
+                typetok = tok;
+            }
 
             vartok = tok->next();
             tok = vartok->next()->link()->next();
@@ -1013,10 +1148,15 @@ void SymbolDatabase::SpaceInfo::getVarList()
 
             // find matching ">"
             int level = 0;
+            const Token *tok1 = NULL;
             for (; tok; tok = tok->next())
             {
                 if (tok->str() == "<")
+                {
+                    if (level == 0)
+                        tok1 = tok->previous();
                     level++;
+                }
                 else if (tok->str() == ">")
                 {
                     level--;
@@ -1040,12 +1180,14 @@ void SymbolDatabase::SpaceInfo::getVarList()
             {
                 isClass = true;
                 vartok = tok->next();
+                typetok = tok1;
                 tok = vartok->next();
             }
             else if (tok && (Token::Match(tok, "> :: %type% %var% ;") || Token::Match(tok, ">> :: %type% %var% ;")))
             {
                 isClass = true;
                 vartok = tok->tokAt(3);
+                typetok = vartok->previous();
                 tok = vartok->next();
             }
             else if (tok && (Token::Match(tok, "> * %var% ;") || Token::Match(tok, ">> * %var% ;")))
@@ -1076,13 +1218,60 @@ void SymbolDatabase::SpaceInfo::getVarList()
                     Check::reportError(errmsg);
             }
 
-            addVar(vartok, varaccess, isMutable, isStatic, isConst, isClass);
+            const SpaceInfo *spaceInfo = NULL;
+
+            if (isClass)
+                spaceInfo = check->findVarType(this, typetok);
+
+            addVar(vartok, varaccess, isMutable, isStatic, isConst, isClass, spaceInfo);
         }
     }
 }
 
 //---------------------------------------------------------------------------
 
+const SymbolDatabase::SpaceInfo *SymbolDatabase::findVarType(const SpaceInfo *start, const Token *type) const
+{
+    std::list<SpaceInfo *>::const_iterator it;
+
+    for (it = spaceInfoList.begin(); it != spaceInfoList.end(); ++it)
+    {
+        const SpaceInfo *info = *it;
+
+        // skip namespaces and functions
+        if (info->type == SpaceInfo::Namespace || info->type == SpaceInfo::Function || info->type == SpaceInfo::Global)
+            continue;
+
+        // do the names match?
+        if (info->className == type->str())
+        {
+            // check if type does not have a namespace
+            if (type->previous()->str() != "::")
+            {
+                const SpaceInfo *parent = start;
+
+                // check if in same namespace
+                while (parent && parent != info->nestedIn)
+                    parent = parent->nestedIn;
+
+                if (info->nestedIn == parent)
+                    return info;
+            }
+
+            // type has a namespace
+            else
+            {
+                // FIXME check if namespace path matches supplied path
+                return info;
+            }
+        }
+    }
+
+    return NULL;
+}
+
+//---------------------------------------------------------------------------
+
 SymbolDatabase::SpaceInfo * SymbolDatabase::SpaceInfo::findInNestedList(const std::string & name)
 {
     std::list<SpaceInfo *>::iterator it;

lib/symboldatabase.h

@@ -41,11 +41,13 @@ public:
     SymbolDatabase(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger);
     ~SymbolDatabase();
 
+    class SpaceInfo;
+
     /** @brief Information about a member variable. Used when checking for uninitialized variables */
     class Var
     {
     public:
-        Var(const Token *token_, unsigned int index_, AccessControl access_, bool mutable_, bool static_, bool const_, bool class_)
+        Var(const Token *token_, unsigned int index_, AccessControl access_, bool mutable_, bool static_, bool const_, bool class_, const SpaceInfo *type_)
             : token(token_),
               index(index_),
               assign(false),
@@ -54,7 +56,8 @@ public:
               isMutable(mutable_),
               isStatic(static_),
               isConst(const_),
-              isClass(class_)
+              isClass(class_),
+              type(type_)
         {
         }
 
@@ -84,6 +87,9 @@ public:
 
         /** @brief is this variable a class (or unknown type)? */
         bool        isClass;
+
+        /** @brief pointer to user defined type info (for known types) */
+        const SpaceInfo   *type;
     };
 
     class Func
@@ -129,8 +135,6 @@ public:
         Type type;             // constructor, destructor, ...
     };
 
-    class SpaceInfo;
-
     struct BaseInfo
     {
         AccessControl access;  // public/protected/private
@@ -148,6 +152,7 @@ public:
     {
     public:
         enum SpaceType { Global, Class, Struct, Union, Namespace, Function };
+        enum NeedInitialization { Unknown, True, False };
 
         SpaceInfo(SymbolDatabase *check_, const Token *classDef_, SpaceInfo *nestedIn_);
 
@@ -165,6 +170,7 @@ public:
         std::list<SpaceInfo *> nestedList;
         AccessControl access;
         unsigned int numConstructors;
+        NeedInitialization needInitialization;
 
         bool isClassOrStruct() const
         {
@@ -189,9 +195,9 @@ public:
          */
         void initVar(const std::string &varname);
 
-        void addVar(const Token *token_, AccessControl access_, bool mutable_, bool static_, bool const_, bool class_)
+        void addVar(const Token *token_, AccessControl access_, bool mutable_, bool static_, bool const_, bool class_, const SpaceInfo *type_)
         {
-            varlist.push_back(Var(token_, varlist.size(), access_, mutable_, static_, const_, class_));
+            varlist.push_back(Var(token_, varlist.size(), access_, mutable_, static_, const_, class_, type_));
         }
 
         /**
@@ -246,6 +252,8 @@ private:
     bool isFunction(const Token *tok, const Token **funcStart, const Token **argStart) const;
     bool argsMatch(const Token *first, const Token *second, const std::string &path, unsigned int depth) const;
 
+    const SpaceInfo *findVarType(const SpaceInfo *start, const Token *type) const;
+
     const Tokenizer *_tokenizer;
     const Settings *_settings;
     ErrorLogger *_errorLogger;

test/testclass.cpp

@@ -69,6 +69,8 @@ private:
         TEST_CASE(uninitVarArray5);
         TEST_CASE(uninitVarArray6);
         TEST_CASE(uninitVarArray2D);
+        TEST_CASE(uninitVarStruct1); // ticket #2172
+        TEST_CASE(uninitVarStruct2); // ticket #838
         TEST_CASE(uninitMissingFuncDef);	// can't expand function in constructor
         TEST_CASE(privateCtor1);        	// If constructor is private..
         TEST_CASE(privateCtor2);        	// If constructor is private..
@@ -1594,8 +1596,7 @@ private:
                        "    };\n"
                        "    Bar bars[2];\n"
                        "};\n");
-        TODO_ASSERT_EQUALS("[test.cpp:4]: (warning) Member variable not initialized in the constructor 'Foo::bars'\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:4]: (warning) Member variable not initialized in the constructor 'Foo::bars'\n", errout.str());
-        ASSERT_EQUALS("", errout.str());	// So we notice if something is reported.
     }
 
     void uninitVar4()
@@ -2083,6 +2084,102 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void uninitVarStruct1() // ticket #2172
+    {
+        checkUninitVar("class A\n"
+                       "{\n"
+                       "private:\n"
+                       "    struct B {\n"
+                       "        std::string str1;\n"
+                       "        std::string str2;\n"
+                       "    }\n"
+                       "    struct B b;\n"
+                       "public:\n"
+                       "    A() {\n"
+                       "    }\n"
+                       "};\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkUninitVar("class A\n"
+                       "{\n"
+                       "private:\n"
+                       "    struct B {\n"
+                       "        char *str1;\n"
+                       "        char *str2;\n"
+                       "    }\n"
+                       "    struct B b;\n"
+                       "public:\n"
+                       "    A() {\n"
+                       "    }\n"
+                       "};\n");
+        ASSERT_EQUALS("[test.cpp:10]: (warning) Member variable not initialized in the constructor 'A::b'\n", errout.str());
+
+        checkUninitVar("class A\n"
+                       "{\n"
+                       "private:\n"
+                       "    struct B {\n"
+                       "        char *str1;\n"
+                       "        char *str2;\n"
+                       "        B() : str1(NULL), str2(NULL) { }\n"
+                       "    }\n"
+                       "    struct B b;\n"
+                       "public:\n"
+                       "    A() {\n"
+                       "    }\n"
+                       "};\n");
+        ASSERT_EQUALS("", errout.str());
+    }
+
+    void uninitVarStruct2() // ticket #838
+    {
+        checkUninitVar("struct POINT\n"
+                       "{\n"
+                       "    int x;\n"
+                       "    int y;\n"
+                       "};\n"
+                       "class Fred\n"
+                       "{\n"
+                       "private:\n"
+                       "    POINT p;\n"
+                       "public:\n"
+                       "    Fred()\n"
+                       "    { }\n"
+                       "};\n");
+        ASSERT_EQUALS("[test.cpp:11]: (warning) Member variable not initialized in the constructor 'Fred::p'\n", errout.str());
+
+        checkUninitVar("struct POINT\n"
+                       "{\n"
+                       "    int x;\n"
+                       "    int y;\n"
+                       "    POINT();\n"
+                       "};\n"
+                       "class Fred\n"
+                       "{\n"
+                       "private:\n"
+                       "    POINT p;\n"
+                       "public:\n"
+                       "    Fred()\n"
+                       "    { }\n"
+                       "};\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkUninitVar("struct POINT\n"
+                       "{\n"
+                       "    int x;\n"
+                       "    int y;\n"
+                       "    POINT() :x(0), y(0) { }\n"
+                       "};\n"
+                       "class Fred\n"
+                       "{\n"
+                       "private:\n"
+                       "    POINT p;\n"
+                       "public:\n"
+                       "    Fred()\n"
+                       "    { }\n"
+                       "};\n");
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void uninitMissingFuncDef()
     {
         // Unknown member function

test/testconstructors.cpp

@@ -820,12 +820,12 @@ private:
               "public:\n"
               "    A();\n"
               "    struct B {\n"
-              "        B();\n"
+              "        B(int x);\n"
               "        struct C {\n"
-              "            C();\n"
+              "            C(int y);\n"
               "            struct D {\n"
               "                int d;\n"
-              "                D();\n"
+              "                D(int z);\n"
               "            };\n"
               "            int c;\n"
               "        };\n"
@@ -836,10 +836,11 @@ private:
               "    B b;\n"
               "};\n"
               "A::A(){}\n"
-              "A::B::B(){}\n"
+              "A::B::B(int x){}\n"
-              "A::B::C::C(){}\n"
+              "A::B::C::C(int y){}\n"
-              "A::B::C::D::D(){}\n");
+              "A::B::C::D::D(int z){}\n");
         ASSERT_EQUALS("[test.cpp:20]: (warning) Member variable not initialized in the constructor 'A::a'\n"
+                      "[test.cpp:20]: (warning) Member variable not initialized in the constructor 'A::b'\n"
                       "[test.cpp:21]: (warning) Member variable not initialized in the constructor 'B::b'\n"
                       "[test.cpp:22]: (warning) Member variable not initialized in the constructor 'C::c'\n"
                       "[test.cpp:23]: (warning) Member variable not initialized in the constructor 'D::d'\n", errout.str());
@@ -848,9 +849,9 @@ private:
               "public:\n"
               "    A();\n"
               "    struct B {\n"
-              "        B();\n"
+              "        B(int x);\n"
               "        struct C {\n"
-              "            C();\n"
+              "            C(int y);\n"
               "            struct D {\n"
               "                D(const D &);\n"
               "                int d;\n"
@@ -864,10 +865,11 @@ private:
               "    B b;\n"
               "};\n"
               "A::A(){}\n"
-              "A::B::B(){}\n"
+              "A::B::B(int x){}\n"
-              "A::B::C::C(){}\n"
+              "A::B::C::C(int y){}\n"
               "A::B::C::D::D(const A::B::C::D & d){}\n");
         ASSERT_EQUALS("[test.cpp:20]: (warning) Member variable not initialized in the constructor 'A::a'\n"
+                      "[test.cpp:20]: (warning) Member variable not initialized in the constructor 'A::b'\n"
                       "[test.cpp:21]: (warning) Member variable not initialized in the constructor 'B::b'\n"
                       "[test.cpp:22]: (warning) Member variable not initialized in the constructor 'C::c'\n"
                       "[test.cpp:23]: (warning) Member variable not initialized in the constructor 'D::d'\n", errout.str());
@@ -876,11 +878,11 @@ private:
               "public:\n"
               "    A();\n"
               "    struct B {\n"
-              "        B();\n"
+              "        B(int x);\n"
               "        struct C {\n"
-              "            C();\n"
+              "            C(int y);\n"
               "            struct D {\n"
-              "                struct E { };\n"
+              "                struct E { int e; };\n"
               "                struct E d;\n"
               "                D(const E &);\n"
               "            };\n"
@@ -893,10 +895,11 @@ private:
               "    B b;\n"
               "};\n"
               "A::A(){}\n"
-              "A::B::B(){}\n"
+              "A::B::B(int x){}\n"
-              "A::B::C::C(){}\n"
+              "A::B::C::C(int y){}\n"
               "A::B::C::D::D(const A::B::C::D::E & e){}\n");
         ASSERT_EQUALS("[test.cpp:21]: (warning) Member variable not initialized in the constructor 'A::a'\n"
+                      "[test.cpp:21]: (warning) Member variable not initialized in the constructor 'A::b'\n"
                       "[test.cpp:22]: (warning) Member variable not initialized in the constructor 'B::b'\n"
                       "[test.cpp:23]: (warning) Member variable not initialized in the constructor 'C::c'\n"
                       "[test.cpp:24]: (warning) Member variable not initialized in the constructor 'D::d'\n", errout.str());