Skip to content

Commit a2fea3d

Browse files
chrchr-githubchrchr-github
authored
Fix #11083 FP knownConditionTrueFalse with reassigned pointer (danmar#4717)
1 parent 50d297b commit a2fea3d

File tree

4 files changed

+57
-33
lines changed

4 files changed

+57
-33
lines changed

‎lib/astutils.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -916,9 +916,10 @@ static const Token * getVariableInitExpression(const Variable * var)
916916
return varDeclEndToken->astOperand2();
917917
}
918918

919-
static bool isInLoopCondition(const Token * tok)
919+
const Token* isInLoopCondition(const Token* tok)
920920
{
921-
return Token::Match(tok->astTop()->previous(), "for|while (");
921+
const Token* top = tok->astTop();
922+
return top && Token::Match(top->previous(), "for|while (") ? top : nullptr;
922923
}
923924

924925
/// If tok2 comes after tok1

‎lib/astutils.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,8 @@ bool isEqualKnownValue(const Token * const tok1, const Token * const tok2);
246246

247247
bool isStructuredBindingVariable(const Variable* var);
248248

249+
const Token* isInLoopCondition(const Token* tok);
250+
249251
/**
250252
* Is token used a boolean, that is to say cast to a bool, or used as a condition in a if/while/for
251253
*/

‎lib/checkother.cpp‎

Lines changed: 36 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2375,6 +2375,8 @@ void CheckOther::checkDuplicateExpression()
23752375
std::list<const Function*> constFunctions;
23762376
getConstFunctions(symbolDatabase, constFunctions);
23772377

2378+
const bool cpp = mTokenizer->isCPP();
2379+
23782380
for (const Scope *scope : symbolDatabase->functionScopes) {
23792381
for (const Token *tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) {
23802382
if (tok->str() == "=" && Token::Match(tok->astOperand1(), "%var%")) {
@@ -2399,8 +2401,8 @@ void CheckOther::checkDuplicateExpression()
23992401
Token::Match(tok->astOperand2()->previous(), "%name% (")
24002402
) &&
24012403
tok->next()->tokType() != Token::eType &&
2402-
isSameExpression(mTokenizer->isCPP(), true, tok->next(), nextAssign->next(), mSettings->library, true, false) &&
2403-
isSameExpression(mTokenizer->isCPP(), true, tok->astOperand2(), nextAssign->astOperand2(), mSettings->library, true, false) &&
2404+
isSameExpression(cpp, true, tok->next(), nextAssign->next(), mSettings->library, true, false) &&
2405+
isSameExpression(cpp, true, tok->astOperand2(), nextAssign->astOperand2(), mSettings->library, true, false) &&
24042406
tok->astOperand2()->expressionString() == nextAssign->astOperand2()->expressionString()) {
24052407
bool differentDomain = false;
24062408
const Scope * varScope = var1->scope() ? var1->scope() : scope;
@@ -2414,7 +2416,7 @@ void CheckOther::checkDuplicateExpression()
24142416

24152417
if (assignTok->astOperand1()->varId() != var1->varId() &&
24162418
assignTok->astOperand1()->varId() != var2->varId() &&
2417-
!isSameExpression(mTokenizer->isCPP(),
2419+
!isSameExpression(cpp,
24182420
true,
24192421
tok->astOperand2(),
24202422
assignTok->astOperand1(),
@@ -2424,7 +2426,7 @@ void CheckOther::checkDuplicateExpression()
24242426
continue;
24252427
if (assignTok->astOperand2()->varId() != var1->varId() &&
24262428
assignTok->astOperand2()->varId() != var2->varId() &&
2427-
!isSameExpression(mTokenizer->isCPP(),
2429+
!isSameExpression(cpp,
24282430
true,
24292431
tok->astOperand2(),
24302432
assignTok->astOperand2(),
@@ -2455,70 +2457,73 @@ void CheckOther::checkDuplicateExpression()
24552457
const bool pointerDereference = (tok->astOperand1() && tok->astOperand1()->isUnaryOp("*")) ||
24562458
(tok->astOperand2() && tok->astOperand2()->isUnaryOp("*"));
24572459
const bool followVar = (!isConstVarExpression(tok) || Token::Match(tok, "%comp%|%oror%|&&")) && !pointerDereference;
2458-
if (isSameExpression(mTokenizer->isCPP(),
2460+
if (isSameExpression(cpp,
24592461
true,
24602462
tok->astOperand1(),
24612463
tok->astOperand2(),
24622464
mSettings->library,
24632465
true,
24642466
followVar,
24652467
&errorPath)) {
2466-
if (isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) {
2467-
const bool assignment = tok->str() == "=";
2468-
if (assignment && warningEnabled)
2469-
selfAssignmentError(tok, tok->astOperand1()->expressionString());
2470-
else if (styleEnabled) {
2471-
if (mTokenizer->isCPP() && mSettings->standards.cpp >= Standards::CPP11 && tok->str() == "==") {
2472-
const Token* parent = tok->astParent();
2473-
while (parent && parent->astParent()) {
2474-
parent = parent->astParent();
2475-
}
2476-
if (parent && parent->previous() && parent->previous()->str() == "static_assert") {
2477-
continue;
2468+
if (isWithoutSideEffects(cpp, tok->astOperand1())) {
2469+
const Token* loopTok = isInLoopCondition(tok);
2470+
if (!loopTok || !isExpressionChanged(tok, tok, loopTok->link()->next()->link(), mSettings, cpp)) {
2471+
const bool assignment = tok->str() == "=";
2472+
if (assignment && warningEnabled)
2473+
selfAssignmentError(tok, tok->astOperand1()->expressionString());
2474+
else if (styleEnabled) {
2475+
if (cpp && mSettings->standards.cpp >= Standards::CPP11 && tok->str() == "==") {
2476+
const Token* parent = tok->astParent();
2477+
while (parent && parent->astParent()) {
2478+
parent = parent->astParent();
2479+
}
2480+
if (parent && parent->previous() && parent->previous()->str() == "static_assert") {
2481+
continue;
2482+
}
24782483
}
2484+
duplicateExpressionError(tok->astOperand1(), tok->astOperand2(), tok, errorPath);
24792485
}
2480-
duplicateExpressionError(tok->astOperand1(), tok->astOperand2(), tok, errorPath);
24812486
}
24822487
}
24832488
} else if (tok->str() == "=" && Token::simpleMatch(tok->astOperand2(), "=") &&
2484-
isSameExpression(mTokenizer->isCPP(),
2489+
isSameExpression(cpp,
24852490
false,
24862491
tok->astOperand1(),
24872492
tok->astOperand2()->astOperand1(),
24882493
mSettings->library,
24892494
true,
24902495
false)) {
2491-
if (warningEnabled && isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) {
2496+
if (warningEnabled && isWithoutSideEffects(cpp, tok->astOperand1())) {
24922497
selfAssignmentError(tok, tok->astOperand1()->expressionString());
24932498
}
24942499
} else if (styleEnabled &&
2495-
isOppositeExpression(mTokenizer->isCPP(),
2500+
isOppositeExpression(cpp,
24962501
tok->astOperand1(),
24972502
tok->astOperand2(),
24982503
mSettings->library,
24992504
false,
25002505
true,
25012506
&errorPath) &&
25022507
!Token::Match(tok, "=|-|-=|/|/=") &&
2503-
isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) {
2508+
isWithoutSideEffects(cpp, tok->astOperand1())) {
25042509
oppositeExpressionError(tok, errorPath);
25052510
} else if (!Token::Match(tok, "[-/%]")) { // These operators are not associative
25062511
if (styleEnabled && tok->astOperand2() && tok->str() == tok->astOperand1()->str() &&
2507-
isSameExpression(mTokenizer->isCPP(),
2512+
isSameExpression(cpp,
25082513
true,
25092514
tok->astOperand2(),
25102515
tok->astOperand1()->astOperand2(),
25112516
mSettings->library,
25122517
true,
25132518
followVar,
25142519
&errorPath) &&
2515-
isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand2()))
2520+
isWithoutSideEffects(cpp, tok->astOperand2()))
25162521
duplicateExpressionError(tok->astOperand2(), tok->astOperand1()->astOperand2(), tok, errorPath);
2517-
else if (tok->astOperand2() && isConstExpression(tok->astOperand1(), mSettings->library, mTokenizer->isCPP())) {
2522+
else if (tok->astOperand2() && isConstExpression(tok->astOperand1(), mSettings->library, cpp)) {
25182523
auto checkDuplicate = [&](const Token* exp1, const Token* exp2, const Token* ast1) {
2519-
if (isSameExpression(mTokenizer->isCPP(), true, exp1, exp2, mSettings->library, true, true, &errorPath) &&
2520-
isWithoutSideEffects(mTokenizer->isCPP(), exp1) &&
2521-
isWithoutSideEffects(mTokenizer->isCPP(), ast1->astOperand2()))
2524+
if (isSameExpression(cpp, true, exp1, exp2, mSettings->library, true, true, &errorPath) &&
2525+
isWithoutSideEffects(cpp, exp1) &&
2526+
isWithoutSideEffects(cpp, ast1->astOperand2()))
25222527
duplicateExpressionError(exp1, exp2, tok, errorPath, /*hasMultipleExpr*/ true);
25232528
};
25242529
const Token *ast1 = tok->astOperand1();
@@ -2532,10 +2537,10 @@ void CheckOther::checkDuplicateExpression()
25322537
}
25332538
} else if (styleEnabled && tok->astOperand1() && tok->astOperand2() && tok->str() == ":" && tok->astParent() && tok->astParent()->str() == "?") {
25342539
if (!tok->astOperand1()->values().empty() && !tok->astOperand2()->values().empty() && isEqualKnownValue(tok->astOperand1(), tok->astOperand2()) &&
2535-
!isVariableChanged(tok->astParent(), /*indirect*/ 0, mSettings, mTokenizer->isCPP()) &&
2536-
isConstStatement(tok->astOperand1(), mTokenizer->isCPP()) && isConstStatement(tok->astOperand2(), mTokenizer->isCPP()))
2540+
!isVariableChanged(tok->astParent(), /*indirect*/ 0, mSettings, cpp) &&
2541+
isConstStatement(tok->astOperand1(), cpp) && isConstStatement(tok->astOperand2(), cpp))
25372542
duplicateValueTernaryError(tok);
2538-
else if (isSameExpression(mTokenizer->isCPP(), true, tok->astOperand1(), tok->astOperand2(), mSettings->library, false, true, &errorPath))
2543+
else if (isSameExpression(cpp, true, tok->astOperand1(), tok->astOperand2(), mSettings->library, false, true, &errorPath))
25392544
duplicateExpressionTernaryError(tok, errorPath);
25402545
}
25412546
}

‎test/testother.cpp‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6414,6 +6414,22 @@ class TestOther : public TestFixture {
64146414
" }\n"
64156415
"}");
64166416
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) The comparison 'a != 1' is always false.\n", errout.str());
6417+
6418+
check("struct T {\n" // #11083
6419+
" std::string m;\n"
6420+
" const std::string & str() const { return m; }\n"
6421+
" T* next();\n"
6422+
"};\n"
6423+
"void f(T* t) {\n"
6424+
" const std::string& s = t->str();\n"
6425+
" while (t && t->str() == s)\n"
6426+
" t = t->next();\n"
6427+
" do {\n"
6428+
" t = t->next();\n"
6429+
" } while (t && t->str() == s);\n"
6430+
" for (; t && t->str() == s; t = t->next());\n"
6431+
"}\n");
6432+
ASSERT_EQUALS("", errout.str());
64176433
}
64186434

64196435
void duplicateExpressionTernary() { // #6391

0 commit comments

Comments
 (0)