From 887b40e08b591eaa017c5be668932927e533d47d Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sat, 26 Sep 2020 03:31:40 -0500 Subject: [PATCH] Fix issue 9916: False positive: duplicateAssignExpression when it's checked if variables have initial value later (#2825) --- lib/checkother.cpp | 41 +++++++++++++++++++++++++++++++---------- test/testother.cpp | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 3103bfee3..7d1046ccf 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2050,19 +2050,40 @@ void CheckOther::checkDuplicateExpression() isSameExpression(mTokenizer->isCPP(), true, tok->next(), nextAssign->next(), mSettings->library, true, false) && isSameExpression(mTokenizer->isCPP(), true, tok->astOperand2(), nextAssign->astOperand2(), mSettings->library, true, false) && tok->astOperand2()->expressionString() == nextAssign->astOperand2()->expressionString()) { - bool assigned = false; + bool differentDomain = false; const Scope * varScope = var1->scope() ? var1->scope() : scope; for (const Token *assignTok = Token::findsimplematch(var2, ";"); assignTok && assignTok != varScope->bodyEnd; assignTok = assignTok->next()) { - if (Token::Match(assignTok, "%varid% = %var%", var1->varId()) && Token::Match(assignTok, "%var% = %varid%", var2->varId())) { - assigned = true; - break; - } - if (Token::Match(assignTok, "%varid% = %var%", var2->varId()) && Token::Match(assignTok, "%var% = %varid%", var1->varId())) { - assigned = true; - break; - } + if (!Token::Match(assignTok, "%assign%|%comp%")) + continue; + if (!assignTok->astOperand1()) + continue; + if (!assignTok->astOperand2()) + continue; + + if (assignTok->astOperand1()->varId() != var1->varId() && + assignTok->astOperand1()->varId() != var2->varId() && + !isSameExpression(mTokenizer->isCPP(), + true, + tok->astOperand2(), + assignTok->astOperand1(), + mSettings->library, + true, + true)) + continue; + if (assignTok->astOperand2()->varId() != var1->varId() && + assignTok->astOperand2()->varId() != var2->varId() && + !isSameExpression(mTokenizer->isCPP(), + true, + tok->astOperand2(), + assignTok->astOperand2(), + mSettings->library, + true, + true)) + continue; + differentDomain = true; + break; } - if (!assigned && !isUniqueExpression(tok->astOperand2())) + if (!differentDomain && !isUniqueExpression(tok->astOperand2())) duplicateAssignExpressionError(var1, var2, false); else if (mSettings->inconclusive) duplicateAssignExpressionError(var1, var2, true); diff --git a/test/testother.cpp b/test/testother.cpp index 40833c093..f85cca9d6 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -5694,6 +5694,42 @@ private: "}"); ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'.\n", errout.str()); + check("struct A { int x; int y; };" + "void use(int);\n" + "void test(A a) {\n" + " int i = a.x;\n" + " int j = a.x;\n" + " use(j);\n" + " if (i == j) {}\n" + "}"); + ASSERT_EQUALS( + "[test.cpp:4] -> [test.cpp:3]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'.\n", + errout.str()); + + check("struct A { int x; int y; };" + "void use(int);\n" + "void test(A a) {\n" + " int i = a.x;\n" + " int j = a.x;\n" + " use(j);\n" + " if (i == a.x) {}\n" + "}"); + ASSERT_EQUALS( + "[test.cpp:4] -> [test.cpp:3]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'.\n", + errout.str()); + + check("struct A { int x; int y; };" + "void use(int);\n" + "void test(A a) {\n" + " int i = a.x;\n" + " int j = a.x;\n" + " use(i);\n" + " if (j == a.x) {}\n" + "}"); + ASSERT_EQUALS( + "[test.cpp:4] -> [test.cpp:3]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'.\n", + errout.str()); + // Issue #8612 check("struct P\n" "{\n"