Skip to content

Commit 847d28d

Browse files
IOBYTEdanmar
authored andcommitted
Fixed cppcheck-opensource#5638 (is there any plan to check noexcept correctness?)
1 parent 4ae204e commit 847d28d

8 files changed

Lines changed: 346 additions & 168 deletions

lib/checkexceptionsafety.cpp

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,3 +176,48 @@ void CheckExceptionSafety::checkCatchExceptionByValue()
176176
catchExceptionByValueError(i->classDef);
177177
}
178178
}
179+
180+
181+
//--------------------------------------------------------------------------
182+
// void func() noexcept { throw x; }
183+
//--------------------------------------------------------------------------
184+
void CheckExceptionSafety::noexceptThrows()
185+
{
186+
const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
187+
188+
const std::size_t functions = symbolDatabase->functionScopes.size();
189+
for (std::size_t i = 0; i < functions; ++i) {
190+
const Scope * scope = symbolDatabase->functionScopes[i];
191+
// onlycheck noexcept functions
192+
if (scope->function && scope->function->isNoExcept &&
193+
(!scope->function->noexceptArg || scope->function->noexceptArg->str() == "true")) {
194+
for (const Token *tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
195+
if (tok->str() != "throw") {
196+
noexceptThrowError(tok);
197+
}
198+
}
199+
}
200+
}
201+
}
202+
203+
//--------------------------------------------------------------------------
204+
// void func() throw() { throw x; }
205+
//--------------------------------------------------------------------------
206+
void CheckExceptionSafety::nothrowThrows()
207+
{
208+
const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
209+
210+
const std::size_t functions = symbolDatabase->functionScopes.size();
211+
for (std::size_t i = 0; i < functions; ++i) {
212+
const Scope * scope = symbolDatabase->functionScopes[i];
213+
// onlycheck throw() functions
214+
if (scope->function && scope->function->isThrow && !scope->function->throwArg) {
215+
for (const Token *tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
216+
if (tok->str() != "throw") {
217+
nothrowThrowError(tok);
218+
}
219+
}
220+
}
221+
}
222+
}
223+

lib/checkexceptionsafety.h

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ class CPPCHECKLIB CheckExceptionSafety : public Check {
6161
checkExceptionSafety.deallocThrow();
6262
checkExceptionSafety.checkRethrowCopy();
6363
checkExceptionSafety.checkCatchExceptionByValue();
64+
checkExceptionSafety.noexceptThrows();
65+
checkExceptionSafety.nothrowThrows();
6466
}
6567

6668
/** Don't throw exceptions in destructors */
@@ -75,6 +77,12 @@ class CPPCHECKLIB CheckExceptionSafety : public Check {
7577
/** @brief %Check for exceptions that are caught by value instead of by reference */
7678
void checkCatchExceptionByValue();
7779

80+
/** @brief %Check for noexcept functions that throw */
81+
void noexceptThrows();
82+
83+
/** @brief %Check for throw() functions that throw */
84+
void nothrowThrows();
85+
7886
private:
7987
/** Don't throw exceptions in destructors */
8088
void destructorsError(const Token * const tok) {
@@ -100,13 +108,25 @@ class CPPCHECKLIB CheckExceptionSafety : public Check {
100108
"as a (const) reference which is usually recommended in C++.");
101109
}
102110

111+
/** Don't throw exceptions in noexcept functions */
112+
void noexceptThrowError(const Token * const tok) {
113+
reportError(tok, Severity::error, "exceptThrowInNoexecptFunction", "Exception thrown in noexcept function.");
114+
}
115+
116+
/** Don't throw exceptions in throw() functions */
117+
void nothrowThrowError(const Token * const tok) {
118+
reportError(tok, Severity::error, "exceptThrowInNoThrowFunction", "Exception thrown in throw() function.");
119+
}
120+
103121
/** Generate all possible errors (for --errorlist) */
104122
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
105123
CheckExceptionSafety c(0, settings, errorLogger);
106124
c.destructorsError(0);
107125
c.deallocThrowError(0, "p");
108126
c.rethrowCopyError(0, "varname");
109127
c.catchExceptionByValueError(0);
128+
c.noexceptThrowError(0);
129+
c.nothrowThrowError(0);
110130
}
111131

112132
/** Short description of class (for --doc) */
@@ -120,7 +140,9 @@ class CPPCHECKLIB CheckExceptionSafety : public Check {
120140
"* Throwing exceptions in destructors\n"
121141
"* Throwing exception during invalid state\n"
122142
"* Throwing a copy of a caught exception instead of rethrowing the original exception\n"
123-
"* Exception caught by value instead of by reference\n";
143+
"* Exception caught by value instead of by reference\n"
144+
"* Throwing exception in noexcept function\n"
145+
"* Throwing exception in nothrow() function\n";
124146
}
125147
};
126148
/// @}

lib/symboldatabase.cpp

Lines changed: 149 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,72 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
458458
scope->functionList.push_back(function);
459459
}
460460

461+
// noexcept;
462+
// const noexcept;
463+
else if (Token::Match(end, ") const| noexcept ;")) {
464+
function.isNoExcept = true;
465+
466+
if (end->next()->str() == "const")
467+
tok = end->tokAt(3);
468+
else
469+
tok = end->tokAt(2);
470+
471+
scope->functionList.push_back(function);
472+
}
473+
474+
// noexcept const;
475+
else if (Token::simpleMatch(end, ") noexcept const ;")) {
476+
function.isNoExcept = true;
477+
478+
tok = end->tokAt(3);
479+
480+
scope->functionList.push_back(function);
481+
}
482+
483+
// noexcept(...);
484+
// noexcept(...) const;
485+
else if (Token::simpleMatch(end, ") noexcept (") &&
486+
Token::Match(end->linkAt(2), ") const| ;")) {
487+
function.isNoExcept = true;
488+
489+
if (end->linkAt(2)->strAt(1) == "const")
490+
tok = end->linkAt(2)->tokAt(2);
491+
else
492+
tok = end->linkAt(2)->next();
493+
494+
scope->functionList.push_back(function);
495+
}
496+
497+
// const noexcept(...);
498+
else if (Token::simpleMatch(end, ") const noexcept (") &&
499+
Token::simpleMatch(end->linkAt(3), ") ;")) {
500+
function.isNoExcept = true;
501+
502+
tok = end->linkAt(3)->next();
503+
504+
scope->functionList.push_back(function);
505+
}
506+
507+
// throw()
508+
// const throw()
509+
else if (Token::Match(end, ") const| throw (") &&
510+
(end->next()->str() == "const" ? Token::Match(end->linkAt(3), ") ;") :
511+
Token::Match(end->linkAt(2), ") ;"))) {
512+
function.isThrow = true;
513+
514+
if (end->next()->str() == "const") {
515+
if (end->strAt(4) != ")")
516+
function.throwArg = end->tokAt(4);
517+
tok = end->linkAt(3)->next();
518+
} else {
519+
if (end->strAt(3) != ")")
520+
function.throwArg = end->tokAt(3);
521+
tok = end->linkAt(2)->next();
522+
}
523+
524+
scope->functionList.push_back(function);
525+
}
526+
461527
// pure virtual function
462528
else if (Token::Match(end, ") const| = %any% ;")) {
463529
function.isPure = true;
@@ -481,6 +547,28 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
481547
function.isInline = true;
482548
function.hasBody = true;
483549

550+
if (Token::Match(end, ") const| noexcept")) {
551+
int arg = 2;
552+
553+
if (end->strAt(1) == "const")
554+
arg++;
555+
556+
if (end->strAt(arg) == "(")
557+
function.noexceptArg = end->tokAt(arg + 1);
558+
559+
function.isNoExcept = true;
560+
} else if (Token::Match(end, ") const| throw (")) {
561+
int arg = 3;
562+
563+
if (end->strAt(1) == "const")
564+
arg++;
565+
566+
if (end->strAt(arg) != ")")
567+
function.throwArg = end->tokAt(arg);
568+
569+
function.isThrow = true;
570+
}
571+
484572
// find start of function '{'
485573
while (end && end->str() != "{" && end->str() != ";")
486574
end = end->next();
@@ -549,11 +637,13 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
549637
if (isFunction(tok, scope, &funcStart, &argStart)) {
550638
bool retFuncPtr = Token::simpleMatch(argStart->link(), ") ) (");
551639
const Token* scopeBegin = argStart->link()->next();
552-
553640
if (retFuncPtr)
554641
scopeBegin = scopeBegin->next()->link()->next();
555642
if (scopeBegin->isName()) { // Jump behind 'const' or unknown Macro
556643
scopeBegin = scopeBegin->next();
644+
if (scopeBegin->str() == "throw")
645+
scopeBegin = scopeBegin->next();
646+
557647
if (scopeBegin->link() && scopeBegin->str() == "(") // Jump behind unknown macro of type THROW(...)
558648
scopeBegin = scopeBegin->link()->next();
559649
}
@@ -576,6 +666,29 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
576666
else {
577667
Function* function = addGlobalFunction(scope, tok, argStart, funcStart);
578668
function->retFuncPtr = retFuncPtr;
669+
670+
// global functions can't be const but we have tests that are
671+
if (Token::Match(argStart->link(), ") const| noexcept")) {
672+
int arg = 2;
673+
674+
if (argStart->link()->strAt(1) == "const")
675+
arg++;
676+
677+
if (argStart->link()->strAt(arg) == "(")
678+
function->noexceptArg = argStart->link()->tokAt(arg + 1);
679+
680+
function->isNoExcept = true;
681+
} else if (Token::Match(argStart->link(), ") const| throw (")) {
682+
int arg = 3;
683+
684+
if (argStart->link()->strAt(1) == "const")
685+
arg++;
686+
687+
if (argStart->link()->strAt(arg) != ")")
688+
function->throwArg = argStart->link()->tokAt(arg);
689+
690+
function->isThrow = true;
691+
}
579692
}
580693

581694
// syntax error?
@@ -598,6 +711,28 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
598711
if (newFunc) {
599712
Function* func = addGlobalFunctionDecl(scope, tok, argStart, funcStart);
600713
func->retFuncPtr = retFuncPtr;
714+
715+
if (Token::Match(argStart->link(), ") const| noexcept")) {
716+
int arg = 2;
717+
718+
if (argStart->link()->strAt(1) == "const")
719+
arg++;
720+
721+
if (argStart->link()->strAt(arg) == "(")
722+
func->noexceptArg = argStart->link()->tokAt(arg + 1);
723+
724+
func->isNoExcept = true;
725+
} else if (Token::Match(argStart->link(), ") const| throw (")) {
726+
int arg = 3;
727+
728+
if (argStart->link()->strAt(1) == "const")
729+
arg++;
730+
731+
if (argStart->link()->strAt(arg) != ")")
732+
func->throwArg = argStart->link()->tokAt(arg);
733+
734+
func->isThrow = true;
735+
}
601736
}
602737

603738
tok = scopeBegin;
@@ -965,11 +1100,16 @@ bool SymbolDatabase::isFunction(const Token *tok, const Scope* outerScope, const
9651100
tok->strAt(-1) == "::" || tok->strAt(-1) == "~" || // or a scope qualifier in front of tok
9661101
outerScope->isClassOrStruct())) { // or a ctor/dtor
9671102
const Token* tok2 = tok->next()->link()->next();
968-
if ((Token::Match(tok2, "const| ;|{|=") ||
1103+
if (tok2 &&
1104+
(Token::Match(tok2, "const| ;|{|=") ||
9691105
(Token::Match(tok2, "%var% ;|{") && tok2->isUpperCaseName()) ||
9701106
(Token::Match(tok2, "%var% (") && tok2->isUpperCaseName() && tok2->next()->link()->strAt(1) == "{") ||
9711107
Token::Match(tok2, ": ::| %var% (|::|<|{") ||
972-
Token::Match(tok2, "= delete|default ;"))) {
1108+
Token::Match(tok2, "= delete|default ;") ||
1109+
Token::Match(tok2, "const| noexcept const| {|:|;") ||
1110+
(Token::Match(tok2, "const| noexcept|throw (") &&
1111+
tok2->str() == "const" ? (tok2->tokAt(2) && Token::Match(tok2->tokAt(2)->link(), ") const| {|:|;")) :
1112+
(tok2->next() && Token::Match(tok2->next()->link(), ") const| {|:|;"))))) {
9731113
*funcStart = tok;
9741114
*argStart = tok->next();
9751115
return true;
@@ -980,7 +1120,8 @@ bool SymbolDatabase::isFunction(const Token *tok, const Scope* outerScope, const
9801120
else if (Token::Match(tok, "%var% <") && Token::simpleMatch(tok->next()->link(), "> (")) {
9811121
const Token* tok2 = tok->next()->link()->next()->link();
9821122
if (Token::Match(tok2, ") const| ;|{|=") ||
983-
Token::Match(tok2, ") : ::| %var% (|::|<|{")) {
1123+
Token::Match(tok2, ") : ::| %var% (|::|<|{") ||
1124+
Token::Match(tok->next()->link()->next()->link(), ") const| noexcept {|;|(")) {
9841125
*funcStart = tok;
9851126
*argStart = tok2->link();
9861127
return true;
@@ -1772,8 +1913,12 @@ void SymbolDatabase::printOut(const char *title) const
17721913
std::cout << " isExplicit: " << (func->isExplicit ? "true" : "false") << std::endl;
17731914
std::cout << " isDefault: " << (func->isDefault ? "true" : "false") << std::endl;
17741915
std::cout << " isDelete: " << (func->isDelete ? "true" : "false") << std::endl;
1916+
std::cout << " isNoExcept: " << (func->isNoExcept ? "true" : "false") << std::endl;
1917+
std::cout << " isThrow: " << (func->isThrow ? "true" : "false") << std::endl;
17751918
std::cout << " isOperator: " << (func->isOperator ? "true" : "false") << std::endl;
17761919
std::cout << " retFuncPtr: " << (func->retFuncPtr ? "true" : "false") << std::endl;
1920+
std::cout << " noexceptArg: " << (func->noexceptArg ? func->noexceptArg->str() : "none") << std::endl;
1921+
std::cout << " throwArg: " << (func->throwArg ? func->throwArg->str() : "none") << std::endl;
17771922
std::cout << " tokenDef: " << func->tokenDef->str() << " " <<_tokenizer->list.fileLine(func->tokenDef) << std::endl;
17781923
std::cout << " argDef: " << _tokenizer->list.fileLine(func->argDef) << std::endl;
17791924
if (!func->isConstructor() && !func->isDestructor())

lib/symboldatabase.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -544,8 +544,12 @@ class CPPCHECKLIB Function {
544544
isExplicit(false),
545545
isDefault(false),
546546
isDelete(false),
547+
isNoExcept(false),
548+
isThrow(false),
547549
isOperator(false),
548-
retFuncPtr(false) {
550+
retFuncPtr(false),
551+
noexceptArg(nullptr),
552+
throwArg(nullptr) {
549553
}
550554

551555
const std::string &name() const {
@@ -610,8 +614,12 @@ class CPPCHECKLIB Function {
610614
bool isExplicit; // is explicit
611615
bool isDefault; // is default
612616
bool isDelete; // is delete
617+
bool isNoExcept; // is noexcept
618+
bool isThrow; // is throw
613619
bool isOperator; // is operator
614620
bool retFuncPtr; // returns function pointer
621+
const Token *noexceptArg;
622+
const Token *throwArg;
615623

616624
static bool argsMatch(const Scope *info, const Token *first, const Token *second, const std::string &path, unsigned int depth);
617625

0 commit comments

Comments
 (0)