Skip to content

Commit 887439a

Browse files
authored
Fix #14717 (FP: same exprId for different expressions leads to wrong redundantAssignment) (cppcheck-opensource#8520)
1 parent 7a7c28c commit 887439a

2 files changed

Lines changed: 41 additions & 15 deletions

File tree

lib/symboldatabase.cpp

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1611,6 +1611,26 @@ static std::string getIncompleteNameID(const Token* tok)
16111611
}
16121612

16131613
namespace {
1614+
int getExprIdForOperand(const Token* tok) {
1615+
if (!tok)
1616+
return 0;
1617+
1618+
int otherExprId = 0;
1619+
1620+
// Look through all referenced tokens.
1621+
// If two exprIds are found and one matches tok->exprId(), return the other.
1622+
// Otherwise, default to returning tok->exprId().
1623+
for (const auto& ref: followAllReferences(tok)) {
1624+
const int refExprId = ref.token->exprId();
1625+
if (refExprId != 0 && refExprId != tok->exprId()) {
1626+
if (otherExprId != 0 && otherExprId != refExprId)
1627+
return tok->exprId();
1628+
otherExprId = refExprId;
1629+
}
1630+
}
1631+
return otherExprId != 0 ? otherExprId : tok->exprId();
1632+
}
1633+
16141634
struct ExprIdKey {
16151635
std::string parentOp;
16161636
nonneg int operand1;
@@ -1645,8 +1665,8 @@ namespace {
16451665

16461666
ExprIdKey key;
16471667
key.parentOp = tok->astParent()->str();
1648-
key.operand1 = op1 ? op1->exprId() : 0;
1649-
key.operand2 = op2 ? op2->exprId() : 0;
1668+
key.operand1 = getExprIdForOperand(op1);
1669+
key.operand2 = getExprIdForOperand(op2);
16501670

16511671
if (tok->astParent()->isCast() && tok->astParent()->str() == "(") {
16521672
const Token* typeStartToken;
@@ -1665,19 +1685,6 @@ namespace {
16651685
key.parentOp += type;
16661686
}
16671687

1668-
for (const auto& ref: followAllReferences(op1)) {
1669-
if (ref.token->exprId() != 0) { // cppcheck-suppress useStlAlgorithm
1670-
key.operand1 = ref.token->exprId();
1671-
break;
1672-
}
1673-
}
1674-
for (const auto& ref: followAllReferences(op2)) {
1675-
if (ref.token->exprId() != 0) { // cppcheck-suppress useStlAlgorithm
1676-
key.operand2 = ref.token->exprId();
1677-
break;
1678-
}
1679-
}
1680-
16811688
if (key.operand1 > key.operand2 && key.operand2 &&
16821689
Token::Match(tok->astParent(), "%or%|%oror%|+|*|&|&&|^|==|!=")) {
16831690
// In C++ the order of operands of + might matter

test/testvarid.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,7 @@ class TestVarID : public TestFixture {
262262
TEST_CASE(exprid12);
263263
TEST_CASE(exprid13);
264264
TEST_CASE(exprid14);
265+
TEST_CASE(exprid15);
265266

266267
TEST_CASE(structuredBindings);
267268
}
@@ -4529,6 +4530,24 @@ class TestVarID : public TestFixture {
45294530
ASSERT_EQUALS(exp, tokenize(code, s)); // don't crash
45304531
}
45314532

4533+
void exprid15()
4534+
{
4535+
// #14717
4536+
const char code[] = "#define MAX(a, b) (((a) > (b)) ? (a) : (b))\n"
4537+
"\n"
4538+
"void f(char *d) {\n"
4539+
" const char *p = &d[0];\n"
4540+
" int a = 1 / MAX(1, p[0]);\n"
4541+
" a = 1 / MAX(1, p[1]);\n"
4542+
"}\n";
4543+
const char exp[] = "3: void f ( char * d ) {\n"
4544+
"4: const char * p@2 ; p@2 =@UNIQUE &@UNIQUE d@1 [@UNIQUE 0 ] ;\n"
4545+
"5: int a@3 ; a@3 =@UNIQUE 1 /@UNIQUE $( $( 1 $>@UNIQUE $( p@2 [@9 0 ] $) $) $?@UNIQUE $( 1 $) $:@UNIQUE $( p@2 [@9 0 ] $) $) ;\n"
4546+
"6: a@3 =@UNIQUE 1 /@UNIQUE $( $( 1 $>@UNIQUE $( p@2 [@15 1 ] $) $) $?@UNIQUE $( 1 $) $:@UNIQUE $( p@2 [@15 1 ] $) $) ;\n"
4547+
"7: }\n";
4548+
ASSERT_EQUALS(exp, tokenizeExpr(code));
4549+
}
4550+
45324551
void structuredBindings() {
45334552
const char code[] = "int foo() { auto [x,y] = xy(); return x+y; }";
45344553
ASSERT_EQUALS("1: int foo ( ) { auto [ x@1 , y@2 ] = xy ( ) ; return x@1 + y@2 ; }\n",

0 commit comments

Comments
 (0)