walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fix varId assignment for uses of variables declared in the if condition (#3231) 

Variables declared in the if condition (or in C++17 init-statement) are
visible not only in the if body but also in the else body. But logic in
Tokenizer::setVarIdPass1() handled such variables as if they were
declared in the if body.

As the result they were removed from variablesMap by the time the else
block was parsed and their uses in the else block were either given an
incorrect varId from variables in some outer scope or not given a varId
at all.

This then resulted in false positive unreadVariable errors for variables
declared in the if condition (or init-statement) and used only in the
else block.

Simplification from "else if ..." to "else { if ... }" was moved before
setVarId() to simplify detection for ends of blocks in if-else chains.
This commit is contained in:
dummyunit 2021-04-26 08:38:03 +03:00 committed by GitHub
parent d6842007a8
commit b18e6f1e10
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 106 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
lib/tokenize.cpp
View File

@ -3728,8 +3728,22 @@ void Tokenizer::setVarIdPass1()
else if (tok->str() == ";") {
if (!variableMap.leaveScope())
cppcheckError(tok);
} else if (tok->str() == "{")
} else if (tok->str() == "{") {
scopeStack.push(VarIdScopeInfo(true, scopeStack.top().isStructInit || tok->strAt(-1) == "=", /*isEnum=*/false, *variableMap.getVarId()));
// check if this '{' is a start of an "if" body
const Token * ifToken = tok->previous();
if (ifToken && ifToken->str() == ")")
ifToken = ifToken->link();
else
ifToken = nullptr;
if (ifToken)
ifToken = ifToken->previous();
if (ifToken && ifToken->str() == "if") {
// open another scope to differentiate between variables declared in the "if" condition and in the "if" body
variableMap.enterScope();
}
}
} else if (!initlist && tok->str()=="(") {
const Token * newFunctionDeclEnd = nullptr;
if (!scopeStack.top().isExecutable)
@ -3788,6 +3802,17 @@ void Tokenizer::setVarIdPass1()
if (!scopeStack.top().isStructInit) {
variableMap.leaveScope();
// check if this '}' is an end of an "else" body or an "if" body without an "else" part
const Token * ifToken = startToken->previous();
if (ifToken && ifToken->str() == ")")
ifToken = ifToken->link()->previous();
else
ifToken = nullptr;
if (startToken->strAt(-1) == "else" || (ifToken && ifToken->str() == "if" && tok->strAt(1) != "else")) {
// leave the extra scope used to differentiate between variables declared in the "if" condition and in the "if" body
variableMap.leaveScope();
}
}
scopeStack.pop();
@ -5096,6 +5121,8 @@ bool Tokenizer::simplifyTokenList1(const char FileName[])
// Split up variable declarations.
simplifyVarDecl(false);
elseif();
validate(); // #6772 "segmentation fault (invalid code) in Tokenizer::setVarId"
if (mTimerResults) {
@ -5141,8 +5168,6 @@ bool Tokenizer::simplifyTokenList1(const char FileName[])
simplifyEmptyNamespaces();
elseif();
simplifyIfSwitchForInit();
simplifyOverloadedOperators();

30
test/testunusedvar.cpp
View File

@ -179,6 +179,7 @@ private:
TEST_CASE(localvarInvert); // Usage with inverted variable
TEST_CASE(localvarIf); // Usage in if
TEST_CASE(localvarIfElse); // return tmp1 ? tmp2 : tmp3;
TEST_CASE(localvarDeclaredInIf);
TEST_CASE(localvarOpAssign); // a |= b;
TEST_CASE(localvarFor); // for ( ; var; )
TEST_CASE(localvarForEach); // #4155 - BOOST_FOREACH, hlist_for_each, etc
@ -4593,6 +4594,35 @@ private:
ASSERT_EQUALS("", errout.str());
}
void localvarDeclaredInIf() {
functionVariableUsage("int foo(int x)\n"
"{\n"
" if (int y = x % 2)\n"
" return 2;\n"
" else\n"
" return 1;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (style) Variable 'y' is assigned a value that is never used.\n", errout.str());
functionVariableUsage("int foo(int x)\n"
"{\n"
" if (int y = x % 2)\n"
" return y;\n"
" else\n"
" return 1;\n"
"}");
ASSERT_EQUALS("", errout.str());
functionVariableUsage("int foo(int x)\n"
"{\n"
" if (int y = x % 2)\n"
" return 2;\n"
" else\n"
" return y;\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void localvarOpAssign() {
functionVariableUsage("void foo()\n"
"{\n"

48
test/testvarid.cpp
View File

@ -173,6 +173,7 @@ private:
TEST_CASE(varid_parameter_pack); // #9383
TEST_CASE(varid_for_auto_cpp17);
TEST_CASE(varid_not); // #9689 'not x'
TEST_CASE(varid_declInIfCondition);
TEST_CASE(varidclass1);
TEST_CASE(varidclass2);
@ -2698,6 +2699,53 @@ private:
ASSERT_EQUALS(exp1, tokenize(code1));
}
void varid_declInIfCondition() {
// if
ASSERT_EQUALS("1: void f ( int x@1 ) {\n"
"2: if ( int x@2 = 0 ) { x@2 ; }\n"
"3: x@1 ;\n"
"4: }\n",
tokenize("void f(int x) {\n"
" if (int x = 0) { x; }\n"
" x;\n"
"}"));
// if, else
ASSERT_EQUALS("1: void f ( int x@1 ) {\n"
"2: if ( int x@2 = 0 ) { x@2 ; }\n"
"3: else { x@2 ; }\n"
"4: x@1 ;\n"
"5: }\n",
tokenize("void f(int x) {\n"
" if (int x = 0) { x; }\n"
" else { x; }\n"
" x;\n"
"}"));
// if, else if
ASSERT_EQUALS("1: void f ( int x@1 ) {\n"
"2: if ( int x@2 = 0 ) { x@2 ; }\n"
"3: else { if ( void * x@3 = & x@3 ) { x@3 ; } }\n"
"4: x@1 ;\n"
"5: }\n",
tokenize("void f(int x) {\n"
" if (int x = 0) x;\n"
" else if (void* x = &x) x;\n"
" x;\n"
"}"));
// if, else if, else
ASSERT_EQUALS("1: void f ( int x@1 ) {\n"
"2: if ( int x@2 = 0 ) { x@2 ; }\n"
"3: else { if ( void * x@3 = & x@3 ) { x@3 ; }\n"
"4: else { x@3 ; } }\n"
"5: x@1 ;\n"
"6: }\n",
tokenize("void f(int x) {\n"
" if (int x = 0) x;\n"
" else if (void* x = &x) x;\n"
" else x;\n"
" x;\n"
"}"));
}
void varidclass1() {
const std::string actual = tokenize(
"class Fred\n"