From 8bd783f820081c78672c3dd3552fbc3e5fa37e73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Thu, 24 Dec 2020 22:58:19 +0100 Subject: [PATCH] Refactoring; Added findBreakScope and Scope::isLoopScope() --- lib/astutils.cpp | 19 +++++++++++-------- lib/astutils.h | 4 ++++ lib/checkbool.cpp | 2 +- lib/checkother.cpp | 2 +- lib/checkstl.cpp | 4 ++-- lib/checkvaarg.cpp | 8 +++----- lib/forwardanalyzer.cpp | 6 ------ lib/symboldatabase.h | 4 ++++ 8 files changed, 26 insertions(+), 23 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 0dcc49807..277d556a0 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -494,6 +494,12 @@ const Token* getCondTokFromEnd(const Token* endBlock) return getCondTokFromEndImpl(endBlock); } +const Scope *findBreakScope(const Scope *s) { + while (s && !s->isLoopScope() && s->type != Scope::ScopeType::eSwitch) + s = s->nestedIn; + return s; +} + bool extractForLoopValues(const Token *forToken, nonneg int * const varid, bool * const knownInitValue, @@ -2240,8 +2246,7 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const if (tok->scope() == expr->scope()) mValueFlowKnown = false; - Scope::ScopeType scopeType = tok->scope()->type; - if (scopeType == Scope::eWhile || scopeType == Scope::eFor || scopeType == Scope::eDo) { + if (tok->scope()->isLoopScope()) { // check condition const Token *conditionStart = nullptr; const Token *conditionEnd = nullptr; @@ -2508,12 +2513,10 @@ FwdAnalysis::Result FwdAnalysis::check(const Token* expr, const Token* startToke // Break => continue checking in outer scope while (mWhat!=What::ValueFlow && result.type == FwdAnalysis::Result::Type::BREAK) { - const Scope *s = result.token->scope(); - while (s->type == Scope::eIf) - s = s->nestedIn; - if (s->type != Scope::eSwitch && s->type != Scope::eWhile && s->type != Scope::eFor) - break; - result = checkRecursive(expr, s->bodyEnd->next(), endToken, exprVarIds, local, false); + const Scope *breakScope = findBreakScope(result.token->scope()); + if (!breakScope) + break; + result = checkRecursive(expr, breakScope->bodyEnd->next(), endToken, exprVarIds, local, false); } return result; diff --git a/lib/astutils.h b/lib/astutils.h index a81d02613..facedaa3e 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -31,6 +31,7 @@ #include "utils.h" class Library; +class Scope; class Settings; class Token; class Variable; @@ -114,6 +115,9 @@ const Token* getCondTok(const Token* tok); Token* getCondTokFromEnd(Token* endBlock); const Token* getCondTokFromEnd(const Token* endBlock); +/// For a "break", locate the outer loop/switch scope that is finished +const Scope *findBreakScope(const Scope *scope); + /** * Extract for loop values: loopvar varid, init value, step value, last value (inclusive) */ diff --git a/lib/checkbool.cpp b/lib/checkbool.cpp index 1f1a52c49..48a889c58 100644 --- a/lib/checkbool.cpp +++ b/lib/checkbool.cpp @@ -369,7 +369,7 @@ void CheckBool::pointerArithBool() const SymbolDatabase* symbolDatabase = mTokenizer->getSymbolDatabase(); for (const Scope &scope : symbolDatabase->scopeList) { - if (scope.type != Scope::eIf && scope.type != Scope::eWhile && scope.type != Scope::eDo && scope.type != Scope::eFor) + if (scope.type != Scope::eIf && !scope.isLoopScope()) continue; const Token* tok = scope.classDef->next()->astOperand2(); if (scope.type == Scope::eFor) { diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 7ae70da23..792bf019e 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -981,7 +981,7 @@ void CheckOther::checkVariableScope() bool CheckOther::checkInnerScope(const Token *tok, const Variable* var, bool& used) { const Scope* scope = tok->next()->scope(); - bool loopVariable = scope->type == Scope::eFor || scope->type == Scope::eWhile || scope->type == Scope::eDo; + bool loopVariable = scope->isLoopScope(); bool noContinue = true; const Token* forHeadEnd = nullptr; const Token* end = tok->link(); diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index ad1539a50..efd15cf45 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -1039,7 +1039,7 @@ void CheckStl::stlOutOfBounds() for (const Scope &scope : symbolDatabase->scopeList) { const Token* tok = scope.classDef; // only interested in conditions - if ((scope.type != Scope::eFor && scope.type != Scope::eWhile && scope.type != Scope::eIf && scope.type != Scope::eDo) || !tok) + if ((!scope.isLoopScope() && scope.type != Scope::eIf) || !tok) continue; const Token *condition = nullptr; @@ -2001,7 +2001,7 @@ void CheckStl::checkDereferenceInvalidIterator() // Iterate over "if", "while", and "for" conditions where there may // be an iterator that is dereferenced before being checked for validity. for (const Scope &scope : mTokenizer->getSymbolDatabase()->scopeList) { - if (!(scope.type == Scope::eIf || scope.type == Scope::eDo || scope.type == Scope::eWhile || scope.type == Scope::eFor)) + if (!(scope.type == Scope::eIf || scope.isLoopScope())) continue; const Token* const tok = scope.classDef; diff --git a/lib/checkvaarg.cpp b/lib/checkvaarg.cpp index 9a0b8773e..e37bd5414 100644 --- a/lib/checkvaarg.cpp +++ b/lib/checkvaarg.cpp @@ -139,12 +139,10 @@ void CheckVaarg::va_list_usage() } else if (Token::Match(tok, "throw|return")) exitOnEndOfStatement = true; else if (tok->str() == "break") { - const Scope* scope = tok->scope(); - while (scope->nestedIn && scope->type != Scope::eFor && scope->type != Scope::eWhile && scope->type != Scope::eDo && scope->type != Scope::eSwitch) - scope = scope->nestedIn; - tok = scope->bodyEnd; - if (!tok) + const Scope *breakScope = findBreakScope(tok->scope()); + if (!breakScope) return; + tok = breakScope->bodyEnd; } else if (tok->str() == "goto" || (mTokenizer->isCPP() && tok->str() == "try")) { open = false; break; diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 08806a9db..da6e3e271 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -505,12 +505,6 @@ struct ForwardTraversal { return nullptr; } - static const Scope* findBreakScope(const Scope* scope) { - while (scope && scope->type != Scope::eWhile && scope->type != Scope::eFor && scope->type != Scope::eSwitch) - scope = scope->nestedIn; - return scope; - } - static Token* skipTo(Token* tok, const Token* dest, const Token* end = nullptr) { if (end && dest->index() > end->index()) return nullptr; diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index 2c16f2c44..081ea8340 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -1067,6 +1067,10 @@ public: return type != eClass && type != eStruct && type != eUnion && type != eGlobal && type != eNamespace && type != eEnum; } + bool isLoopScope() const { + return type == Scope::ScopeType::eFor || type == Scope::ScopeType::eWhile || type == Scope::ScopeType::eDo; + } + bool isLocal() const { return (type == eIf || type == eElse || type == eFor || type == eWhile || type == eDo ||