Skip to content

Commit 1e21c8d

Browse files
danmardanmar
committed
Fix #13773 (False positive: should not warn when different ternary 2nd and 3rd expressions happens to have same value)
1 parent 3e8c9d4 commit 1e21c8d

2 files changed

Lines changed: 39 additions & 23 deletions

File tree

‎lib/checkother.cpp‎

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2163,7 +2163,7 @@ static bool isConstant(const Token* tok) {
21632163
return tok && (tok->isEnumerator() || Token::Match(tok, "%bool%|%num%|%str%|%char%|nullptr|NULL"));
21642164
}
21652165

2166-
static bool isConstStatement(const Token *tok, const Library& library, bool isNestedBracket = false)
2166+
static bool isConstStatement(const Token *tok, const Library& library, bool platformIndependent, bool isNestedBracket = false)
21672167
{
21682168
if (!tok)
21692169
return false;
@@ -2185,42 +2185,48 @@ static bool isConstStatement(const Token *tok, const Library& library, bool isNe
21852185
tok2 = tok2->astParent();
21862186
}
21872187
if (Token::Match(tok, "&&|%oror%"))
2188-
return isConstStatement(tok->astOperand1(), library) && isConstStatement(tok->astOperand2(), library);
2188+
return isConstStatement(tok->astOperand1(), library, platformIndependent) && isConstStatement(tok->astOperand2(), library, platformIndependent);
21892189
if (Token::Match(tok, "!|~|%cop%") && (tok->astOperand1() || tok->astOperand2()))
21902190
return true;
2191-
if (Token::Match(tok->previous(), "sizeof|alignof|noexcept|typeid (") && tok->previous()->isKeyword())
2191+
if (Token::simpleMatch(tok->previous(), "sizeof ("))
2192+
return !platformIndependent;
2193+
if (Token::Match(tok->previous(), "alignof|noexcept|typeid (") && tok->previous()->isKeyword())
21922194
return true;
21932195
if (isCPPCast(tok)) {
21942196
if (Token::simpleMatch(tok->astOperand1(), "dynamic_cast") && Token::simpleMatch(tok->astOperand1()->linkAt(1)->previous(), "& >"))
21952197
return false;
2196-
return isWithoutSideEffects(tok) && isConstStatement(tok->astOperand2(), library);
2198+
return isWithoutSideEffects(tok) && isConstStatement(tok->astOperand2(), library, platformIndependent);
21972199
}
21982200
if (tok->isCast() && tok->next() && tok->next()->isStandardType())
2199-
return isWithoutSideEffects(tok->astOperand1()) && isConstStatement(tok->astOperand1(), library);
2201+
return isWithoutSideEffects(tok->astOperand1()) && isConstStatement(tok->astOperand1(), library, platformIndependent);
22002202
if (Token::simpleMatch(tok, "."))
2201-
return isConstStatement(tok->astOperand2(), library);
2203+
return isConstStatement(tok->astOperand2(), library, platformIndependent);
22022204
if (Token::simpleMatch(tok, ",")) {
22032205
if (tok->astParent()) // warn about const statement on rhs at the top level
2204-
return isConstStatement(tok->astOperand1(), library) && isConstStatement(tok->astOperand2(), library);
2206+
return isConstStatement(tok->astOperand1(), library, platformIndependent) &&
2207+
isConstStatement(tok->astOperand2(), library, platformIndependent);
22052208

22062209
const Token* lml = previousBeforeAstLeftmostLeaf(tok); // don't warn about matrix/vector assignment (e.g. Eigen)
22072210
if (lml)
22082211
lml = lml->next();
22092212
const Token* stream = lml;
22102213
while (stream && Token::Match(stream->astParent(), ".|[|(|*"))
22112214
stream = stream->astParent();
2212-
return (!stream || !isLikelyStream(stream)) && isConstStatement(tok->astOperand2(), library);
2215+
return (!stream || !isLikelyStream(stream)) && isConstStatement(tok->astOperand2(), library, platformIndependent);
22132216
}
22142217
if (Token::simpleMatch(tok, "?") && Token::simpleMatch(tok->astOperand2(), ":")) // ternary operator
2215-
return isConstStatement(tok->astOperand1(), library) && isConstStatement(tok->astOperand2()->astOperand1(), library) && isConstStatement(tok->astOperand2()->astOperand2(), library);
2218+
return isConstStatement(tok->astOperand1(), library, platformIndependent) &&
2219+
isConstStatement(tok->astOperand2()->astOperand1(), library, platformIndependent) &&
2220+
isConstStatement(tok->astOperand2()->astOperand2(), library, platformIndependent);
22162221
if (isBracketAccess(tok) && isWithoutSideEffects(tok->astOperand1(), /*checkArrayAccess*/ true, /*checkReference*/ false)) {
22172222
const bool isChained = succeeds(tok->astParent(), tok);
22182223
if (Token::simpleMatch(tok->astParent(), "[")) {
22192224
if (isChained)
2220-
return isConstStatement(tok->astOperand2(), library) && isConstStatement(tok->astParent(), library);
2221-
return isNestedBracket && isConstStatement(tok->astOperand2(), library);
2225+
return isConstStatement(tok->astOperand2(), library, platformIndependent) &&
2226+
isConstStatement(tok->astParent(), library, platformIndependent);
2227+
return isNestedBracket && isConstStatement(tok->astOperand2(), library, platformIndependent);
22222228
}
2223-
return isConstStatement(tok->astOperand2(), library, /*isNestedBracket*/ !isChained);
2229+
return isConstStatement(tok->astOperand2(), library, platformIndependent, /*isNestedBracket*/ !isChained);
22242230
}
22252231
if (!tok->astParent() && findLambdaEndToken(tok))
22262232
return true;
@@ -2331,7 +2337,7 @@ void CheckOther::checkIncompleteStatement()
23312337
// Skip statement expressions
23322338
if (Token::simpleMatch(rtok, "; } )"))
23332339
continue;
2334-
if (!isConstStatement(tok, mSettings->library))
2340+
if (!isConstStatement(tok, mSettings->library, false))
23352341
continue;
23362342
if (isVoidStmt(tok))
23372343
continue;
@@ -2526,7 +2532,7 @@ void CheckOther::checkMisusedScopedObject()
25262532
if (Token::simpleMatch(parTok, "<") && parTok->link())
25272533
parTok = parTok->link()->next();
25282534
if (const Token* arg = parTok->astOperand2()) {
2529-
if (!isConstStatement(arg, mSettings->library))
2535+
if (!isConstStatement(arg, mSettings->library, false))
25302536
continue;
25312537
if (parTok->str() == "(") {
25322538
if (arg->varId() && !(arg->variable() && arg->variable()->nameToken() != arg))
@@ -2985,12 +2991,13 @@ void CheckOther::checkDuplicateExpression()
29852991
}
29862992
}
29872993
} else if (tok->astOperand1() && tok->astOperand2() && tok->str() == ":" && tok->astParent() && tok->astParent()->str() == "?") {
2988-
if (!tok->astOperand1()->values().empty() && !tok->astOperand2()->values().empty() && isEqualKnownValue(tok->astOperand1(), tok->astOperand2()) &&
2994+
if (isSameExpression(true, tok->astOperand1(), tok->astOperand2(), *mSettings, false, true, &errorPath))
2995+
duplicateExpressionTernaryError(tok, std::move(errorPath));
2996+
2997+
else if (!tok->astOperand1()->values().empty() && !tok->astOperand2()->values().empty() && isEqualKnownValue(tok->astOperand1(), tok->astOperand2()) &&
29892998
!isVariableChanged(tok->astParent(), /*indirect*/ 0, *mSettings) &&
2990-
isConstStatement(tok->astOperand1(), mSettings->library) && isConstStatement(tok->astOperand2(), mSettings->library))
2999+
isConstStatement(tok->astOperand1(), mSettings->library, true) && isConstStatement(tok->astOperand2(), mSettings->library, true))
29913000
duplicateValueTernaryError(tok);
2992-
else if (isSameExpression(true, tok->astOperand1(), tok->astOperand2(), *mSettings, false, true, &errorPath))
2993-
duplicateExpressionTernaryError(tok, std::move(errorPath));
29943001
}
29953002
}
29963003
}

‎test/testother.cpp‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ class TestOther : public TestFixture {
197197
TEST_CASE(duplicateExpression18);
198198
TEST_CASE(duplicateExpressionLoop);
199199
TEST_CASE(duplicateValueTernary);
200+
TEST_CASE(duplicateValueTernarySizeof); // #13773
200201
TEST_CASE(duplicateExpressionTernary); // #6391
201202
TEST_CASE(duplicateExpressionTemplate); // #6930
202203
TEST_CASE(duplicateExpressionCompareWithZero);
@@ -8106,31 +8107,31 @@ class TestOther : public TestFixture {
81068107
check("void f() {\n"
81078108
" if( a ? (b ? false:false): false ) ;\n"
81088109
"}");
8109-
ASSERT_EQUALS("[test.cpp:2:23]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
8110+
ASSERT_EQUALS("[test.cpp:2:23]: (style) Same expression in both branches of ternary operator. [duplicateExpressionTernary]\n", errout_str());
81108111

81118112
check("int f1(int a) {return (a == 1) ? (int)1 : 1; }");
81128113
ASSERT_EQUALS("[test.cpp:1:41]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
81138114

81148115
check("int f2(int a) {return (a == 1) ? (int)1 : (int)1; }");
8115-
ASSERT_EQUALS("[test.cpp:1:41]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
8116+
ASSERT_EQUALS("[test.cpp:1:41]: (style) Same expression in both branches of ternary operator. [duplicateExpressionTernary]\n", errout_str());
81168117

81178118
check("int f3(int a) {return (a == 1) ? 1 : (int)1; }");
81188119
ASSERT_EQUALS("[test.cpp:1:36]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
81198120

81208121
check("int f4(int a) {return (a == 1) ? 1 : 1; }");
8121-
ASSERT_EQUALS("[test.cpp:1:36]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
8122+
ASSERT_EQUALS("[test.cpp:1:36]: (style) Same expression in both branches of ternary operator. [duplicateExpressionTernary]\n", errout_str());
81228123

81238124
check("int f5(int a) {return (a == (int)1) ? (int)1 : 1; }");
81248125
ASSERT_EQUALS("[test.cpp:1:46]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
81258126

81268127
check("int f6(int a) {return (a == (int)1) ? (int)1 : (int)1; }");
8127-
ASSERT_EQUALS("[test.cpp:1:46]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
8128+
ASSERT_EQUALS("[test.cpp:1:46]: (style) Same expression in both branches of ternary operator. [duplicateExpressionTernary]\n", errout_str());
81288129

81298130
check("int f7(int a) {return (a == (int)1) ? 1 : (int)1; }");
81308131
ASSERT_EQUALS("[test.cpp:1:41]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
81318132

81328133
check("int f8(int a) {return (a == (int)1) ? 1 : 1; }");
8133-
ASSERT_EQUALS("[test.cpp:1:41]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
8134+
ASSERT_EQUALS("[test.cpp:1:41]: (style) Same expression in both branches of ternary operator. [duplicateExpressionTernary]\n", errout_str());
81348135

81358136
check("struct Foo {\n"
81368137
" std::vector<int> bar{1,2,3};\n"
@@ -8170,6 +8171,14 @@ class TestOther : public TestFixture {
81708171
ASSERT_EQUALS("", errout_str());
81718172
}
81728173

8174+
void duplicateValueTernarySizeof() { // #13773
8175+
check("int f() { return x ? sizeof(uint32_t) : sizeof(unsigned int); }");
8176+
ASSERT_EQUALS("", errout_str());
8177+
8178+
check("int f() { return x ? sizeof(uint32_t) : sizeof(uint32_t); }");
8179+
ASSERT_EQUALS("[test.cpp:1:39]: (style) Same expression in both branches of ternary operator. [duplicateExpressionTernary]\n", errout_str());
8180+
}
8181+
81738182
void duplicateExpressionTemplate() {
81748183
check("template <int I> void f() {\n" // #6930
81758184
" if (I >= 0 && I < 3) {}\n"

0 commit comments

Comments
 (0)