diff --git a/lib/checkexceptionsafety.cpp b/lib/checkexceptionsafety.cpp index a9037726e..9d4dc5450 100644 --- a/lib/checkexceptionsafety.cpp +++ b/lib/checkexceptionsafety.cpp @@ -18,7 +18,7 @@ //--------------------------------------------------------------------------- #include "checkexceptionsafety.h" - +#include "symboldatabase.h" #include "token.h" //--------------------------------------------------------------------------- @@ -37,33 +37,21 @@ void CheckExceptionSafety::destructors() if (!_settings->isEnabled("style")) return; + const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase(); + // Perform check.. - for (const Token * tok = _tokenizer->tokens(); tok; tok = tok->next()) { - // Skip executable scopes - if (Token::simpleMatch(tok, ") {")) - tok = tok->next()->link(); - - // only looking for destructors - if (!Token::Match(tok, "~ %var% ( ) {")) - continue; - - // Inspect this destructor.. - unsigned int indentlevel = 0; - for (const Token *tok2 = tok->tokAt(5); tok2; tok2 = tok2->next()) { - if (tok2->str() == "{") { - ++indentlevel; - } - - else if (tok2->str() == "}") { - if (indentlevel <= 1) - break; - --indentlevel; - } - - // throw found within a destructor - else if (tok2->str() == "throw") { - destructorsError(tok2); - break; + for (std::list::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) { + for (std::list::const_iterator j = i->functionList.begin(); j != i->functionList.end(); ++j) { + // only looking for destructors + if (j->type == Function::eDestructor && j->start) { + // Inspect this destructor.. + for (const Token *tok = j->start->next(); tok != j->start->link(); tok = tok->next()) { + // throw found within a destructor + if (tok->str() == "throw") { + destructorsError(tok); + break; + } + } } } } @@ -74,6 +62,8 @@ void CheckExceptionSafety::destructors() void CheckExceptionSafety::deallocThrow() { + const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase(); + // Deallocate a global/member pointer and then throw exception // the pointer will be a dead pointer for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { @@ -93,35 +83,10 @@ void CheckExceptionSafety::deallocThrow() if (varid == 0) continue; - // is this variable a global variable? - { - // TODO: Isn't it better to use symbol database instead? - bool globalVar = false; - for (const Token *tok2 = _tokenizer->tokens(); tok2; tok2 = tok2->next()) { - if (tok2->varId() == varid) { - globalVar = true; - break; - } - - if (tok2->str() == "class") { - while (tok2 && tok2->str() != ";" && tok2->str() != "{") - tok2 = tok2->next(); - tok2 = tok2 ? tok2->next() : 0; - if (!tok2) - break; - } - - if (tok2->str() == "{") { - tok2 = tok2->link(); - if (!tok2) - break; - } - } - - // Not a global variable.. skip checking this var. - if (!globalVar) - continue; - } + // we only look for global variables + const Variable* var = symbolDatabase->getVariableFromVarId(varid); + if (!var || !var->isGlobal()) + continue; // indentlevel.. unsigned int indentlevel = 0; @@ -139,7 +104,7 @@ void CheckExceptionSafety::deallocThrow() --indentlevel; } - if (tok2->str() == "throw") + else if (tok2->str() == "throw") ThrowToken = tok2; // if the variable is not assigned after the throw then it @@ -163,20 +128,27 @@ void CheckExceptionSafety::checkRethrowCopy() { if (!_settings->isEnabled("style")) return; - const char catchPattern[] = "catch ( const| %type% &|*| %var% ) { %any%"; - const Token *tok = Token::findmatch(_tokenizer->tokens(), catchPattern); + const char catchPattern1[] = "catch ("; + const char catchPattern2[] = "%var% ) { %any%"; + + const Token* tok = Token::findsimplematch(_tokenizer->tokens(), catchPattern1); while (tok) { - const Token *startBlockTok = tok->next()->link()->next(); - const Token *endBlockTok = startBlockTok->link(); - const unsigned int varid = startBlockTok->tokAt(-2)->varId(); + const Token* endScopeTok = tok->next(); + const Token* endBracketTok = tok->next()->link(); - const Token* rethrowTok = Token::findmatch(startBlockTok, "throw %varid%", endBlockTok, varid); - if (rethrowTok) { - rethrowCopyError(rethrowTok, startBlockTok->strAt(-2)); + if (endBracketTok && Token::Match(endBracketTok->previous(), catchPattern2)) { + const Token* startScopeTok = endBracketTok->next(); + endScopeTok = startScopeTok->link(); + const unsigned int varid = endBracketTok->previous()->varId(); + + const Token* rethrowTok = Token::findmatch(startScopeTok->next(), "throw %varid%", endScopeTok->previous(), varid); + if (rethrowTok) { + rethrowCopyError(rethrowTok, endBracketTok->strAt(-1)); + } } - tok = Token::findmatch(endBlockTok->next(), catchPattern); + tok = Token::findsimplematch(endScopeTok->next(), catchPattern1); } } diff --git a/test/testexceptionsafety.cpp b/test/testexceptionsafety.cpp index e50b142fe..6e55a139a 100644 --- a/test/testexceptionsafety.cpp +++ b/test/testexceptionsafety.cpp @@ -38,6 +38,7 @@ private: TEST_CASE(rethrowCopy1); TEST_CASE(rethrowCopy2); TEST_CASE(rethrowCopy3); + TEST_CASE(rethrowCopy4); } void check(const std::string &code) { @@ -59,11 +60,25 @@ private: } void destructors() { - check("x::~x()\n" - "{\n" - " throw e;\n" - "}\n"); + check("class x {\n" + " ~x() {\n" + " throw e;\n" + " }\n" + "};"); ASSERT_EQUALS("[test.cpp:3]: (error) Throwing exception in destructor\n", errout.str()); + + check("class x {\n" + " ~x();\n" + "};\n" + "x::~x() {\n" + " throw e;\n" + "}"); + ASSERT_EQUALS("[test.cpp:5]: (error) Throwing exception in destructor\n", errout.str()); + + check("x::~x() {\n" + " throw e;\n" + "}"); + TODO_ASSERT_EQUALS("[test.cpp:3]: (error) Throwing exception in destructor\n", "", errout.str()); } void deallocThrow1() { @@ -117,6 +132,18 @@ private: } void rethrowCopy3() { + check("void f() {\n" + " try {\n" + " foo();\n" + " }\n" + " catch(std::runtime_error err) {\n" + " throw err;\n" + " }\n" + "}"); + ASSERT_EQUALS("[test.cpp:6]: (style) Throwing a copy of the caught exception instead of rethrowing the original exception\n", errout.str()); + } + + void rethrowCopy4() { check("void f() {\n" " try\n" " {\n"