Skip to content

Commit

Permalink
Another fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@olabetskyi
olabetskyi committed May 13, 2024
1 parent 056140d commit 013903d
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 105 deletions.
158 changes: 56 additions & 102 deletions lib/checkother.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3265,90 +3265,65 @@ void CheckOther::unusedLabelError(const Token* tok, bool inSwitch, bool hasIfdef
Certainty::normal);
}

void CheckOther::checkEvaluationOrderPre11()
static bool checkEvaluationOrderPre11(const Token * tok, const Token * tok2, const Token * parent, const Settings & settings, bool & selfAssignmentError)
{
logChecker("CheckOther::checkEvaluationOrder");
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
for (const Scope * functionScope : symbolDatabase->functionScopes) {
for (const Token* tok = functionScope->bodyStart; tok != functionScope->bodyEnd; tok = tok->next()) {
if (tok->tokType() != Token::eIncDecOp && !tok->isAssignmentOp())
continue;
if (!tok->astOperand1())
continue;
for (const Token *tok2 = tok;; tok2 = tok2->astParent()) {
// If ast parent is a sequence point then break
const Token * const parent = tok2->astParent();
if (!parent)
break;
if (Token::Match(parent, "%oror%|&&|?|:|;"))
break;
if (parent->str() == ",") {
const Token *par = parent;
while (Token::simpleMatch(par,","))
par = par->astParent();
// not function or in a while clause => break
if (!(par && par->str() == "(" && par->astOperand2() && par->strAt(-1) != "while"))
break;
// control flow (if|while|etc) => break
if (Token::simpleMatch(par->link(),") {"))
break;
// sequence point in function argument: dostuff((1,2),3) => break
par = par->next();
while (par && (par->previous() != parent))
par = par->nextArgument();
if (!par)
break;
}
if (parent->str() == "(" && parent->astOperand2())
break;

// self assignment..
if (tok2 == tok &&
tok->str() == "=" &&
parent->str() == "=" &&
isSameExpression(false, tok->astOperand1(), parent->astOperand1(), *mSettings, true, false)) {
if (mSettings->severity.isEnabled(Severity::warning) &&
isSameExpression(true, tok->astOperand1(), parent->astOperand1(), *mSettings, true, false))
selfAssignmentError(parent, tok->astOperand1()->expressionString());
break;
}

// Is expression used?
bool foundError = false;
visitAstNodes((parent->astOperand1() != tok2) ? parent->astOperand1() : parent->astOperand2(),
[&](const Token *tok3) {
if (tok3->str() == "&" && !tok3->astOperand2())
return ChildrenToVisit::none; // don't handle address-of for now
if (tok3->str() == "(" && Token::simpleMatch(tok3->previous(), "sizeof"))
return ChildrenToVisit::none; // don't care about sizeof usage
if (isSameExpression(false, tok->astOperand1(), tok3, *mSettings, true, false))
foundError = true;
return foundError ? ChildrenToVisit::done : ChildrenToVisit::op1_and_op2;
});

if (foundError) {
unknownEvaluationOrder(parent);
break;
}
}
}
// self assignment..
if (tok2 == tok && tok->str() == "=" && parent->str() == "=" && isSameExpression(false, tok->astOperand1(), parent->astOperand1(), settings, true, false)) {
if (settings.severity.isEnabled(Severity::warning) && isSameExpression(true, tok->astOperand1(), parent->astOperand1(), settings, true, false))
selfAssignmentError = true;
return false;
}
// Is expression used?
bool foundError = false;
visitAstNodes((parent->astOperand1() != tok2) ? parent->astOperand1() : parent->astOperand2(), [&](const Token *tok3) {
if (tok3->str() == "&" && !tok3->astOperand2())
return ChildrenToVisit::none; // don't handle address-of for now
if (tok3->str() == "(" && Token::simpleMatch(tok3->previous(), "sizeof"))
return ChildrenToVisit::none; // don't care about sizeof usage
if (isSameExpression(false, tok->astOperand1(), tok3, settings, true, false))
foundError = true;
return foundError ? ChildrenToVisit::done : ChildrenToVisit::op1_and_op2;
});

return foundError;
}

void CheckOther::checkEvaluationOrderPost11()
static bool checkEvaluationOrderPost17(const Token * tok, const Token * tok2, const Token * parent, const Settings & settings, bool & foundUnspecified)
{
// TODO
if (tok->isAssignmentOp())
return false;
bool foundUndefined{false};
visitAstNodes((parent->astOperand1() != tok2) ? parent->astOperand1() : parent->astOperand2(), [&](const Token *tok3) {
if (tok3->str() == "&" && !tok3->astOperand2())
return ChildrenToVisit::none; // don't handle address-of for now
if (tok3->str() == "(" && Token::simpleMatch(tok3->previous(), "sizeof"))
return ChildrenToVisit::none; // don't care about sizeof usage
if (isSameExpression(false, tok->astOperand1(), tok3, settings, true, false) && parent->isArithmeticalOp() && parent->isBinaryOp())
foundUndefined = true;
if (tok3->tokType() == Token::eIncDecOp && isSameExpression(false, tok->astOperand1(), tok3->astOperand1(), settings, true, false)) {
if (parent->isArithmeticalOp() && parent->isBinaryOp())
foundUndefined = true;
else
foundUnspecified = true;
}
return (foundUndefined || foundUnspecified) ? ChildrenToVisit::done : ChildrenToVisit::op1_and_op2;
});

return foundUndefined || foundUnspecified;
}

void CheckOther::checkEvaluationOrderPost17()
void CheckOther::checkEvaluationOrder()
{
logChecker("CheckOther::checkEvaluationOrder");
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
for (const Scope * functionScope : symbolDatabase->functionScopes) {
for (const Token* tok = functionScope->bodyStart; tok != functionScope->bodyEnd; tok = tok->next()) {
if (tok->tokType() != Token::eIncDecOp || !tok->astOperand1())
if (!tok->isIncDecOp() && !tok->isAssignmentOp())
continue;
if (!tok->astOperand1())
continue;
for (const Token *tok2 = tok;; tok2 = tok2->astParent()) {
// If ast parent is a sequence point then break
const Token * const parent = tok2->astParent();
if (!parent)
break;
Expand All @@ -3374,46 +3349,25 @@ void CheckOther::checkEvaluationOrderPost17()
if (parent->str() == "(" && parent->astOperand2())
break;

// Is expression used?
bool foundUndefined{false}, foundUnspecified{false};
visitAstNodes((parent->astOperand1() != tok2) ? parent->astOperand1() : parent->astOperand2(),
[&](const Token *tok3) {
if (tok3->str() == "&" && !tok3->astOperand2())
return ChildrenToVisit::none; // don't handle address-of for now
if (tok3->str() == "(" && Token::simpleMatch(tok3->previous(), "sizeof"))
return ChildrenToVisit::none; // don't care about sizeof usage
if (isSameExpression(false, tok->astOperand1(), tok3, *mSettings, true, false) && parent->isArithmeticalOp() && parent->isBinaryOp())
foundUndefined = true;
if (tok3->tokType() == Token::eIncDecOp && isSameExpression(false, tok->astOperand1(), tok3->astOperand1(), *mSettings, true, false)) {
if (parent->isArithmeticalOp() && parent->isBinaryOp())
foundUndefined = true;
else
foundUnspecified = true;
}
return (foundUndefined || foundUnspecified) ? ChildrenToVisit::done : ChildrenToVisit::op1_and_op2;
});
bool foundError{false}, foundUnspecified{false}, bSelfAssignmentError{false};
if (mTokenizer->isCPP() && mSettings->standards.cpp >= Standards::CPP17)
foundError = checkEvaluationOrderPost17(tok, tok2, parent, *mSettings, foundUnspecified);
else
foundError = checkEvaluationOrderPre11(tok, tok2, parent, *mSettings, bSelfAssignmentError);

if (foundUndefined || foundUnspecified) {
unknownEvaluationOrder(parent, !foundUndefined);
if (foundError) {
unknownEvaluationOrder(parent, foundUnspecified);
break;
}
if (bSelfAssignmentError) {
selfAssignmentError(parent, tok->astOperand1()->expressionString());
break;
}
}
}
}
}

void CheckOther::checkEvaluationOrder()
{
if (mTokenizer->isCPP() && mSettings->standards.cpp >= Standards::CPP11) {
if (mSettings->standards.cpp >= Standards::CPP17)
checkEvaluationOrderPost17();
else
checkEvaluationOrderPost11();
}
else
checkEvaluationOrderPre11();
}

void CheckOther::unknownEvaluationOrder(const Token* tok, bool isUnspecifiedBehavior)
{
isUnspecifiedBehavior ?
Expand Down
3 changes: 0 additions & 3 deletions lib/checkother.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,6 @@ class CPPCHECKLIB CheckOther : public Check {

/** @brief %Check for expression that depends on order of evaluation of side effects */
void checkEvaluationOrder();
void checkEvaluationOrderPost17();
void checkEvaluationOrderPost11();
void checkEvaluationOrderPre11();

/** @brief %Check for access of moved or forwarded variable */
void checkAccessOfMovedVariable();
Expand Down

0 comments on commit 013903d

Please sign in to comment.