From 44e8ca9123b11a4035d76584bdc629a947c8e131 Mon Sep 17 00:00:00 2001 From: chrchr Date: Mon, 17 Apr 2023 15:49:40 +0200 Subject: [PATCH 1/5] Fix #551 Detect unused Private member variables --- lib/checkunusedvar.cpp | 17 +++++++++++------ lib/checkunusedvar.h | 2 +- test/testunusedvar.cpp | 8 ++++++++ 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/lib/checkunusedvar.cpp b/lib/checkunusedvar.cpp index 181d2cb9c3b..3d9adf7fac4 100644 --- a/lib/checkunusedvar.cpp +++ b/lib/checkunusedvar.cpp @@ -1437,7 +1437,7 @@ void CheckUnusedVar::checkStructMemberUsage() const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); for (const Scope &scope : symbolDatabase->scopeList) { - if (scope.type != Scope::eStruct && scope.type != Scope::eUnion) + if (scope.type != Scope::eStruct && scope.type != Scope::eClass && scope.type != Scope::eUnion) continue; if (scope.bodyStart->fileIndex() != 0 || scope.className.empty()) @@ -1521,16 +1521,21 @@ void CheckUnusedVar::checkStructMemberUsage() break; } } - if (!use) - unusedStructMemberError(var.nameToken(), scope.className, var.name(), scope.type == Scope::eUnion); + if (!use) { + std::string prefix = "struct"; + if (scope.type == Scope::ScopeType::eClass) + prefix = "class"; + else if (scope.type == Scope::ScopeType::eUnion) + prefix = "union"; + unusedStructMemberError(var.nameToken(), scope.className, var.name(), prefix); + } } } } -void CheckUnusedVar::unusedStructMemberError(const Token *tok, const std::string &structname, const std::string &varname, bool isUnion) +void CheckUnusedVar::unusedStructMemberError(const Token* tok, const std::string& structname, const std::string& varname, const std::string& prefix) { - const std::string prefix = isUnion ? "union member " : "struct member "; - reportError(tok, Severity::style, "unusedStructMember", "$symbol:" + structname + "::" + varname + '\n' + prefix + "'$symbol' is never used.", CWE563, Certainty::normal); + reportError(tok, Severity::style, "unusedStructMember", "$symbol:" + structname + "::" + varname + '\n' + prefix + " member '$symbol' is never used.", CWE563, Certainty::normal); } bool CheckUnusedVar::isRecordTypeWithoutSideEffects(const Type* type) diff --git a/lib/checkunusedvar.h b/lib/checkunusedvar.h index 130cef26a11..743a732ba42 100644 --- a/lib/checkunusedvar.h +++ b/lib/checkunusedvar.h @@ -77,7 +77,7 @@ class CPPCHECKLIB CheckUnusedVar : public Check { std::list checkedFuncs); // Error messages.. - void unusedStructMemberError(const Token *tok, const std::string &structname, const std::string &varname, bool isUnion = false); + void unusedStructMemberError(const Token *tok, const std::string &structname, const std::string &varname, const std::string& prefix = "struct"); void unusedVariableError(const Token *tok, const std::string &varname); void allocatedButUnusedVariableError(const Token *tok, const std::string &varname); void unreadVariableError(const Token *tok, const std::string &varname, bool modified); diff --git a/test/testunusedvar.cpp b/test/testunusedvar.cpp index 362bdfe11f6..4ff19d347a0 100644 --- a/test/testunusedvar.cpp +++ b/test/testunusedvar.cpp @@ -75,6 +75,7 @@ class TestUnusedVar : public TestFixture { TEST_CASE(structmember21); // #4759 TEST_CASE(structmember22); // #11016 TEST_CASE(structmember23); + TEST_CASE(classmember); TEST_CASE(localvar1); TEST_CASE(localvar2); @@ -1870,6 +1871,13 @@ class TestUnusedVar : public TestFixture { ASSERT_EQUALS("", errout.str()); } + void classmember() { + checkStructMemberUsage("class C {\n" + " int i{};\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:2]: (style) class member 'C::i' is never used.\n", errout.str()); + } + void functionVariableUsage_(const char* file, int line, const char code[], const char filename[] = "test.cpp") { // Clear the error buffer.. errout.str(""); From 4d2c4b334b0bc9ef5461098c768b1d000f607a70 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 17 Apr 2023 22:48:08 +0200 Subject: [PATCH 2/5] Fix FN unusedStructMember when there are member functions --- lib/checkunusedvar.cpp | 4 ---- test/testunusedvar.cpp | 7 +++++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/checkunusedvar.cpp b/lib/checkunusedvar.cpp index 3d9adf7fac4..2d69f12e03c 100644 --- a/lib/checkunusedvar.cpp +++ b/lib/checkunusedvar.cpp @@ -1455,10 +1455,6 @@ void CheckUnusedVar::checkStructMemberUsage() continue; } - // Bail out if struct/union contains any functions - if (!scope.functionList.empty()) - continue; - // Bail out for template struct, members might be used in non-matching instantiations if (scope.className.find('<') != std::string::npos) continue; diff --git a/test/testunusedvar.cpp b/test/testunusedvar.cpp index 4ff19d347a0..93e66bc3eb4 100644 --- a/test/testunusedvar.cpp +++ b/test/testunusedvar.cpp @@ -1876,6 +1876,13 @@ class TestUnusedVar : public TestFixture { " int i{};\n" "};\n"); ASSERT_EQUALS("[test.cpp:2]: (style) class member 'C::i' is never used.\n", errout.str()); + + checkStructMemberUsage("class C {\n" + " int i{}, j{};\n" + "public:\n" + " int& get() { return i; }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:2]: (style) class member 'C::j' is never used.\n", errout.str()); } void functionVariableUsage_(const char* file, int line, const char code[], const char filename[] = "test.cpp") { From 0558b2f3ab839e00840d475db3a4524250fc6659 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 17 Apr 2023 23:28:52 +0200 Subject: [PATCH 3/5] Unused member --- lib/checkstl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 77041883e98..481364053d6 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -919,7 +919,7 @@ struct InvalidContainerAnalyzer { const Token* ftok; }; std::unordered_map expressions; - ErrorPath errorPath; + void add(const std::vector& refs) { for (const Reference& r : refs) { add(r); From ffd967af251fbe87c5061477851b05202690f3e0 Mon Sep 17 00:00:00 2001 From: chrchr Date: Tue, 18 Apr 2023 11:38:38 +0200 Subject: [PATCH 4/5] Warn for unused private variables in base class --- lib/checkunusedvar.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/checkunusedvar.cpp b/lib/checkunusedvar.cpp index 3d9adf7fac4..fba0c582769 100644 --- a/lib/checkunusedvar.cpp +++ b/lib/checkunusedvar.cpp @@ -1464,16 +1464,15 @@ void CheckUnusedVar::checkStructMemberUsage() continue; // bail out if struct is inherited - bool bailout = std::any_of(symbolDatabase->scopeList.cbegin(), symbolDatabase->scopeList.cend(), [&](const Scope& derivedScope) { + const bool isInherited = std::any_of(symbolDatabase->scopeList.cbegin(), symbolDatabase->scopeList.cend(), [&](const Scope& derivedScope) { const Type* dType = derivedScope.definedType; return dType && std::any_of(dType->derivedFrom.cbegin(), dType->derivedFrom.cend(), [&](const Type::BaseInfo& derivedFrom) { return derivedFrom.type == scope.definedType; }); }); - if (bailout) - continue; // bail out for extern/global struct + bool bailout = false; for (const Variable* var : symbolDatabase->variableList()) { if (var && (var->isExtern() || (var->isGlobal() && !var->isStatic())) && var->typeEndToken()->str() == scope.className) { bailout = true; @@ -1510,6 +1509,8 @@ void CheckUnusedVar::checkStructMemberUsage() // only warn for variables without side effects if (!var.typeStartToken()->isStandardType() && !var.isPointer() && !astIsContainer(var.nameToken()) && !isRecordTypeWithoutSideEffects(var.type())) continue; + if (isInherited && !var.isPrivate()) + continue; // Check if the struct member variable is used anywhere in the file bool use = false; From 13a4a8e725c6179ad8c7e58595cf25d24269dc45 Mon Sep 17 00:00:00 2001 From: chrchr Date: Tue, 18 Apr 2023 11:51:25 +0200 Subject: [PATCH 5/5] Warn for private inheritance, add test --- lib/checkunusedvar.cpp | 2 +- test/testunusedvar.cpp | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/checkunusedvar.cpp b/lib/checkunusedvar.cpp index dcde9247ad2..5a47ba8d69d 100644 --- a/lib/checkunusedvar.cpp +++ b/lib/checkunusedvar.cpp @@ -1463,7 +1463,7 @@ void CheckUnusedVar::checkStructMemberUsage() const bool isInherited = std::any_of(symbolDatabase->scopeList.cbegin(), symbolDatabase->scopeList.cend(), [&](const Scope& derivedScope) { const Type* dType = derivedScope.definedType; return dType && std::any_of(dType->derivedFrom.cbegin(), dType->derivedFrom.cend(), [&](const Type::BaseInfo& derivedFrom) { - return derivedFrom.type == scope.definedType; + return derivedFrom.type == scope.definedType && derivedFrom.access != AccessControl::Private; }); }); diff --git a/test/testunusedvar.cpp b/test/testunusedvar.cpp index 93e66bc3eb4..d19c2eef38b 100644 --- a/test/testunusedvar.cpp +++ b/test/testunusedvar.cpp @@ -1883,6 +1883,27 @@ class TestUnusedVar : public TestFixture { " int& get() { return i; }\n" "};\n"); ASSERT_EQUALS("[test.cpp:2]: (style) class member 'C::j' is never used.\n", errout.str()); + + checkStructMemberUsage("class C {\n" + "private:\n" + " int i;\n" + "};\n" + "class D : public C {};\n"); + ASSERT_EQUALS("[test.cpp:3]: (style) class member 'C::i' is never used.\n", errout.str()); + + checkStructMemberUsage("class C {\n" + "public:\n" + " int i;\n" + "};\n" + "class D : C {};\n"); + ASSERT_EQUALS("[test.cpp:3]: (style) class member 'C::i' is never used.\n", errout.str()); + + checkStructMemberUsage("class C {\n" + "public:\n" + " int i;\n" + "};\n" + "class D : public C {};\n"); + ASSERT_EQUALS("", errout.str()); } void functionVariableUsage_(const char* file, int line, const char code[], const char filename[] = "test.cpp") {