Skip to content

Commit b2d8361

Browse files
chrchr-githubweb-flow
chrchr-github
and
web-flow
authored
Fix #15021 FN redundantInitialization with structured binding (#8847)
Best viewed with whitespace changes hidden. --------- Co-authored-by: chrchr-github <noreply@github.com>
1 parent a3361b3 commit b2d8361

2 files changed

Lines changed: 89 additions & 39 deletions

File tree

‎lib/checkother.cpp‎

Lines changed: 49 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -688,53 +688,63 @@ void CheckOtherImpl::checkRedundantAssignment()
688688
start = tok->astOperand2();
689689
else
690690
start = tok->findExpressionStartEndTokens().second->next();
691-
const Token * tokenToCheck = tok->astOperand1();
692691

693-
// Check if we are working with union
694-
for (const Token* tempToken = tokenToCheck; Token::simpleMatch(tempToken, ".");) {
695-
tempToken = tempToken->astOperand1();
696-
if (tempToken && tempToken->variable() && tempToken->variable()->type() && tempToken->variable()->type()->isUnionType())
697-
tokenToCheck = tempToken;
692+
std::vector<const Token*> tokensToCheck{ tok->astOperand1() };
693+
if (Token::simpleMatch(tok->astOperand1(), "[") && Token::simpleMatch(tok->astOperand1()->astOperand1(), "auto")) { // structured binding
694+
tokensToCheck = astFlatten(tok->astOperand1()->astOperand2(), ",");
695+
isInitialization = true;
698696
}
699697

700-
if (start->hasKnownSymbolicValue(tokenToCheck) && Token::simpleMatch(start->astParent(), "=") && !diag(tok)) {
701-
const ValueFlow::Value* val = start->getKnownValue(ValueFlow::Value::ValueType::SYMBOLIC);
702-
if (val->intvalue == 0) // no offset
703-
redundantAssignmentSameValueError(tokenToCheck, val, tok->astOperand1()->expressionString());
704-
}
698+
for (const Token* tokenToCheck : tokensToCheck) {
705699

706-
// Get next assignment..
707-
const Token *nextAssign = fwdAnalysis.reassign(tokenToCheck, start, scope->bodyEnd);
708-
// extra check for union
709-
if (nextAssign && tokenToCheck != tok->astOperand1()) {
710-
nextAssign = fwdAnalysis.reassign(tok->astOperand1(), start, scope->bodyEnd);
711-
// reading another member of the same union in the rhs is a use through aliasing
712-
if (nextAssign && fwdAnalysis.hasOperand(nextAssign->astOperand2(), tokenToCheck))
713-
nextAssign = nullptr;
714-
}
700+
// Check if we are working with union
701+
for (const Token* tempToken = tokenToCheck; Token::simpleMatch(tempToken, ".");) {
702+
tempToken = tempToken->astOperand1();
703+
if (tempToken && tempToken->variable() && tempToken->variable()->type() && tempToken->variable()->type()->isUnionType())
704+
tokenToCheck = tempToken;
705+
}
715706

716-
if (!nextAssign)
717-
continue;
707+
if (start->hasKnownSymbolicValue(tokenToCheck) && Token::simpleMatch(start->astParent(), "=") && !diag(tok)) {
708+
const ValueFlow::Value* val = start->getKnownValue(ValueFlow::Value::ValueType::SYMBOLIC);
709+
if (val->intvalue == 0) // no offset
710+
redundantAssignmentSameValueError(tokenToCheck, val, tok->astOperand1()->expressionString());
711+
}
718712

719-
// there is redundant assignment. Is there a case between the assignments?
720-
bool hasCase = false;
721-
for (const Token *tok2 = tok; tok2 != nextAssign; tok2 = tok2->next()) {
722-
if (tok2->str() == "break" || tok2->str() == "return")
723-
break;
724-
if (tok2->str() == "case") {
725-
hasCase = true;
726-
break;
713+
// Get next assignment..
714+
const Token* nextAssign = fwdAnalysis.reassign(tokenToCheck, start, scope->bodyEnd);
715+
// extra check for union
716+
const bool isUnion = tokenToCheck != tok->astOperand1() && !isStructuredBindingVariable(tokenToCheck->variable());
717+
if (nextAssign && isUnion) {
718+
nextAssign = fwdAnalysis.reassign(tok->astOperand1(), start, scope->bodyEnd);
719+
// reading another member of the same union in the rhs is a use through aliasing
720+
if (nextAssign && fwdAnalysis.hasOperand(nextAssign->astOperand2(), tokenToCheck))
721+
nextAssign = nullptr;
727722
}
728-
}
729723

730-
// warn
731-
if (hasCase)
732-
redundantAssignmentInSwitchError(tok, nextAssign, tok->astOperand1()->expressionString());
733-
else if (isInitialization)
734-
redundantInitializationError(tok, nextAssign, tok->astOperand1()->expressionString(), inconclusive);
735-
else {
736-
diag(nextAssign);
737-
redundantAssignmentError(tok, nextAssign, tok->astOperand1()->expressionString(), inconclusive);
724+
if (!nextAssign)
725+
continue;
726+
727+
// there is redundant assignment. Is there a case between the assignments?
728+
bool hasCase = false;
729+
for (const Token* tok2 = tok; tok2 != nextAssign; tok2 = tok2->next()) {
730+
if (tok2->str() == "break" || tok2->str() == "return")
731+
break;
732+
if (tok2->str() == "case") {
733+
hasCase = true;
734+
break;
735+
}
736+
}
737+
738+
// warn
739+
const Token* exprTok = isUnion ? tok->astOperand1() : tokenToCheck;
740+
if (hasCase)
741+
redundantAssignmentInSwitchError(tok, nextAssign, exprTok->expressionString());
742+
else if (isInitialization)
743+
redundantInitializationError(tok, nextAssign, exprTok->expressionString(), inconclusive);
744+
else {
745+
diag(nextAssign);
746+
redundantAssignmentError(tok, nextAssign, exprTok->expressionString(), inconclusive);
747+
}
738748
}
739749
}
740750
}

‎test/testother.cpp‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11555,6 +11555,46 @@ class TestOther : public TestFixture {
1155511555
"[test.cpp:8:10]: note: i is initialized\n"
1155611556
"[test.cpp:9:7]: note: i is overwritten\n",
1155711557
errout_str());
11558+
11559+
check("struct S { int a, b; };\n" // #15021
11560+
"int f(S s) {\n"
11561+
" auto [x, y] = s;\n"
11562+
" x = 1;\n"
11563+
" y = 2;\n"
11564+
" return x + y;\n"
11565+
"}\n"
11566+
"struct T { int c; };\n"
11567+
"int g(T t) {\n"
11568+
" auto [z] = t;\n"
11569+
" z = 0;\n"
11570+
" return z;\n"
11571+
"}\n");
11572+
ASSERT_EQUALS("[test.cpp:4:7]: style: Redundant initialization for 'x'. The initialized value is overwritten before it is read. [redundantInitialization]\n"
11573+
"[test.cpp:3:17]: note: x is initialized\n"
11574+
"[test.cpp:4:7]: note: x is overwritten\n"
11575+
"[test.cpp:5:7]: style: Redundant initialization for 'y'. The initialized value is overwritten before it is read. [redundantInitialization]\n"
11576+
"[test.cpp:3:17]: note: y is initialized\n"
11577+
"[test.cpp:5:7]: note: y is overwritten\n"
11578+
"[test.cpp:11:7]: style: Redundant initialization for 'z'. The initialized value is overwritten before it is read. [redundantInitialization]\n"
11579+
"[test.cpp:10:14]: note: z is initialized\n"
11580+
"[test.cpp:11:7]: note: z is overwritten\n",
11581+
errout_str());
11582+
11583+
check("struct S {\n"
11584+
" int a, b;\n"
11585+
"};\n"
11586+
"void f(S& s) {\n"
11587+
" auto& [x, y] = s;\n"
11588+
" y = 1;\n"
11589+
"}\n"
11590+
"void f(S s) {\n"
11591+
" auto [x, y] = s;\n"
11592+
" y = 1;\n"
11593+
"}\n");
11594+
ASSERT_EQUALS("[test.cpp:10:7]: style: Redundant initialization for 'y'. The initialized value is overwritten before it is read. [redundantInitialization]\n"
11595+
"[test.cpp:9:17]: note: y is initialized\n"
11596+
"[test.cpp:10:7]: note: y is overwritten\n",
11597+
errout_str());
1155811598
}
1155911599

1156011600
// cppcheck-suppress unusedPrivateFunction

0 commit comments

Comments
 (0)