diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index f4ccc23c6..d2fe39eac 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -2098,6 +2098,187 @@ void CheckClass::selfInitializationError(const Token* tok, const std::string& va } +//--------------------------------------------------------------------------- +// Check for member usage before initialization in constructor +//--------------------------------------------------------------------------- + +struct VarDependency { + VarDependency(const Variable *_var, const Token *_tok) + : var(_var), tok(_tok), initOrder(_var->index()) { } + + VarDependency(const Variable *_var, const Token *_tok, std::size_t _initOrder) + : var(_var), tok(_tok), initOrder(_initOrder) { } + + bool operator==(const Variable* _rhs) const { + return var == _rhs; + } + + std::vector< VarInfo > dependencies; + const Variable *var; + const Token *tok; + std::size_t initOrder; +}; + +void CheckClass::checkUsageBeforeInitialization() +{ + const std::size_t classes = symbolDatabase->classAndStructScopes.size(); + std::vector< VarDependency > vars; + + for (std::size_t i = 0; i < classes; ++i) { + const Scope * info = symbolDatabase->classAndStructScopes[i]; + + // lets do some preallocation for the vector + vars.reserve(info->varlist.size()); + + // iterate through all member functions looking for constructors + for (std::list::const_iterator funcIter = info->functionList.cbegin(); funcIter != info->functionList.cend(); ++funcIter) { + if (!(*funcIter).isConstructor() || !(*funcIter).hasBody()) + continue; + + vars.clear(); + // check for initializer list + const Token *tok = (*funcIter).arg->link()->next(); + if (tok->str() == ":") { + tok = tok->next(); + + // find all variable initializations in list + while (tok && tok != (*funcIter).functionScope->classStart) { + if (Token::Match(tok, "%name% (|{")) { + const Variable *var = tok->variable(); + + bool lhsIsReference = false; + + if (var && (var->scope() == info)) { + vars.push_back(VarDependency(var, tok)); + lhsIsReference = var->isReference(); + } else { //Sanity check: Seems lhs is not a member of this class + tok = tok->next(); + continue; + } + + tok = tok->next(); + + const Token* tokEnd = tok->link(); + + if (tokEnd) { + tok = tok->next(); + + while (tok && tok != tokEnd) { + if (tok->str() == "&") { // treat adresses as always correct value (correct?) + tok = tok->next(); + } else { + const Variable *dep = tok->variable(); + + if (dep && (dep->scope() == info) && !dep->isStatic() && (!lhsIsReference || dep->isReference())) { + if (tok->varId() == dep->nameToken()->varId()) { + vars.back().dependencies.push_back(VarInfo(dep, tok)); + } + } + } + tok = tok->next(); + } + } + } else { + tok = tok->next(); + } + } + } + + // report initialization list errors + for (std::size_t j = 0, j_max = vars.size(); j < j_max; ++j) { + const std::vector< VarInfo >& deps = vars[j].dependencies; + const std::size_t init = vars[j].initOrder; + for (std::size_t k = 0, k_max = deps.size(); k < k_max; ++k) { + const VarInfo& varInfo = deps[k]; + + if (init <= varInfo.var->index()) { + usageBeforeInitializationError(varInfo.tok, info->className, varInfo.var->name(), false); + } + } + } + + // check constructor compound statement for initializations + if (tok == (*funcIter).functionScope->classStart) { + tok = tok->next(); + + while (tok && tok != (*funcIter).functionScope->classEnd) { + if (Token::Match(tok, "%name% =")) { + const Variable *var = tok->variable(); + const std::vector< VarDependency >::const_iterator currentEnd = vars.cend(); + + if (var && (var->scope() == info)) { + const std::vector< VarDependency >::const_iterator depIter = std::find(vars.cbegin(), currentEnd, var); + if (depIter == vars.cend()) { + vars.push_back(VarDependency(var, tok, vars.size())); + } + } + + tok = tok->next()->next(); + while (tok && tok->str() != ";") { // parse variables until end of assignment + if (Token::Match(tok, "%name% =")) { // cascading assignments + const Variable *varRhs = tok->variable(); + + if (varRhs && (varRhs->scope() == info)) { + if (tok->varId() == varRhs->nameToken()->varId()) { + const std::vector< VarDependency >::const_iterator depIter = std::find(vars.cbegin(), vars.cend(), varRhs); + if (depIter == vars.cend()) { + if (vars.size() > 0) { + vars.push_back(VarDependency(varRhs, tok, vars.back().initOrder)); + } else { + vars.push_back(VarDependency(varRhs, tok)); + } + } + } + } + + tok = tok->next()->next(); + } else { + bool isAddress = false; + if (tok->str() == "&") { + isAddress = true; + tok = tok->next(); + } + + const Variable *dep = tok->variable(); + + + if (dep && (dep->scope() == info) && !(dep->isClass() || dep->isStatic() || dep->isReference() || isAddress)) { + if (tok->varId() == dep->nameToken()->varId()) { + const std::vector< VarDependency >::const_iterator depIter = std::find(vars.cbegin(), currentEnd, dep); + if (depIter == currentEnd) + usageBeforeInitializationError(tok, info->className, dep->name(), false); + } + } + + tok = tok->next(); + } + } + } else if (Token::Match(tok, "%name% ++|--")) { // check for post increment/decrement of unitialized members + const Variable *var = tok->variable(); + + if (var && (var->scope() == info) && !(var->isClass() || var->isStatic())) { + if (tok->varId() == var->nameToken()->varId()) { + const std::vector< VarDependency >::const_iterator depIter = std::find(vars.cbegin(), vars.cend(), var); + if (depIter == vars.cend()) + usageBeforeInitializationError(tok, info->className, var->name(), false); + } + } + + tok = tok->next()->next(); + } else + tok = tok->next(); + } + } + } + } +} + +void CheckClass::usageBeforeInitializationError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive) +{ + reportError(tok, Severity::error, "usageBeforeInitialization", "Member variable '" + classname + "::" + varname + "' is used before it is initialized.", inconclusive); +} + + //--------------------------------------------------------------------------- // Check for pure virtual function calls //--------------------------------------------------------------------------- diff --git a/lib/checkclass.h b/lib/checkclass.h index d259c7788..c859b9506 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -69,6 +69,7 @@ public: checkClass.initializerListOrder(); checkClass.initializationListUsage(); checkClass.checkSelfInitialization(); + checkClass.checkUsageBeforeInitialization(); checkClass.virtualDestructor(); checkClass.checkConst(); @@ -128,6 +129,9 @@ public: /** @brief Check for initialization of a member with itself */ void checkSelfInitialization(); + /** @brief Check for member usage before it is initialized */ + void checkUsageBeforeInitialization(); + void copyconstructors(); /** @brief call of pure virtual function */ @@ -166,6 +170,7 @@ private: void initializerListError(const Token *tok1,const Token *tok2, const std::string & classname, const std::string &varname); void suggestInitializationList(const Token *tok, const std::string& varname); void selfInitializationError(const Token* tok, const std::string& varname); + void usageBeforeInitializationError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive); void callsPureVirtualFunctionError(const Function & scopeFunction, const std::list & tokStack, const std::string &purefuncname); void duplInheritedMembersError(const Token* tok1, const Token* tok2, const std::string &derivedname, const std::string &basename, const std::string &variablename, bool derivedIsStruct, bool baseIsStruct); @@ -197,6 +202,7 @@ private: c.initializerListError(0, 0, "class", "variable"); c.suggestInitializationList(0, "variable"); c.selfInitializationError(0, "var"); + c.usageBeforeInitializationError(0, "classname", "varname", false); c.duplInheritedMembersError(0, 0, "class", "class", "variable", false, false); } @@ -221,6 +227,7 @@ private: "- Order of initializations\n" "- Suggest usage of initialization list\n" "- Initialization of a member with itself\n" + "- Usage of a member before it is initialized\n" "- Suspicious subtraction from 'this'\n" "- Call of pure virtual function in constructor/destructor\n" "- Duplicated inherited data members\n"; diff --git a/test/testclass.cpp b/test/testclass.cpp index 0f6d7896c..3c74b7751 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -169,6 +169,7 @@ private: TEST_CASE(initializerListOrder); TEST_CASE(initializerListUsage); TEST_CASE(selfInitialization); + TEST_CASE(usageBeforeInitialization); TEST_CASE(pureVirtualFunctionCall); TEST_CASE(pureVirtualFunctionCallOtherClass); @@ -6036,6 +6037,101 @@ private: ASSERT_EQUALS("", errout.str()); } + void checkUsageBeforeInitialization(const char code []) { + // Clear the error log + errout.str(""); + + // Check.. + Settings settings; + + // Tokenize.. + Tokenizer tokenizer(&settings, this); + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + tokenizer.simplifyTokenList2(); + + CheckClass checkClass(&tokenizer, &settings, this); + checkClass.checkUsageBeforeInitialization(); + } + + void usageBeforeInitialization() { + checkUsageBeforeInitialization("class A {\n" // Ticket #220 + " int a;\n" + " double b;\n" + " double c;\n" + "public:\n" + " A(const double& x) {\n" + " a = a + 4;\n" + " c = b * 2;\n" + " b = x;\n" + " }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:7]: (error) Member variable 'A::a' is used before it is initialized.\n" + "[test.cpp:8]: (error) Member variable 'A::b' is used before it is initialized.\n", errout.str()); + + checkUsageBeforeInitialization("class A {\n" + " int a;\n" + " double b;\n" + " double c;\n" + "public:\n" + " A(const double& x) : a(a+4), c(b*2), b(x) {}\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:6]: (error) Member variable 'A::a' is used before it is initialized.\n", errout.str()); + + checkUsageBeforeInitialization("class A {\n" + " int a;\n" + " double b;\n" + " double c;\n" + "public:\n" + " A(const double& x) : a(a++), b(c*2), c(x) {}\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:6]: (error) Member variable 'A::a' is used before it is initialized.\n" + "[test.cpp:6]: (error) Member variable 'A::c' is used before it is initialized.\n", errout.str()); + + checkUsageBeforeInitialization("class A {\n" // Ticket #4856 + " int a;\n" + "public:\n" + " A() {\n" + " a++;\n" + " }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:5]: (error) Member variable 'A::a' is used before it is initialized.\n", errout.str()); + + checkUsageBeforeInitialization("class A {\n" + " int* a;\n" + " int& b;\n" + " int c;\n" + "public:\n" + " A() : a(&c), b(c), c(0) {\n" + " c--;\n" + " }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + checkUsageBeforeInitialization("class A {\n" + " int a;\n" + " int b;\n" + " int c;\n" + "public:\n" + " A(int _a, int _b) : a(++_a), b(a + _b) {\n" + " _a = c + _b;\n" + " c = a + b + _a + _b;\n" + " }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:7]: (error) Member variable 'A::c' is used before it is initialized.\n", errout.str()); + + + checkUsageBeforeInitialization("class A {\n" + " int a;\n" + " int b;\n" + "public:\n" + " A(const A& that) : a(that.a) {\n" + " b = that.b;\n" + " }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + } + void checkPureVirtualFunctionCall(const char code[], const Settings *s = 0, bool inconclusive = true) { // Clear the error log errout.str("");