From 682a6d1c02c9c5e588195f72df1ccfe04cd60a4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Thu, 23 Jul 2020 11:10:08 +0200 Subject: [PATCH] Fixed #9017 (Simple classes without side effects not reported as unused) --- lib/checkunusedvar.cpp | 52 ++++++++++++++++------ test/testunusedvar.cpp | 97 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 13 deletions(-) diff --git a/lib/checkunusedvar.cpp b/lib/checkunusedvar.cpp index d54d29f98..7bd5c9de8 100644 --- a/lib/checkunusedvar.cpp +++ b/lib/checkunusedvar.cpp @@ -1442,24 +1442,50 @@ bool CheckUnusedVar::isRecordTypeWithoutSideEffects(const Type* type) const std::pair::iterator,bool> found=mIsRecordTypeWithoutSideEffectsMap.insert( std::pair(type,false)); //Initialize with side effects for possible recursions - bool & withoutSideEffects=found.first->second; + bool & withoutSideEffects = found.first->second; if (!found.second) return withoutSideEffects; - if (type && type->classScope && type->classScope->numConstructors == 0 && - (type->classScope->varlist.empty() || type->needInitialization == Type::NeedInitialization::True)) { - for (std::vector::const_iterator i = type->derivedFrom.begin(); i != type->derivedFrom.end(); ++i) { - if (!isRecordTypeWithoutSideEffects(i->type)) { - withoutSideEffects=false; - return withoutSideEffects; - } - } - withoutSideEffects=true; - return withoutSideEffects; + // unknown types are assumed to have side effects + if (!type || !type->classScope) + return (withoutSideEffects = false); + + // Non-empty constructors => possible side effects + for (const Function& f : type->classScope->functionList) { + if (!f.isConstructor()) + continue; + if (f.argDef && Token::Match(f.argDef->link(), ") =")) + continue; // ignore default/deleted constructors + const bool emptyBody = (f.functionScope && Token::simpleMatch(f.functionScope->bodyStart, "{ }")); + const bool hasInitList = f.argDef && Token::simpleMatch(f.argDef->link(), ") :"); + if (!emptyBody || hasInitList) + return (withoutSideEffects = false); } - withoutSideEffects=false; // unknown types are assumed to have side effects - return withoutSideEffects; + // Derived from type that has side effects? + for (const Type::BaseInfo& derivedFrom : type->derivedFrom) { + if (!isRecordTypeWithoutSideEffects(derivedFrom.type)) + return (withoutSideEffects = false); + } + + // Is there a member variable with possible side effects + for (const Variable& var : type->classScope->varlist) { + if (var.isPointer()) + continue; + + const Type* variableType = var.type(); + if (variableType) { + if (!isRecordTypeWithoutSideEffects(variableType)) + return (withoutSideEffects = false); + } else { + ValueType::Type valueType = var.valueType()->type; + if ((valueType == ValueType::Type::UNKNOWN_TYPE) || (valueType == ValueType::Type::NONSTD)) + return (withoutSideEffects = false); + } + } + + + return (withoutSideEffects = true); } bool CheckUnusedVar::isEmptyType(const Type* type) diff --git a/test/testunusedvar.cpp b/test/testunusedvar.cpp index 7450c38fd..4d91daac2 100644 --- a/test/testunusedvar.cpp +++ b/test/testunusedvar.cpp @@ -38,6 +38,8 @@ private: settings.checkLibrary = true; LOAD_LIB_2(settings.library, "std.cfg"); + TEST_CASE(isRecordTypeWithoutSideEffects); + TEST_CASE(emptyclass); // #5355 - False positive: Variable is not assigned a value. TEST_CASE(emptystruct); // #5355 - False positive: Variable is not assigned a value. @@ -232,6 +234,101 @@ private: checkUnusedVar.checkStructMemberUsage(); } + void isRecordTypeWithoutSideEffects() { + functionVariableUsage( + "class A {};\n" + "void f() {\n" + " A a;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (style) Unused variable: a\n", errout.str()); + + functionVariableUsage( + "class A {};\n" + "class B {\n" + "public:\n" + " A a;\n" + "};\n" + "void f() {\n" + " B b;\n" + "}"); + ASSERT_EQUALS("[test.cpp:7]: (style) Unused variable: b\n", errout.str()); + + functionVariableUsage( + "class C {\n" + "public:\n" + " C() = default;\n" + "};\n" + "void f() {\n" + " C c;\n" + "}"); + ASSERT_EQUALS("[test.cpp:6]: (style) Unused variable: c\n", errout.str()); + + functionVariableUsage( + "class D {\n" + "public:\n" + " D() {}\n" + "};\n" + "void f() {\n" + " D d;\n" + "}"); + ASSERT_EQUALS("[test.cpp:6]: (style) Unused variable: d\n", errout.str()); + + functionVariableUsage( + "class E {\n" + "public:\n" + " uint32_t u{1};\n" + "};\n" + "void f() {\n" + " E e;\n" + "}"); + ASSERT_EQUALS("[test.cpp:6]: (style) Unused variable: e\n", errout.str()); + + // non-empty constructor + functionVariableUsage( + "class F {\n" + "public:\n" + " F() {\n" + " int i =0;\n" + " (void) i;" + " }\n" + "};\n" + "void f() {\n" + " F f;\n" + "}"); + TODO_ASSERT_EQUALS("error", "", errout.str()); + + // side-effect variable + functionVariableUsage( + "class F {\n" + "public:\n" + " F() {\n" + " int i =0;\n" + " (void) i;" + " }\n" + "};\n" + "class G {\n" + "public:\n" + " F f;\n" + "};\n" + "void f() {\n" + " G g;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + // unknown variable type + functionVariableUsage( + "class H {\n" + "public:\n" + " unknown_type u{1};\n" + "};\n" + "void f() {\n" + " H h;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + + } + // #5355 - False positive: Variable is not assigned a value. void emptyclass() { functionVariableUsage("class Carla {\n"