Skip to content

Commit 6734571

Browse files
danmardanmar
committed
Refactoring: Create FwdAnalysis class in astutils
1 parent 61878c5 commit 6734571

4 files changed

Lines changed: 162 additions & 156 deletions

File tree

‎lib/astutils.cpp‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,3 +1037,135 @@ bool isLikelyStreamRead(bool cpp, const Token *op)
10371037
return (!parent->astOperand1()->valueType() || !parent->astOperand1()->valueType()->isIntegral());
10381038
}
10391039

1040+
1041+
static bool nonLocal(const Variable* var)
1042+
{
1043+
return !var || (!var->isLocal() && !var->isArgument()) || var->isStatic() || var->isReference();
1044+
}
1045+
1046+
static bool hasFunctionCall(const Token *tok)
1047+
{
1048+
if (!tok)
1049+
return false;
1050+
if (Token::Match(tok, "%name% ("))
1051+
// todo, const/pure function?
1052+
return true;
1053+
return hasFunctionCall(tok->astOperand1()) || hasFunctionCall(tok->astOperand2());
1054+
}
1055+
1056+
FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *assign1, const Token *startToken, const Token *endToken)
1057+
{
1058+
// all variable ids in assign1 LHS.
1059+
std::set<unsigned int> assign1LhsVarIds;
1060+
bool local = true;
1061+
visitAstNodes(assign1->astOperand1(),
1062+
[&](const Token *tok) {
1063+
if (tok->varId() > 0) {
1064+
assign1LhsVarIds.insert(tok->varId());
1065+
if (!Token::simpleMatch(tok->previous(), "."))
1066+
local &= !nonLocal(tok->variable());
1067+
}
1068+
return ChildrenToVisit::op1_and_op2;
1069+
});
1070+
1071+
// Parse the given tokens
1072+
for (const Token *tok = startToken; tok != endToken; tok = tok->next()) {
1073+
if (Token::simpleMatch(tok, "try {")) {
1074+
// TODO: handle try
1075+
return Result(Result::Type::BAILOUT);
1076+
}
1077+
1078+
if (tok->str() == "}" && (tok->scope()->type == Scope::eFor || tok->scope()->type == Scope::eWhile)) {
1079+
// TODO: handle loops better
1080+
return Result(Result::Type::BAILOUT);
1081+
}
1082+
1083+
if (Token::simpleMatch(tok, "break ;")) {
1084+
return Result(Result::Type::BREAK, tok);
1085+
}
1086+
1087+
if (Token::Match(tok, "continue|return|throw|goto")) {
1088+
// TODO: Handle these better
1089+
return Result(Result::Type::RETURN);
1090+
}
1091+
1092+
if (Token::simpleMatch(tok, "else {"))
1093+
tok = tok->linkAt(1);
1094+
1095+
if (Token::simpleMatch(tok, "asm (")) {
1096+
return Result(Result::Type::BAILOUT);
1097+
}
1098+
1099+
if (!local && Token::Match(tok, "%name% (") && !Token::simpleMatch(tok->linkAt(1), ") {")) {
1100+
// TODO: this is a quick bailout
1101+
return Result(Result::Type::BAILOUT);
1102+
}
1103+
1104+
if (assign1LhsVarIds.find(tok->varId()) != assign1LhsVarIds.end()) {
1105+
const Token *parent = tok;
1106+
while (Token::Match(parent->astParent(), ".|::|["))
1107+
parent = parent->astParent();
1108+
if (Token::simpleMatch(parent->astParent(), "=") && parent == parent->astParent()->astOperand1()) {
1109+
if (!local && hasFunctionCall(parent->astParent()->astOperand2())) {
1110+
// TODO: this is a quick bailout
1111+
return Result(Result::Type::BAILOUT);
1112+
}
1113+
if (hasOperand(parent->astParent()->astOperand2(), assign1->astOperand1())) {
1114+
return Result(Result::Type::READ);
1115+
}
1116+
const bool reassign = isSameExpression(mCpp, false, assign1->astOperand1(), parent, mLibrary, false, false, nullptr);
1117+
if (reassign)
1118+
return Result(Result::Type::WRITE, parent->astParent());
1119+
return Result(Result::Type::READ);
1120+
} else {
1121+
// TODO: this is a quick bailout
1122+
return Result(Result::Type::BAILOUT);
1123+
}
1124+
}
1125+
1126+
if (Token::Match(tok, ") {")) {
1127+
const Result &result1 = checkRecursive(assign1, tok->tokAt(2), tok->linkAt(1));
1128+
if (result1.type == Result::Type::READ || result1.type == Result::Type::BAILOUT)
1129+
return result1;
1130+
if (Token::simpleMatch(tok->linkAt(1), "} else {")) {
1131+
const Token *elseStart = tok->linkAt(1)->tokAt(2);
1132+
const Result &result2 = checkRecursive(assign1, elseStart, elseStart->link());
1133+
if (result2.type == Result::Type::READ || result2.type == Result::Type::BAILOUT)
1134+
return result2;
1135+
if (result1.type == Result::Type::WRITE && result2.type == Result::Type::WRITE)
1136+
return result1;
1137+
tok = elseStart->link();
1138+
} else {
1139+
tok = tok->linkAt(1);
1140+
}
1141+
}
1142+
}
1143+
1144+
return Result(Result::Type::NONE);
1145+
}
1146+
1147+
FwdAnalysis::Result FwdAnalysis::check(const Token *assign1, const Token *startToken, const Token *endToken)
1148+
{
1149+
Result result = checkRecursive(assign1, startToken, endToken);
1150+
1151+
// Break => continue checking in outer scope
1152+
while (result.type == FwdAnalysis::Result::Type::BREAK) {
1153+
const Scope *s = result.token->scope();
1154+
while (s->type == Scope::eIf)
1155+
s = s->nestedIn;
1156+
if (s->type != Scope::eSwitch)
1157+
break;
1158+
result = checkRecursive(assign1, s->bodyEnd->next(), endToken);
1159+
}
1160+
1161+
return result;
1162+
}
1163+
1164+
bool FwdAnalysis::hasOperand(const Token *tok, const Token *lhs) const
1165+
{
1166+
if (!tok)
1167+
return false;
1168+
if (isSameExpression(mCpp, false, tok, lhs, mLibrary, false, false, nullptr))
1169+
return true;
1170+
return hasOperand(tok->astOperand1(), lhs) || hasOperand(tok->astOperand2(), lhs);
1171+
}

‎lib/astutils.h‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,4 +160,28 @@ const Token *findLambdaEndToken(const Token *first);
160160
*/
161161
bool isLikelyStreamRead(bool cpp, const Token *op);
162162

163+
class FwdAnalysis {
164+
public:
165+
FwdAnalysis(bool cpp, const Library &library) : mCpp(cpp), mLibrary(library) {}
166+
167+
/** Result of forward analysis */
168+
struct Result {
169+
enum class Type { NONE, READ, WRITE, BREAK, RETURN, BAILOUT } type;
170+
Result(Type type) : type(type), token(nullptr) {}
171+
Result(Type type, const Token *token) : type(type), token(token) {}
172+
const Token *token;
173+
};
174+
175+
/** forward analysis */
176+
struct Result check(const Token *assign1, const Token *startToken, const Token *endToken);
177+
178+
bool hasOperand(const Token *tok, const Token *lhs) const;
179+
180+
private:
181+
struct Result checkRecursive(const Token *assign1, const Token *startToken, const Token *endToken);
182+
183+
const bool mCpp;
184+
const Library &mLibrary;
185+
};
186+
163187
#endif // astutilsH

‎lib/checkother.cpp‎

Lines changed: 6 additions & 147 deletions
Original file line numberDiff line numberDiff line change
@@ -412,132 +412,6 @@ void CheckOther::checkPipeParameterSizeError(const Token *tok, const std::string
412412
// Detect redundant assignments: x = 0; x = 4;
413413
//---------------------------------------------------------------------------
414414

415-
static bool nonLocal(const Variable* var)
416-
{
417-
return !var || (!var->isLocal() && !var->isArgument()) || var->isStatic() || var->isReference();
418-
}
419-
420-
static void eraseMemberAssignments(const unsigned int varId, const std::map<unsigned int, std::set<unsigned int> > &membervars, std::map<unsigned int, const Token*> &varAssignments)
421-
{
422-
const std::map<unsigned int, std::set<unsigned int> >::const_iterator it = membervars.find(varId);
423-
if (it != membervars.end()) {
424-
const std::set<unsigned int>& vars = it->second;
425-
for (unsigned int var : vars) {
426-
varAssignments.erase(var);
427-
if (var != varId)
428-
eraseMemberAssignments(var, membervars, varAssignments);
429-
}
430-
}
431-
}
432-
433-
static bool hasFunctionCall(const Token *tok)
434-
{
435-
if (!tok)
436-
return false;
437-
if (Token::Match(tok, "%name% ("))
438-
// todo, const/pure function?
439-
return true;
440-
return hasFunctionCall(tok->astOperand1()) || hasFunctionCall(tok->astOperand2());
441-
}
442-
443-
static bool hasOperand(const Token *tok, const Token *lhs, bool cpp, const Library &library)
444-
{
445-
if (!tok)
446-
return false;
447-
if (isSameExpression(cpp, false, tok, lhs, library, false, false, nullptr))
448-
return true;
449-
return hasOperand(tok->astOperand1(), lhs, cpp, library) || hasOperand(tok->astOperand2(), lhs, cpp, library);
450-
}
451-
452-
struct CheckOther::ScopeResult CheckOther::checkRedundantAssignmentRecursive(const Token *assign1, const Token *startToken, const Token *endToken) const {
453-
// all variable ids in assign1 LHS.
454-
std::set<unsigned int> assign1LhsVarIds;
455-
bool local = true;
456-
visitAstNodes(assign1->astOperand1(),
457-
[&](const Token *tok) {
458-
if (tok->varId() > 0) {
459-
assign1LhsVarIds.insert(tok->varId());
460-
if (!Token::simpleMatch(tok->previous(), "."))
461-
local &= !nonLocal(tok->variable());
462-
}
463-
return ChildrenToVisit::op1_and_op2;
464-
});
465-
466-
// Parse the given tokens
467-
for (const Token *tok = startToken; tok != endToken; tok = tok->next()) {
468-
if (Token::simpleMatch(tok, "try {")) {
469-
// TODO: handle try
470-
return ScopeResult(ScopeResult::BAILOUT);
471-
}
472-
473-
if (tok->str() == "}" && (tok->scope()->type == Scope::eFor || tok->scope()->type == Scope::eWhile)) {
474-
// TODO: handle loops better
475-
return ScopeResult(ScopeResult::BAILOUT);
476-
}
477-
478-
if (Token::simpleMatch(tok, "break ;")) {
479-
return ScopeResult(ScopeResult::BREAK, tok);
480-
}
481-
482-
if (Token::Match(tok, "continue|return|throw|goto")) {
483-
// TODO: Handle these better
484-
return ScopeResult(ScopeResult::RETURN);
485-
}
486-
487-
if (Token::simpleMatch(tok, "else {"))
488-
tok = tok->linkAt(1);
489-
490-
if (Token::simpleMatch(tok, "asm (")) {
491-
return ScopeResult(ScopeResult::BAILOUT);
492-
}
493-
494-
if (!local && Token::Match(tok, "%name% (") && !Token::simpleMatch(tok->linkAt(1), ") {")) {
495-
// TODO: this is a quick bailout
496-
return ScopeResult(ScopeResult::BAILOUT);
497-
}
498-
499-
if (assign1LhsVarIds.find(tok->varId()) != assign1LhsVarIds.end()) {
500-
const Token *parent = tok;
501-
while (Token::Match(parent->astParent(), ".|::|["))
502-
parent = parent->astParent();
503-
if (Token::simpleMatch(parent->astParent(), "=") && parent == parent->astParent()->astOperand1()) {
504-
if (!local && hasFunctionCall(parent->astParent()->astOperand2())) {
505-
// TODO: this is a quick bailout
506-
return ScopeResult(ScopeResult::BAILOUT);
507-
}
508-
if (hasOperand(parent->astParent()->astOperand2(), assign1->astOperand1(), mTokenizer->isCPP(), mSettings->library)) {
509-
return ScopeResult(ScopeResult::READ);
510-
}
511-
const bool reassign = isSameExpression(mTokenizer->isCPP(), false, assign1->astOperand1(), parent, mSettings->library, false, false, nullptr);
512-
if (reassign)
513-
return ScopeResult(ScopeResult::WRITE, parent->astParent());
514-
return ScopeResult(ScopeResult::READ);
515-
} else {
516-
// TODO: this is a quick bailout
517-
return ScopeResult(ScopeResult::BAILOUT);
518-
}
519-
}
520-
521-
if (Token::Match(tok, ") {")) {
522-
const ScopeResult &result1 = checkRedundantAssignmentRecursive(assign1, tok->tokAt(2), tok->linkAt(1));
523-
if (result1.type == ScopeResult::READ || result1.type == ScopeResult::BAILOUT)
524-
return result1;
525-
if (Token::simpleMatch(tok->linkAt(1), "} else {")) {
526-
const Token *elseStart = tok->linkAt(1)->tokAt(2);
527-
const ScopeResult &result2 = checkRedundantAssignmentRecursive(assign1, elseStart, elseStart->link());
528-
if (result2.type == ScopeResult::READ || result2.type == ScopeResult::BAILOUT)
529-
return result2;
530-
if (result1.type == ScopeResult::WRITE && result2.type == ScopeResult::WRITE)
531-
return result1;
532-
tok = elseStart->link();
533-
} else {
534-
tok = tok->linkAt(1);
535-
}
536-
}
537-
}
538-
return ScopeResult(ScopeResult::NONE);
539-
}
540-
541415
void CheckOther::checkRedundantAssignment()
542416
{
543417
if (!mSettings->isEnabled(Settings::STYLE))
@@ -586,9 +460,6 @@ void CheckOther::checkRedundantAssignment()
586460
// todo: check static variables
587461
continue;
588462

589-
if (hasOperand(tok->astOperand2(), tok->astOperand1(), mTokenizer->isCPP(), mSettings->library))
590-
continue;
591-
592463
// If there is a custom assignment operator => this is inconclusive
593464
bool inconclusive = false;
594465
if (mTokenizer->isCPP() && tok->astOperand1()->valueType() && tok->astOperand1()->valueType()->typeScope) {
@@ -603,6 +474,10 @@ void CheckOther::checkRedundantAssignment()
603474
if (inconclusive && !mSettings->inconclusive)
604475
continue;
605476

477+
FwdAnalysis fwdAnalysis(mTokenizer->isCPP(), mSettings->library);
478+
if (fwdAnalysis.hasOperand(tok->astOperand2(), tok->astOperand1()))
479+
continue;
480+
606481
// Is there a redundant assignment?
607482
const Token *start;
608483
if (tok->isAssignmentOp())
@@ -611,25 +486,9 @@ void CheckOther::checkRedundantAssignment()
611486
start = tok->findExpressionStartEndTokens().second->next();
612487

613488
// Get next assignment..
614-
ScopeResult nextAssign(ScopeResult::NONE);
615-
while (true) {
616-
nextAssign = checkRedundantAssignmentRecursive(tok, start, scope->bodyEnd);
617-
618-
// Break => continue checking in outer scope
619-
if (nextAssign.type == ScopeResult::BREAK) {
620-
const Scope *s = nextAssign.token->scope();
621-
while (s->type == Scope::eIf)
622-
s = s->nestedIn;
623-
if (s->type == Scope::eSwitch) {
624-
start = s->bodyEnd->next();
625-
continue;
626-
}
627-
}
628-
629-
break;
630-
}
489+
FwdAnalysis::Result nextAssign = fwdAnalysis.check(tok, start, scope->bodyEnd);
631490

632-
if (nextAssign.type != ScopeResult::WRITE || !nextAssign.token)
491+
if (nextAssign.type != FwdAnalysis::Result::Type::WRITE || !nextAssign.token)
633492
continue;
634493

635494
// there is redundant assignment. Is there a case between the assignments?

‎lib/checkother.h‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -217,15 +217,6 @@ class CPPCHECKLIB CheckOther : public Check {
217217

218218
private:
219219

220-
struct ScopeResult {
221-
enum Type { NONE, READ, WRITE, BREAK, RETURN, BAILOUT } type;
222-
ScopeResult(Type type) : type(type), token(nullptr) {}
223-
ScopeResult(Type type, const Token *token) : type(type), token(token) {}
224-
const Token *token;
225-
};
226-
/** Recursively check for redundant assignments. */
227-
struct ScopeResult checkRedundantAssignmentRecursive(const Token *assign1, const Token *startToken, const Token *endToken) const;
228-
229220
// Error messages..
230221
void checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* tok, const std::string &functionName, const std::string &varName, const bool result);
231222
void checkCastIntToCharAndBackError(const Token *tok, const std::string &strFunctionName);

0 commit comments

Comments
 (0)