Skip to content

Commit

Permalink
CheckInternal::checkRedundantNextPrevious(): Fix FN, simplify (#6478)
Browse files Browse the repository at this point in the history
  • Loading branch information
chrchr-github authored Jun 3, 2024
1 parent bd694db commit 32fefc6
Show file tree
Hide file tree
Showing 35 changed files with 370 additions and 372 deletions.
34 changes: 17 additions & 17 deletions lib/astutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -453,8 +453,8 @@ bool isTemporary(const Token* tok, const Library* library, bool unknown)
return tok->valueType()->reference == Reference::None && tok->valueType()->pointer == 0;
}
const Token* ftok = nullptr;
if (Token::simpleMatch(tok->previous(), ">") && tok->previous()->link())
ftok = tok->previous()->link()->previous();
if (Token::simpleMatch(tok->previous(), ">") && tok->linkAt(-1))
ftok = tok->linkAt(-1)->previous();
else
ftok = tok->previous();
if (!ftok)
Expand Down Expand Up @@ -538,7 +538,7 @@ static T* nextAfterAstRightmostLeafGeneric(T* tok)
else
break;
} while (rightmostLeaf->astOperand1() || rightmostLeaf->astOperand2());
while (Token::Match(rightmostLeaf->next(), "]|)") && !hasToken(rightmostLeaf->next()->link(), rightmostLeaf->next(), tok))
while (Token::Match(rightmostLeaf->next(), "]|)") && !hasToken(rightmostLeaf->linkAt(1), rightmostLeaf->next(), tok))
rightmostLeaf = rightmostLeaf->next();
if (Token::Match(rightmostLeaf, "{|(|[") && rightmostLeaf->link())
rightmostLeaf = rightmostLeaf->link();
Expand Down Expand Up @@ -568,7 +568,7 @@ Token* astParentSkipParens(Token* tok)
if (parent->link() != nextAfterAstRightmostLeaf(tok))
return parent;
if (Token::Match(parent->previous(), "%name% (") ||
(Token::simpleMatch(parent->previous(), "> (") && parent->previous()->link()))
(Token::simpleMatch(parent->previous(), "> (") && parent->linkAt(-1)))
return parent;
return astParentSkipParens(parent);
}
Expand Down Expand Up @@ -839,7 +839,7 @@ static T* getCondTokFromEndImpl(T* endBlock)
if (Token::simpleMatch(startBlock->previous(), "do"))
return getCondTok(startBlock->previous());
if (Token::simpleMatch(startBlock->previous(), ")"))
return getCondTok(startBlock->previous()->link());
return getCondTok(startBlock->linkAt(-1));
if (Token::simpleMatch(startBlock->tokAt(-2), "} else {"))
return getCondTokFromEnd(startBlock->tokAt(-2));
return nullptr;
Expand Down Expand Up @@ -1669,7 +1669,7 @@ bool isSameExpression(bool macro, const Token *tok1, const Token *tok2, const Se
if (!compareTokenFlags(tok1, tok2, macro))
return false;

if (pure && tok1->isName() && tok1->next()->str() == "(" && tok1->str() != "sizeof" && !(tok1->variable() && tok1 == tok1->variable()->nameToken())) {
if (pure && tok1->isName() && tok1->strAt(1) == "(" && tok1->str() != "sizeof" && !(tok1->variable() && tok1 == tok1->variable()->nameToken())) {
if (!tok1->function()) {
if (Token::simpleMatch(tok1->previous(), ".")) {
const Token *lhs = tok1->previous();
Expand All @@ -1694,11 +1694,11 @@ bool isSameExpression(bool macro, const Token *tok1, const Token *tok2, const Se
}
}
// templates/casts
if ((tok1->next() && tok1->next()->link() && Token::Match(tok1, "%name% <")) ||
(tok2->next() && tok2->next()->link() && Token::Match(tok2, "%name% <"))) {
if ((tok1->next() && tok1->linkAt(1) && Token::Match(tok1, "%name% <")) ||
(tok2->next() && tok2->linkAt(1) && Token::Match(tok2, "%name% <"))) {

// non-const template function that is not a dynamic_cast => return false
if (pure && Token::simpleMatch(tok1->next()->link(), "> (") &&
if (pure && Token::simpleMatch(tok1->linkAt(1), "> (") &&
!(tok1->function() && tok1->function()->isConst()) &&
tok1->str() != "dynamic_cast")
return false;
Expand All @@ -1725,7 +1725,7 @@ bool isSameExpression(bool macro, const Token *tok1, const Token *tok2, const Se
// cast => assert that the casts are equal
if (tok1->str() == "(" && tok1->previous() &&
!tok1->previous()->isName() &&
!(tok1->previous()->str() == ">" && tok1->previous()->link())) {
!(tok1->strAt(-1) == ">" && tok1->linkAt(-1))) {
const Token *t1 = tok1->next();
const Token *t2 = tok2->next();
while (t1 && t2 &&
Expand Down Expand Up @@ -2052,7 +2052,7 @@ bool isConstExpression(const Token *tok, const Library& library)
return true;
if (tok->variable() && tok->variable()->isVolatile())
return false;
if (tok->isName() && tok->next()->str() == "(") {
if (tok->isName() && tok->strAt(1) == "(") {
if (!isConstFunctionCall(tok, library))
return false;
}
Expand Down Expand Up @@ -2233,8 +2233,8 @@ bool isReturnScope(const Token* const endToken, const Library& library, const To
*unknownFunc = prev->previous();
return false;
}
if (Token::simpleMatch(prev->previous(), ") ;") && prev->previous()->link() &&
isEscaped(prev->previous()->link()->astTop(), functionScope, library))
if (Token::simpleMatch(prev->previous(), ") ;") && prev->linkAt(-1) &&
isEscaped(prev->linkAt(-1)->astTop(), functionScope, library))
return true;
if (isEscaped(prev->previous()->astTop(), functionScope, library))
return true;
Expand Down Expand Up @@ -2740,7 +2740,7 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings &settings,

// structured binding, nonconst reference variable in lhs
if (Token::Match(tok2->astParent(), ":|=") && tok2 == tok2->astParent()->astOperand2() && Token::simpleMatch(tok2->astParent()->previous(), "]")) {
const Token *typeStart = tok2->astParent()->previous()->link()->previous();
const Token *typeStart = tok2->astParent()->linkAt(-1)->previous();
if (Token::simpleMatch(typeStart, "&"))
typeStart = typeStart->previous();
if (typeStart && Token::Match(typeStart->previous(), "[;{}(] auto &| [")) {
Expand Down Expand Up @@ -3095,7 +3095,7 @@ int numberOfArgumentsWithoutAst(const Token* start)
const Token* openBracket = start->next();
while (Token::simpleMatch(openBracket, ")"))
openBracket = openBracket->next();
if (openBracket && openBracket->str()=="(" && openBracket->next() && openBracket->next()->str()!=")") {
if (openBracket && openBracket->str()=="(" && openBracket->next() && openBracket->strAt(1)!=")") {
const Token* argument=openBracket->next();
while (argument) {
++arguments;
Expand Down Expand Up @@ -3595,7 +3595,7 @@ bool isGlobalData(const Token *expr)
return ChildrenToVisit::none;
}
}
if (tok->varId() == 0 && tok->isName() && tok->previous()->str() != ".") {
if (tok->varId() == 0 && tok->isName() && tok->strAt(-1) != ".") {
globalData = true;
return ChildrenToVisit::none;
}
Expand All @@ -3609,7 +3609,7 @@ bool isGlobalData(const Token *expr)
globalData = true;
return ChildrenToVisit::none;
}
if (tok->previous()->str() != "." && !tok->variable()->isLocal() && !tok->variable()->isArgument()) {
if (tok->strAt(-1) != "." && !tok->variable()->isLocal() && !tok->variable()->isArgument()) {
globalData = true;
return ChildrenToVisit::none;
}
Expand Down
22 changes: 11 additions & 11 deletions lib/checkclass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -983,7 +983,7 @@ void CheckClass::initializeVarList(const Function &func, std::list<const Functio
tok2 = tok2->next();
if (tok2->str() == "&")
tok2 = tok2->next();
if (isVariableChangedByFunctionCall(tok2, tok2->previous()->str() == "&", tok2->varId(), *mSettings, nullptr))
if (isVariableChangedByFunctionCall(tok2, tok2->strAt(-1) == "&", tok2->varId(), *mSettings, nullptr))
assignVar(usage, tok2->varId());
}
}
Expand Down Expand Up @@ -1047,7 +1047,7 @@ void CheckClass::initializeVarList(const Function &func, std::list<const Functio
const Token *tok2 = ftok;
while (tok2) {
if (tok2->strAt(1) == "[")
tok2 = tok2->next()->link();
tok2 = tok2->linkAt(1);
else if (Token::Match(tok2->next(), ". %name%"))
tok2 = tok2->tokAt(2);
else
Expand Down Expand Up @@ -1622,22 +1622,22 @@ void CheckClass::checkReturnPtrThis(const Scope *scope, const Function *func, co

// check if a function is called
if (tok->strAt(2) == "(" &&
tok->linkAt(2)->next()->str() == ";") {
tok->linkAt(2)->strAt(1) == ";") {
// check if it is a member function
for (std::list<Function>::const_iterator it = scope->functionList.cbegin(); it != scope->functionList.cend(); ++it) {
// check for a regular function with the same name and a body
if (it->type == Function::eFunction && it->hasBody() &&
it->token->str() == tok->next()->str()) {
it->token->str() == tok->strAt(1)) {
// check for the proper return type
if (it->tokenDef->previous()->str() == "&" &&
if (it->tokenDef->strAt(-1) == "&" &&
it->tokenDef->strAt(-2) == scope->className) {
// make sure it's not a const function
if (!it->isConst()) {
/** @todo make sure argument types match */
// avoid endless recursions
if (analyzedFunctions.find(&*it) == analyzedFunctions.end()) {
analyzedFunctions.insert(&*it);
checkReturnPtrThis(scope, &*it, it->arg->link()->next(), it->arg->link()->next()->link(),
checkReturnPtrThis(scope, &*it, it->arg->link()->next(), it->arg->link()->linkAt(1),
analyzedFunctions);
}
// just bail for now
Expand All @@ -1653,7 +1653,7 @@ void CheckClass::checkReturnPtrThis(const Scope *scope, const Function *func, co
else if (!(Token::simpleMatch(tok->next(), "operator= (") ||
Token::simpleMatch(tok->next(), "this . operator= (") ||
(Token::Match(tok->next(), "%type% :: operator= (") &&
tok->next()->str() == scope->className)))
tok->strAt(1) == scope->className)))
operatorEqRetRefThisError(func->token);
}
if (foundReturn) {
Expand Down Expand Up @@ -1853,7 +1853,7 @@ const Token * CheckClass::getIfStmtBodyStart(const Token *tok, const Token *rhs)
case Bool::TRUE:
return top->link()->next();
case Bool::FALSE:
return top->link()->next()->link();
return top->link()->linkAt(1);
}
}
return nullptr;
Expand Down Expand Up @@ -2407,7 +2407,7 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, Member
const Token *lpar = funcTok->next();
if (Token::simpleMatch(lpar, "( ) ("))
lpar = lpar->tokAt(2);
for (const Token* tok = lpar->next(); tok && tok != funcTok->next()->link(); tok = tok->next()) {
for (const Token* tok = lpar->next(); tok && tok != funcTok->linkAt(1); tok = tok->next()) {
if (tok->str() == "(")
tok = tok->link();
else if ((tok->isName() && isMemberVar(scope, tok)) || (tok->isUnaryOp("&") && (tok = tok->astOperand1()) && isMemberVar(scope, tok))) {
Expand Down Expand Up @@ -2848,11 +2848,11 @@ const std::list<const Token *> & CheckClass::getVirtualFunctionCalls(const Funct
continue;

if (tok->previous() &&
tok->previous()->str() == "(") {
tok->strAt(-1) == "(") {
const Token * prev = tok->previous();
if (prev->previous() &&
(mSettings->library.ignorefunction(tok->str())
|| mSettings->library.ignorefunction(prev->previous()->str())))
|| mSettings->library.ignorefunction(prev->strAt(-1))))
continue;
}

Expand Down
12 changes: 6 additions & 6 deletions lib/checkcondition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ void CheckCondition::assignIf()

if (Token::Match(tok->next(), "%num% [&|]")) {
bitop = tok->strAt(2).at(0);
num = MathLib::toBigNumber(tok->next()->str());
num = MathLib::toBigNumber(tok->strAt(1));
} else {
const Token *endToken = Token::findsimplematch(tok, ";");

Expand All @@ -116,7 +116,7 @@ void CheckCondition::assignIf()

if (endToken && Token::Match(endToken->tokAt(-2), "[&|] %num% ;")) {
bitop = endToken->strAt(-2).at(0);
num = MathLib::toBigNumber(endToken->previous()->str());
num = MathLib::toBigNumber(endToken->strAt(-1));
}
}

Expand Down Expand Up @@ -191,7 +191,7 @@ bool CheckCondition::assignIfParseScope(const Token * const assignTok,
ret = true;
if (ret && tok2->str() == ";")
return false;
if (!islocal && Token::Match(tok2, "%name% (") && !Token::simpleMatch(tok2->next()->link(), ") {"))
if (!islocal && Token::Match(tok2, "%name% (") && !Token::simpleMatch(tok2->linkAt(1), ") {"))
return true;
if (Token::Match(tok2, "if|while (")) {
if (!islocal && tok2->str() == "while")
Expand All @@ -205,7 +205,7 @@ bool CheckCondition::assignIfParseScope(const Token * const assignTok,
}

// parse condition
const Token * const end = tok2->next()->link();
const Token * const end = tok2->linkAt(1);
for (; tok2 != end; tok2 = tok2->next()) {
if (Token::Match(tok2, "[(,] &| %varid% [,)]", varid)) {
return true;
Expand All @@ -227,8 +227,8 @@ bool CheckCondition::assignIfParseScope(const Token * const assignTok,

const bool ret1 = assignIfParseScope(assignTok, end->tokAt(2), varid, islocal, bitop, num);
bool ret2 = false;
if (Token::simpleMatch(end->next()->link(), "} else {"))
ret2 = assignIfParseScope(assignTok, end->next()->link()->tokAt(3), varid, islocal, bitop, num);
if (Token::simpleMatch(end->linkAt(1), "} else {"))
ret2 = assignIfParseScope(assignTok, end->linkAt(1)->tokAt(3), varid, islocal, bitop, num);
if (ret1 || ret2)
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/checkexceptionsafety.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ void CheckExceptionSafety::checkRethrowCopy()
if (varid) {
for (const Token* tok = scope.bodyStart->next(); tok && tok != scope.bodyEnd; tok = tok->next()) {
if (Token::simpleMatch(tok, "catch (") && tok->linkAt(1) && tok->linkAt(1)->next()) { // Don't check inner catch - it is handled in another iteration of outer loop.
tok = tok->linkAt(1)->next()->link();
tok = tok->linkAt(1)->linkAt(1);
if (!tok)
break;
} else if (Token::Match(tok, "%varid% .", varid)) {
Expand Down
2 changes: 1 addition & 1 deletion lib/checkfunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ static bool isForwardJump(const Token *gotoToken)
if (!Token::Match(gotoToken, "goto %name% ;"))
return false;
for (const Token *prev = gotoToken; gotoToken; gotoToken = gotoToken->previous()) {
if (Token::Match(prev, "%name% :") && prev->str() == gotoToken->next()->str())
if (Token::Match(prev, "%name% :") && prev->str() == gotoToken->strAt(1))
return true;
if (prev->str() == "{" && prev->scope()->type == Scope::eFunction)
return false;
Expand Down
10 changes: 1 addition & 9 deletions lib/checkinternal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -296,22 +296,14 @@ void CheckInternal::checkRedundantNextPrevious()
continue;
tok = tok->next();

if (Token::Match(tok, "previous ( ) . next|tokAt|strAt|linkAt (") || Token::Match(tok, "next ( ) . previous|tokAt|strAt|linkAt (") ||
if (Token::Match(tok, "previous ( ) . previous|next|tokAt|str|strAt|link|linkAt (") || Token::Match(tok, "next ( ) . previous|next|tokAt|str|strAt|link|linkAt (") ||
(Token::simpleMatch(tok, "tokAt (") && Token::Match(tok->linkAt(1), ") . previous|next|tokAt|strAt|linkAt|str|link ("))) {
const std::string& func1 = tok->str();
const std::string& func2 = tok->linkAt(1)->strAt(2);

if ((func2 == "previous" || func2 == "next" || func2 == "str" || func2 == "link") && tok->linkAt(1)->strAt(4) != ")")
continue;

redundantNextPreviousError(tok, func1, func2);
} else if (Token::Match(tok, "next|previous ( ) . next|previous ( ) . next|previous|linkAt|strAt|link|str (")) {
const std::string& func1 = tok->str();
const std::string& func2 = tok->strAt(8);

if ((func2 == "previous" || func2 == "next" || func2 == "str" || func2 == "link") && tok->strAt(10) != ")")
continue;

redundantNextPreviousError(tok, func1, func2);
}
}
Expand Down
14 changes: 7 additions & 7 deletions lib/checkio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1434,7 +1434,7 @@ CheckIO::ArgumentInfo::ArgumentInfo(const Token * arg, const Settings &settings,
const Token *tok1 = arg->next();
for (; tok1; tok1 = tok1->next()) {
if (tok1->str() == "," || tok1->str() == ")") {
if (tok1->previous()->str() == "]") {
if (tok1->strAt(-1) == "]") {
varTok = tok1->linkAt(-1)->previous();
if (varTok->str() == ")" && varTok->link()->previous()->tokType() == Token::eFunction) {
const Function * function = varTok->link()->previous()->function();
Expand All @@ -1455,7 +1455,7 @@ CheckIO::ArgumentInfo::ArgumentInfo(const Token * arg, const Settings &settings,
}
return;
}
} else if (tok1->previous()->str() == ")" && tok1->linkAt(-1)->previous()->tokType() == Token::eFunction) {
} else if (tok1->strAt(-1) == ")" && tok1->linkAt(-1)->previous()->tokType() == Token::eFunction) {
const Function * function = tok1->linkAt(-1)->previous()->function();
if (function && function->retType && function->retType->isEnumType()) {
if (function->retType->classScope->enumType)
Expand Down Expand Up @@ -1484,9 +1484,9 @@ CheckIO::ArgumentInfo::ArgumentInfo(const Token * arg, const Settings &settings,

// check for some common well known functions
else if (isCPP && ((Token::Match(tok1->previous(), "%var% . size|empty|c_str ( ) [,)]") && isStdContainer(tok1->previous())) ||
(Token::Match(tok1->previous(), "] . size|empty|c_str ( ) [,)]") && isStdContainer(tok1->previous()->link()->previous())))) {
(Token::Match(tok1->previous(), "] . size|empty|c_str ( ) [,)]") && isStdContainer(tok1->linkAt(-1)->previous())))) {
tempToken = new Token(tok1);
if (tok1->next()->str() == "size") {
if (tok1->strAt(1) == "size") {
// size_t is platform dependent
if (settings.platform.sizeof_size_t == 8) {
tempToken->str("long");
Expand All @@ -1502,9 +1502,9 @@ CheckIO::ArgumentInfo::ArgumentInfo(const Token * arg, const Settings &settings,

tempToken->originalName("size_t");
tempToken->isUnsigned(true);
} else if (tok1->next()->str() == "empty") {
} else if (tok1->strAt(1) == "empty") {
tempToken->str("bool");
} else if (tok1->next()->str() == "c_str") {
} else if (tok1->strAt(1) == "c_str") {
tempToken->str("const");
tempToken->insertToken("*");
if (typeToken->strAt(2) == "string")
Expand Down Expand Up @@ -1534,7 +1534,7 @@ CheckIO::ArgumentInfo::ArgumentInfo(const Token * arg, const Settings &settings,

if (varTok) {
variableInfo = varTok->variable();
element = tok1->previous()->str() == "]";
element = tok1->strAt(-1) == "]";

// look for std::vector operator [] and use template type as return type
if (variableInfo) {
Expand Down
Loading

0 comments on commit 32fefc6

Please sign in to comment.