Skip to content

Commit ba56407

Browse files
committed
Fixed #7907 (FN: redundant assignment inside switchcase, overwritten by assignment outside of switch)
1 parent 88785dd commit ba56407

2 files changed

Lines changed: 35 additions & 2 deletions

File tree

lib/checkother.cpp

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ struct CheckOther::ScopeResult CheckOther::checkRedundantAssignmentRecursive(con
476476
}
477477

478478
if (Token::simpleMatch(tok, "break ;")) {
479-
return ScopeResult(ScopeResult::BREAK);
479+
return ScopeResult(ScopeResult::BREAK, tok);
480480
}
481481

482482
if (Token::Match(tok, "continue|return|throw|goto")) {
@@ -609,7 +609,26 @@ void CheckOther::checkRedundantAssignment()
609609
start = tok->next();
610610
else
611611
start = tok->findExpressionStartEndTokens().second->next();
612-
const ScopeResult &nextAssign = checkRedundantAssignmentRecursive(tok, start, scope->bodyEnd);
612+
613+
// Get next assignment..
614+
ScopeResult nextAssign(ScopeResult::NONE);
615+
while (true) {
616+
nextAssign = checkRedundantAssignmentRecursive(tok, start, scope->bodyEnd);
617+
618+
// Break => continue checking in outer scope
619+
if (nextAssign.type == ScopeResult::BREAK) {
620+
const Scope *s = nextAssign.token->scope();
621+
while (s->type == Scope::eIf)
622+
s = s->nestedIn;
623+
if (s->type == Scope::eSwitch) {
624+
start = s->bodyEnd->next();
625+
continue;
626+
}
627+
}
628+
629+
break;
630+
}
631+
613632
if (nextAssign.type != ScopeResult::WRITE || !nextAssign.token)
614633
continue;
615634

test/testother.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ class TestOther : public TestFixture {
164164
TEST_CASE(redundantVarAssignment_stackoverflow);
165165
TEST_CASE(redundantVarAssignment_lambda);
166166
TEST_CASE(redundantVarAssignment_for);
167+
TEST_CASE(redundantVarAssignment_after_switch);
167168
TEST_CASE(redundantMemWrite);
168169

169170
TEST_CASE(varFuncNullUB);
@@ -6070,6 +6071,19 @@ class TestOther : public TestFixture {
60706071
ASSERT_EQUALS("", errout.str());
60716072
}
60726073

6074+
void redundantVarAssignment_after_switch() {
6075+
check("void f(int x) {\n" // #7907
6076+
" int ret;\n"
6077+
" switch (x) {\n"
6078+
" case 123:\n"
6079+
" ret = 1;\n" // redundant assignment
6080+
" break;\n"
6081+
" }\n"
6082+
" ret = 3;\n"
6083+
"}");
6084+
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:8]: (style) Variable 'ret' is reassigned a value before the old one has been used.\n", errout.str());
6085+
}
6086+
60736087
void redundantMemWrite() {
60746088
return; // FIXME: temporary hack
60756089

0 commit comments

Comments
 (0)