Skip to content

Commit 0fa89ff

Browse files
danmardanmar
committed
Revert 14365ff and f1cc3ad, there was a performance regression
1 parent 14365ff commit 0fa89ff

6 files changed

Lines changed: 76 additions & 138 deletions

File tree

‎lib/checkcondition.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -507,12 +507,14 @@ void CheckCondition::multiCondition()
507507
break;
508508
tok2 = tok2->tokAt(4);
509509

510-
if (tok2->astOperand2()) {
510+
if (tok2->astOperand2() &&
511+
!cond1->hasKnownIntValue() &&
512+
!tok2->astOperand2()->hasKnownIntValue()) {
511513
ErrorPath errorPath;
512514
if (isOverlappingCond(cond1, tok2->astOperand2(), true))
513-
overlappingElseIfConditionError(tok2->astOperand2(), cond1->linenr());
515+
overlappingElseIfConditionError(tok2, cond1->linenr());
514516
else if (isOppositeCond(true, mTokenizer->isCPP(), cond1, tok2->astOperand2(), mSettings->library, true, true, &errorPath))
515-
oppositeElseIfConditionError(cond1, tok2->astOperand2(), errorPath);
517+
oppositeElseIfConditionError(cond1, tok2, errorPath);
516518
}
517519
}
518520
}

‎lib/valueflow.cpp‎

Lines changed: 51 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1427,6 +1427,37 @@ static void valueFlowRightShift(TokenList *tokenList, const Settings* settings)
14271427
}
14281428
}
14291429

1430+
static void valueFlowOppositeCondition(SymbolDatabase *symboldatabase, const Settings *settings)
1431+
{
1432+
for (const Scope &scope : symboldatabase->scopeList) {
1433+
if (scope.type != Scope::eIf)
1434+
continue;
1435+
Token *tok = const_cast<Token *>(scope.classDef);
1436+
if (!Token::simpleMatch(tok, "if ("))
1437+
continue;
1438+
const Token *cond1 = tok->next()->astOperand2();
1439+
if (!cond1 || !cond1->isComparisonOp())
1440+
continue;
1441+
const bool cpp = symboldatabase->isCPP();
1442+
Token *tok2 = tok->linkAt(1);
1443+
while (Token::simpleMatch(tok2, ") {")) {
1444+
tok2 = tok2->linkAt(1);
1445+
if (!Token::simpleMatch(tok2, "} else { if ("))
1446+
break;
1447+
Token *ifOpenBraceTok = tok2->tokAt(4);
1448+
Token *cond2 = ifOpenBraceTok->astOperand2();
1449+
if (!cond2 || !cond2->isComparisonOp())
1450+
continue;
1451+
if (isOppositeCond(true, cpp, cond1, cond2, settings->library, true, true)) {
1452+
ValueFlow::Value value(1);
1453+
value.setKnown();
1454+
setTokenValue(cond2, value, settings);
1455+
}
1456+
tok2 = ifOpenBraceTok->link();
1457+
}
1458+
}
1459+
}
1460+
14301461
static void valueFlowEnumValue(SymbolDatabase * symboldatabase, const Settings * settings)
14311462
{
14321463

@@ -3725,19 +3756,7 @@ static const Token* findIncompleteVar(const Token* start, const Token* end)
37253756
return nullptr;
37263757
}
37273758

3728-
static ValueFlow::Value makeConditionValue(long long val, const Token* condTok, bool assume)
3729-
{
3730-
ValueFlow::Value v(val);
3731-
v.setKnown();
3732-
v.condition = condTok;
3733-
if (assume)
3734-
v.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is true");
3735-
else
3736-
v.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is false");
3737-
return v;
3738-
}
3739-
//
3740-
static void valueFlowConditionExpressions(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
3759+
static void valueFlowTerminatingCondition(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
37413760
{
37423761
for (const Scope * scope : symboldatabase->functionScopes) {
37433762
if (const Token* incompleteTok = findIncompleteVar(scope->bodyStart, scope->bodyEnd)) {
@@ -3754,58 +3773,28 @@ static void valueFlowConditionExpressions(TokenList *tokenlist, SymbolDatabase*
37543773
// Skip known values
37553774
if (tok->next()->hasKnownValue())
37563775
continue;
3757-
Token * parenTok = tok->next();
3776+
const Token * parenTok = tok->next();
37583777
if (!Token::simpleMatch(parenTok->link(), ") {"))
37593778
continue;
3760-
Token * blockTok = parenTok->link()->tokAt(1);
3761-
const Token* condTok = parenTok->astOperand2();
3762-
3763-
Token* startTok = blockTok;
3764-
// Inner condition
3765-
{
3766-
std::vector<const Token*> conds = {condTok};
3767-
if (Token::simpleMatch(condTok, "&&")) {
3768-
std::vector<const Token*> args = astFlatten(condTok, "&&");
3769-
conds.insert(conds.end(), args.begin(), args.end());
3770-
}
3771-
for(const Token* condTok2:conds) {
3772-
ExpressionAnalyzer a1(condTok2, makeConditionValue(1, condTok2, true), tokenlist);
3773-
valueFlowGenericForward(startTok, startTok->link(), a1, settings);
3774-
3775-
OppositeExpressionAnalyzer a2(true, condTok2, makeConditionValue(0, condTok2, true), tokenlist);
3776-
valueFlowGenericForward(startTok, startTok->link(), a2, settings);
3777-
}
3778-
}
3779-
3780-
std::vector<const Token*> conds = {condTok};
3781-
if (Token::simpleMatch(condTok, "||")) {
3782-
std::vector<const Token*> args = astFlatten(condTok, "||");
3783-
conds.insert(conds.end(), args.begin(), args.end());
3784-
}
3785-
3786-
// Check else block
3787-
if (Token::simpleMatch(startTok->link(), "} else {")) {
3788-
startTok = startTok->link()->tokAt(2);
3789-
for(const Token* condTok2:conds) {
3790-
ExpressionAnalyzer a1(condTok2, makeConditionValue(0, condTok2, false), tokenlist);
3791-
valueFlowGenericForward(startTok, startTok->link(), a1, settings);
3792-
3793-
OppositeExpressionAnalyzer a2(true, condTok2, makeConditionValue(1, condTok2, false), tokenlist);
3794-
valueFlowGenericForward(startTok, startTok->link(), a2, settings);
3795-
}
3796-
}
3797-
3779+
const Token * blockTok = parenTok->link()->tokAt(1);
37983780
// Check if the block terminates early
3799-
if (isEscapeScope(blockTok, tokenlist)) {
3800-
for(const Token* condTok2:conds) {
3801-
ExpressionAnalyzer a1(condTok2, makeConditionValue(0, condTok2, false), tokenlist);
3802-
valueFlowGenericForward(startTok->link()->next(), scope->bodyEnd, a1, settings);
3781+
if (!isEscapeScope(blockTok, tokenlist))
3782+
continue;
38033783

3804-
OppositeExpressionAnalyzer a2(true, condTok2, makeConditionValue(1, condTok2, false), tokenlist);
3805-
valueFlowGenericForward(startTok->link()->next(), scope->bodyEnd, a2, settings);
3806-
}
3807-
}
3784+
const Token* condTok = parenTok->astOperand2();
3785+
ValueFlow::Value v1(0);
3786+
v1.setKnown();
3787+
v1.condition = condTok;
3788+
v1.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is true");
3789+
ExpressionAnalyzer a1(condTok, v1, tokenlist);
3790+
valueFlowGenericForward(blockTok->link()->next(), scope->bodyEnd, a1, settings);
38083791

3792+
ValueFlow::Value v2(1);
3793+
v2.setKnown();
3794+
v2.condition = condTok;
3795+
v2.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is false");
3796+
OppositeExpressionAnalyzer a2(true, condTok, v2, tokenlist);
3797+
valueFlowGenericForward(blockTok->link()->next(), scope->bodyEnd, a2, settings);
38093798
}
38103799
}
38113800
}
@@ -6635,7 +6624,6 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
66356624
valueFlowLifetime(tokenlist, symboldatabase, errorLogger, settings);
66366625
valueFlowBitAnd(tokenlist);
66376626
valueFlowSameExpressions(tokenlist);
6638-
valueFlowConditionExpressions(tokenlist, symboldatabase, errorLogger, settings);
66396627
valueFlowFwdAnalysis(tokenlist, settings);
66406628

66416629
std::size_t values = 0;
@@ -6645,6 +6633,8 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
66456633
valueFlowPointerAliasDeref(tokenlist);
66466634
valueFlowArrayBool(tokenlist);
66476635
valueFlowRightShift(tokenlist, settings);
6636+
valueFlowOppositeCondition(symboldatabase, settings);
6637+
valueFlowTerminatingCondition(tokenlist, symboldatabase, errorLogger, settings);
66486638
valueFlowAfterMove(tokenlist, symboldatabase, errorLogger, settings);
66496639
valueFlowCondition(SimpleConditionHandler{}, tokenlist, symboldatabase, errorLogger, settings);
66506640
valueFlowInferCondition(tokenlist, settings);

‎test/testcondition.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -533,27 +533,27 @@ class TestCondition : public TestFixture {
533533
" if (a) { b = 1; }\n"
534534
" else { if (a) { b = 2; } }\n"
535535
"}");
536-
ASSERT_EQUALS("[test.cpp:3]: (style) Expression is always false because 'else if' condition matches previous condition at line 2.\n", errout.str());
536+
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Condition 'a' is always false\n", errout.str());
537537

538538
check("void f(int a, int &b) {\n"
539539
" if (a) { b = 1; }\n"
540540
" else { if (a) { b = 2; } }\n"
541541
"}");
542-
ASSERT_EQUALS("[test.cpp:3]: (style) Expression is always false because 'else if' condition matches previous condition at line 2.\n", errout.str());
542+
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Condition 'a' is always false\n", errout.str());
543543

544544
check("void f(int a, int &b) {\n"
545545
" if (a == 1) { b = 1; }\n"
546546
" else { if (a == 2) { b = 2; }\n"
547547
" else { if (a == 1) { b = 3; } } }\n"
548548
"}");
549-
ASSERT_EQUALS("[test.cpp:4]: (style) Expression is always false because 'else if' condition matches previous condition at line 2.\n", errout.str());
549+
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (style) Condition 'a==1' is always false\n", errout.str());
550550

551551
check("void f(int a, int &b) {\n"
552552
" if (a == 1) { b = 1; }\n"
553553
" else { if (a == 2) { b = 2; }\n"
554554
" else { if (a == 2) { b = 3; } } }\n"
555555
"}");
556-
ASSERT_EQUALS("[test.cpp:4]: (style) Expression is always false because 'else if' condition matches previous condition at line 3.\n", errout.str());
556+
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Condition 'a==2' is always false\n", errout.str());
557557

558558
check("void f(int a, int &b) {\n"
559559
" if (a++) { b = 1; }\n"
@@ -721,9 +721,9 @@ class TestCondition : public TestFixture {
721721
" if (x) {}\n"
722722
" else if (!x) {}\n"
723723
"}");
724-
ASSERT_EQUALS("test.cpp:3:style:Expression is always true because 'else if' condition is opposite to previous condition at line 2.\n"
725-
"test.cpp:2:note:first condition\n"
726-
"test.cpp:3:note:else if condition is opposite to first condition\n", errout.str());
724+
ASSERT_EQUALS("test.cpp:3:style:Condition '!x' is always true\n"
725+
"test.cpp:2:note:condition 'x'\n"
726+
"test.cpp:3:note:Condition '!x' is always true\n", errout.str());
727727

728728
check("void f(int x) {\n"
729729
" int y = x;\n"

‎test/testsimplifytypedef.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1670,7 +1670,7 @@ class TestSimplifyTypedef : public TestFixture {
16701670
"( ( int ( * * ( * ) ( char * , char * , int , int ) ) ( ) ) global [ 6 ] ) ( \"assoc\" , \"eggdrop\" , 106 , 0 ) ; "
16711671
"}";
16721672
ASSERT_EQUALS(expected, tok(code));
1673-
ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:3]: (debug) valueflow.cpp:1319:valueFlowConditionExpressions bailout: Skipping function due to incomplete variable global\n", errout.str());
1673+
ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:3]: (debug) valueflow.cpp:1319:valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable global\n", errout.str());
16741674
}
16751675

16761676
void simplifyTypedef68() { // ticket #2355
@@ -1877,7 +1877,7 @@ class TestSimplifyTypedef : public TestFixture {
18771877
" B * b = new B;\n"
18781878
" b->f = new A::F * [ 10 ];\n"
18791879
"}");
1880-
ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:12]: (debug) valueflow.cpp:1319:valueFlowConditionExpressions bailout: Skipping function due to incomplete variable idx\n", errout.str());
1880+
ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:12]: (debug) valueflow.cpp:1319:valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable idx\n", errout.str());
18811881
}
18821882

18831883
void simplifyTypedef83() { // ticket #2620

‎test/testsymboldatabase.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2743,7 +2743,7 @@ class TestSymbolDatabase: public TestFixture {
27432743
check("::y(){x}");
27442744

27452745
ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:1]: (debug) Executable scope 'y' with unknown function.\n"
2746-
"[test.cpp:1]: (debug) valueflow.cpp:1321:valueFlowConditionExpressions bailout: Skipping function due to incomplete variable x\n", errout.str());
2746+
"[test.cpp:1]: (debug) valueflow.cpp:1321:valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable x\n", errout.str());
27472747
}
27482748

27492749
void symboldatabase20() {

‎test/testvalueflow.cpp‎

Lines changed: 10 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ class TestValueFlow : public TestFixture {
122122

123123
TEST_CASE(valueFlowUninit);
124124

125-
TEST_CASE(valueFlowConditionExpressions);
125+
TEST_CASE(valueFlowTerminatingCond);
126126

127127
TEST_CASE(valueFlowContainerSize);
128128

@@ -1266,7 +1266,7 @@ class TestValueFlow : public TestFixture {
12661266
" if (x == 123) {}\n"
12671267
"}");
12681268
ASSERT_EQUALS_WITHOUT_LINENUMBERS(
1269-
"[test.cpp:2]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable y\n",
1269+
"[test.cpp:2]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable y\n",
12701270
errout.str());
12711271
}
12721272

@@ -1383,7 +1383,7 @@ class TestValueFlow : public TestFixture {
13831383
" y = ((x<0) ? x : ((x==2)?3:4));\n"
13841384
"}");
13851385
ASSERT_EQUALS_WITHOUT_LINENUMBERS(
1386-
"[test.cpp:2]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable y\n",
1386+
"[test.cpp:2]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable y\n",
13871387
errout.str());
13881388

13891389
bailout("int f(int x) {\n"
@@ -1448,7 +1448,7 @@ class TestValueFlow : public TestFixture {
14481448
" if (x == 123) {}\n"
14491449
"}");
14501450
ASSERT_EQUALS_WITHOUT_LINENUMBERS(
1451-
"[test.cpp:2]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable b\n",
1451+
"[test.cpp:2]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable b\n",
14521452
errout.str());
14531453

14541454
code = "void f(int x, bool abc) {\n"
@@ -1497,7 +1497,7 @@ class TestValueFlow : public TestFixture {
14971497
" };\n"
14981498
"}");
14991499
ASSERT_EQUALS_WITHOUT_LINENUMBERS(
1500-
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable a\n",
1500+
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n",
15011501
errout.str());
15021502

15031503
bailout("void f(int x, int y) {\n"
@@ -1507,7 +1507,7 @@ class TestValueFlow : public TestFixture {
15071507
" };\n"
15081508
"}");
15091509
ASSERT_EQUALS_WITHOUT_LINENUMBERS(
1510-
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable a\n",
1510+
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n",
15111511
errout.str());
15121512
}
15131513

@@ -1519,7 +1519,7 @@ class TestValueFlow : public TestFixture {
15191519
" M;\n"
15201520
"}");
15211521
ASSERT_EQUALS_WITHOUT_LINENUMBERS(
1522-
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable a\n"
1522+
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n"
15231523
"[test.cpp:4]: (debug) valueflow.cpp:1260:(valueFlow) bailout: variable 'x', condition is defined in macro\n",
15241524
errout.str());
15251525

@@ -1529,7 +1529,7 @@ class TestValueFlow : public TestFixture {
15291529
" FREE(x);\n"
15301530
"}");
15311531
ASSERT_EQUALS_WITHOUT_LINENUMBERS(
1532-
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable a\n"
1532+
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n"
15331533
"[test.cpp:4]: (debug) valueflow.cpp:1260:(valueFlow) bailout: variable 'x', condition is defined in macro\n",
15341534
errout.str());
15351535
}
@@ -1543,7 +1543,7 @@ class TestValueFlow : public TestFixture {
15431543
" if (x==123){}\n"
15441544
"}");
15451545
ASSERT_EQUALS_WITHOUT_LINENUMBERS(
1546-
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable a\n",
1546+
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n",
15471547
errout.str());
15481548

15491549
// #5721 - FP
@@ -4202,7 +4202,7 @@ class TestValueFlow : public TestFixture {
42024202
ASSERT_EQUALS(0, values.size());
42034203
}
42044204

4205-
void valueFlowConditionExpressions() {
4205+
void valueFlowTerminatingCond() {
42064206
const char* code;
42074207

42084208
// opposite condition
@@ -4272,60 +4272,6 @@ class TestValueFlow : public TestFixture {
42724272
"}\n";
42734273
ASSERT_EQUALS(false, valueOfTok(code, "!=").intvalue == 1);
42744274

4275-
code = "void f(bool b, int i, int j) {\n"
4276-
" if (b || i == j) return;\n"
4277-
" if(i != j) {}\n"
4278-
"}\n";
4279-
ASSERT_EQUALS(true, valueOfTok(code, "!=").intvalue == 1);
4280-
4281-
code = "void f(bool b, int i, int j) {\n"
4282-
" if (b && i == j) return;\n"
4283-
" if(i != j) {}\n"
4284-
"}\n";
4285-
ASSERT_EQUALS(true, tokenValues(code, "!=").empty());
4286-
4287-
code = "void f(int i, int j) {\n"
4288-
" if (i == j) {\n"
4289-
" if (i != j) {}\n"
4290-
" }\n"
4291-
"}\n";
4292-
ASSERT_EQUALS(true, valueOfTok(code, "!=").intvalue == 0);
4293-
4294-
code = "void f(int i, int j) {\n"
4295-
" if (i == j) {} else {\n"
4296-
" if (i != j) {}\n"
4297-
" }\n"
4298-
"}\n";
4299-
ASSERT_EQUALS(true, valueOfTok(code, "!=").intvalue == 1);
4300-
4301-
code = "void f(bool b, int i, int j) {\n"
4302-
" if (b && i == j) {\n"
4303-
" if (i != j) {}\n"
4304-
" }\n"
4305-
"}\n";
4306-
ASSERT_EQUALS(true, valueOfTok(code, "!=").intvalue == 0);
4307-
4308-
code = "void f(bool b, int i, int j) {\n"
4309-
" if (b || i == j) {\n"
4310-
" if (i != j) {}\n"
4311-
" }\n"
4312-
"}\n";
4313-
ASSERT_EQUALS(true, tokenValues(code, "!=").empty());
4314-
4315-
code = "void f(bool b, int i, int j) {\n"
4316-
" if (b || i == j) {} else {\n"
4317-
" if (i != j) {}\n"
4318-
" }\n"
4319-
"}\n";
4320-
ASSERT_EQUALS(true, valueOfTok(code, "!=").intvalue == 1);
4321-
4322-
code = "void f(bool b, int i, int j) {\n"
4323-
" if (b && i == j) {} else {\n"
4324-
" if (i != j) {}\n"
4325-
" }\n"
4326-
"}\n";
4327-
ASSERT_EQUALS(true, tokenValues(code, "!=").empty());
4328-
43294275
code = "void foo()\n" // #8924
43304276
"{\n"
43314277
" if ( this->FileIndex >= 0 )\n"

0 commit comments

Comments
 (0)