From 540207533b6052d3a8f98f6f2497071c31a728f3 Mon Sep 17 00:00:00 2001 From: Robert Reif Date: Tue, 27 Sep 2011 21:07:37 -0400 Subject: [PATCH] fix #3008 (New check: Order of initialisation list) --- lib/checkclass.cpp | 101 +++++++++++++++++++++++++++++++++++++++++++++ lib/checkclass.h | 6 +++ test/testclass.cpp | 33 +++++++++++++++ 3 files changed, 140 insertions(+) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index f99947609..cae26e77f 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -1784,3 +1784,104 @@ void CheckClass::checkConstError2(const Token *tok1, const Token *tok2, const st "sense conceptually. Think about your design and task of the function first - is " "it a function that must not change object internal state?"); } + +//--------------------------------------------------------------------------- +// ClassCheck: Check that initializer list is in declared order. +//--------------------------------------------------------------------------- + +struct VarInfo +{ + VarInfo(const Variable *_var, const Token *_tok) + : var(_var), tok(_tok) { } + + const Variable *var; + const Token *tok; +}; + +void CheckClass::initializerList() +{ + if (!_settings->isEnabled("style")) + return; + + // This check is not inconclusive. However it only determines if the initialization + // order is incorrect. It does not determine if being out of order causes + // a real error. Out of order is not necessarily an error but you can never + // have an error if the list is in order so this enforces defensive programming. + if (!_settings->inconclusive) + return; + + createSymbolDatabase(); + + std::list::const_iterator info; + + // iterate through all scopes looking for classes and structures + for (info = symbolDatabase->scopeList.begin(); info != symbolDatabase->scopeList.end(); ++info) + { + if (!info->isClassOrStruct()) + continue; + + std::list::const_iterator func; + + // iterate through all member functions looking for constructors + for (func = info->functionList.begin(); func != info->functionList.end(); ++func) + { + if (func->type == Function::eConstructor && func->hasBody) + { + // check for initializer list + const Token *tok = func->arg->link()->next(); + + if (tok->str() == ":") + { + std::vector vars; + tok = tok->next(); + + // find all variable initializations in list + while (tok && tok->str() != "{") + { + if (Token::Match(tok, "%var% (")) + { + const Variable *var = info->getVariable(tok->str()); + + if (var) + vars.push_back(VarInfo(var, tok)); + + if (Token::Match(tok->tokAt(2), "%var% =")) + { + var = info->getVariable(tok->strAt(2)); + + if (var) + vars.push_back(VarInfo(var, tok->tokAt(2))); + } + } + tok = tok->next(); + } + + // need at least 2 members to have out of order initialization + if (vars.size() > 1) + { + for (unsigned int i = 1; i < vars.size(); i++) + { + // check for out of order initialization + if (vars[i].var->index() < vars[i - 1].var->index()) + initializerListError(vars[i].tok,vars[i].var->nameToken(), info->className, vars[i].var->name()); + } + } + } + } + } + } +} + +void CheckClass::initializerListError(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &varname) +{ + std::list toks; + toks.push_back(tok1); + toks.push_back(tok2); + reportError(toks, Severity::style, "initializerList", + "Member variable '" + classname + "::" + + varname + "' is in the wrong order in the initializer list.\n" + "Members are initialized in the order they are declared, not the " + "order they are in the initializer list. Keeping the initializer list " + "in the same order that the members were declared prevents order dependent " + "initialization errors."); +} diff --git a/lib/checkclass.h b/lib/checkclass.h index 6c4f41823..93a24c0f0 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -63,6 +63,7 @@ public: checkClass.operatorEqRetRefThis(); checkClass.thisSubtraction(); checkClass.operatorEqToSelf(); + checkClass.initializerList(); checkClass.virtualDestructor(); checkClass.checkConst(); @@ -104,6 +105,9 @@ public: /** @brief can member function be const? */ void checkConst(); + /** @brief Check initializer list order */ + void initializerList(); + private: /** * @brief Create symbol database. For performance reasons, only call @@ -126,6 +130,7 @@ private: void operatorEqToSelfError(const Token *tok); void checkConstError(const Token *tok, const std::string &classname, const std::string &funcname); void checkConstError2(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &funcname); + void initializerListError(const Token *tok1,const Token *tok2, const std::string & classname, const std::string &varname); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) { @@ -141,6 +146,7 @@ private: c.operatorEqRetRefThisError(0); c.operatorEqToSelfError(0); c.checkConstError(0, "class", "function"); + c.initializerListError(0, 0, "class", "variable"); } std::string myName() const diff --git a/test/testclass.cpp b/test/testclass.cpp index 4a88d3243..81ef21d5c 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -204,6 +204,8 @@ private: TEST_CASE(constIfCfg); // ticket #1881 - fp when there are #if TEST_CASE(constFriend); // ticket #1921 - fp for friend function TEST_CASE(constUnion); // ticket #2111 - fp when there are union + + TEST_CASE(initializerList); } // Check the operator Equal @@ -6441,6 +6443,37 @@ private: ASSERT_EQUALS("", errout.str()); } + void checkInitializerList(const char code[]) + { + // Clear the error log + errout.str(""); + + // Check.. + Settings settings; + settings.addEnabled("style"); + settings.inconclusive = true; + + // Tokenize.. + Tokenizer tokenizer(&settings, this); + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + tokenizer.simplifyTokenList(); + + CheckClass checkClass(&tokenizer, &settings, this); + checkClass.initializerList(); + } + + void initializerList() + { + checkInitializerList("class Fred {\n" + " int a, b, c;\n" + "public:\n" + " Fred() : c(0), b(0), a(0) { }\n" + "};"); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2]: (style) Member variable 'Fred::b' is in the wrong order in the initializer list.\n" + "[test.cpp:4] -> [test.cpp:2]: (style) Member variable 'Fred::a' is in the wrong order in the initializer list.\n", errout.str()); + } + }; REGISTER_TEST(TestClass)