diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 14ce236d793..2e784a327a2 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -3036,7 +3036,7 @@ void CheckClass::overrideError(const Function *funcInBase, const Function *funcI Certainty::normal); } -void CheckClass::uselessOverrideError(const Function *funcInBase, const Function *funcInDerived) +void CheckClass::uselessOverrideError(const Function *funcInBase, const Function *funcInDerived, bool isSameCode) { const std::string functionName = funcInDerived ? ((funcInDerived->isDestructor() ? "~" : "") + funcInDerived->name()) : ""; const std::string funcType = (funcInDerived && funcInDerived->isDestructor()) ? "destructor" : "function"; @@ -3047,9 +3047,15 @@ void CheckClass::uselessOverrideError(const Function *funcInBase, const Function errorPath.emplace_back(funcInDerived->tokenDef, char(std::toupper(funcType[0])) + funcType.substr(1) + " in derived class"); } + std::string errStr = "\nThe " + funcType + " '$symbol' overrides a " + funcType + " in a base class but "; + if (isSameCode) { + errStr += "is identical to the overridden function"; + } + else + errStr += "just delegates back to the base class."; reportError(errorPath, Severity::style, "uselessOverride", - "$symbol:" + functionName + "\n" - "The " + funcType + " '$symbol' overrides a " + funcType + " in a base class but just delegates back to the base class.", + "$symbol:" + functionName + + errStr, CWE(0U) /* Unknown CWE! */, Certainty::normal); } @@ -3071,6 +3077,23 @@ static const Token* getSingleFunctionCall(const Scope* scope) { return nullptr; } +static bool compareTokenRanges(const Token* start1, const Token* end1, const Token* start2, const Token* end2) { + const Token* tok1 = start1; + const Token* tok2 = start2; + bool isEqual = false; + while (tok1 && tok2) { + if (tok1->str() != tok2->str()) + break; + if (tok1 == end1 && tok2 == end2) { + isEqual = true; + break; + } + tok1 = tok1->next(); + tok2 = tok2->next(); + } + return isEqual; +} + void CheckClass::checkUselessOverride() { if (!mSettings->severity.isEnabled(Severity::style)) @@ -3093,6 +3116,18 @@ void CheckClass::checkUselessOverride() return f.name() == func.name(); })) continue; + if (baseFunc->functionScope) { + bool isSameCode = compareTokenRanges(baseFunc->argDef, baseFunc->argDef->link(), func.argDef, func.argDef->link()); // function arguments + if (isSameCode) { + isSameCode = compareTokenRanges(baseFunc->functionScope->bodyStart, baseFunc->functionScope->bodyEnd, // function body + func.functionScope->bodyStart, func.functionScope->bodyEnd); + + if (isSameCode) { + uselessOverrideError(baseFunc, &func, true); + continue; + } + } + } if (const Token* const call = getSingleFunctionCall(func.functionScope)) { if (call->function() != baseFunc) continue; diff --git a/lib/checkclass.h b/lib/checkclass.h index 49c1cda74d1..3d0b1f8fdfc 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -231,7 +231,7 @@ class CPPCHECKLIB CheckClass : public Check { void duplInheritedMembersError(const Token* tok1, const Token* tok2, const std::string &derivedName, const std::string &baseName, const std::string &variableName, bool derivedIsStruct, bool baseIsStruct); void copyCtorAndEqOperatorError(const Token *tok, const std::string &classname, bool isStruct, bool hasCopyCtor); void overrideError(const Function *funcInBase, const Function *funcInDerived); - void uselessOverrideError(const Function *funcInBase, const Function *funcInDerived); + void uselessOverrideError(const Function *funcInBase, const Function *funcInDerived, bool isSameCode = false); void thisUseAfterFree(const Token *self, const Token *free, const Token *use); void unsafeClassRefMemberError(const Token *tok, const std::string &varname); void checkDuplInheritedMembersRecursive(const Type* typeCurrent, const Type* typeBase); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 369a143af2f..2c6bdac7e31 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -7239,10 +7239,6 @@ struct MultiValueFlowAnalyzer : ValueFlowAnalyzer { return false; } - bool isGlobal() const override { - return false; - } - bool lowerToPossible() override { for (auto&& p:values) { if (p.second.isImpossible()) diff --git a/test/testclass.cpp b/test/testclass.cpp index 995e343835d..34a20b22625 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -8449,6 +8449,22 @@ class TestClass : public TestFixture { " }\n" "};"); ASSERT_EQUALS("", errout.str()); + + checkUselessOverride("struct B {\n" + " virtual int f() { return 1; }\n" + " virtual int g() { return 7; }\n" + " virtual int h(int i, int j) { return i + j; }\n" + " virtual int j(int i, int j) { return i + j; }\n" + "};\n" + "struct D : B {\n" + " int f() override { return 2; }\n" + " int g() override { return 7; }\n" + " int h(int j, int i) override { return i + j; }\n" + " int j(int i, int j) override { return i + j; }\n" + "};"); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:9]: (style) The function 'g' overrides a function in a base class but is identical to the overridden function\n" + "[test.cpp:5] -> [test.cpp:11]: (style) The function 'j' overrides a function in a base class but is identical to the overridden function\n", + errout.str()); } #define checkUnsafeClassRefMember(code) checkUnsafeClassRefMember_(code, __FILE__, __LINE__)