From b18e6f1e107c69ab5f5b48b9ce878d22d8f32ae8 Mon Sep 17 00:00:00 2001 From: dummyunit Date: Mon, 26 Apr 2021 08:38:03 +0300 Subject: [PATCH] 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. --- lib/tokenize.cpp | 31 ++++++++++++++++++++++++--- test/testunusedvar.cpp | 30 ++++++++++++++++++++++++++ test/testvarid.cpp | 48 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 3 deletions(-) diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 3c97405ae..dfc9a246c 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -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(); diff --git a/test/testunusedvar.cpp b/test/testunusedvar.cpp index d83f34f67..312a44d9c 100644 --- a/test/testunusedvar.cpp +++ b/test/testunusedvar.cpp @@ -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" diff --git a/test/testvarid.cpp b/test/testvarid.cpp index 492050329..7c8b960b3 100644 --- a/test/testvarid.cpp +++ b/test/testvarid.cpp @@ -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"