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

Fix uselessOverride FPs (#5223)

This commit is contained in:
chrchr-github 2023-07-07 13:18:00 +02:00 committed by GitHub
parent fa03f49d2b
commit c738627d15
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 77 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
lib/checkclass.cpp
View File

@ -2886,8 +2886,18 @@ void CheckClass::checkDuplInheritedMembers()
}
}
void CheckClass::checkDuplInheritedMembersRecursive(const Type* typeCurrent, const Type* typeBase)
namespace {
struct DuplMemberInfo {
DuplMemberInfo(const Variable* cv, const Variable* pcv, const Type::BaseInfo* pc) : classVar(cv), parentClassVar(pcv), parentClass(pc) {}
const Variable* classVar;
const Variable* parentClassVar;
const Type::BaseInfo* parentClass;
};
}
static std::vector<DuplMemberInfo> hasDuplInheritedMembersRecursive(const Type* typeCurrent, const Type* typeBase)
{
std::vector<DuplMemberInfo> results;
for (const Type::BaseInfo &parentClassIt : typeBase->derivedFrom) {
// Check if there is info about the 'Base' class
if (!parentClassIt.type || !parentClassIt.type->classScope)
@ -2898,16 +2908,26 @@ void CheckClass::checkDuplInheritedMembersRecursive(const Type* typeCurrent, con
// Check if they have a member variable in common
for (const Variable &classVarIt : typeCurrent->classScope->varlist) {
for (const Variable &parentClassVarIt : parentClassIt.type->classScope->varlist) {
if (classVarIt.name() == parentClassVarIt.name() && !parentClassVarIt.isPrivate()) { // Check if the class and its parent have a common variable
duplInheritedMembersError(classVarIt.nameToken(), parentClassVarIt.nameToken(),
typeCurrent->name(), parentClassIt.type->name(), classVarIt.name(),
typeCurrent->classScope->type == Scope::eStruct,
parentClassIt.type->classScope->type == Scope::eStruct);
}
if (classVarIt.name() == parentClassVarIt.name() && !parentClassVarIt.isPrivate()) // Check if the class and its parent have a common variable
results.emplace_back(&classVarIt, &parentClassVarIt, &parentClassIt);
}
}
if (typeCurrent != parentClassIt.type)
checkDuplInheritedMembersRecursive(typeCurrent, parentClassIt.type);
if (typeCurrent != parentClassIt.type) {
const auto recursive = hasDuplInheritedMembersRecursive(typeCurrent, parentClassIt.type);
results.insert(results.end(), recursive.begin(), recursive.end());
}
}
return results;
}
void CheckClass::checkDuplInheritedMembersRecursive(const Type* typeCurrent, const Type* typeBase)
{
const auto results = hasDuplInheritedMembersRecursive(typeCurrent, typeBase);
for (const auto& r : results) {
duplInheritedMembersError(r.classVar->nameToken(), r.parentClassVar->nameToken(),
typeCurrent->name(), r.parentClass->type->name(), r.classVar->name(),
typeCurrent->classScope->type == Scope::eStruct,
r.parentClass->type->classScope->type == Scope::eStruct);
}
}
@ -3084,6 +3104,8 @@ static bool compareTokenRanges(const Token* start1, const Token* end1, const Tok
while (tok1 && tok2) {
if (tok1->str() != tok2->str())
break;
if (tok1->str() == "this")
break;
if (tok1 == end1 && tok2 == end2) {
isEqual = true;
break;
@ -3116,6 +3138,10 @@ void CheckClass::checkUselessOverride()
return f.name() == func.name();
}))
continue;
if (func.token->isExpandedMacro() || baseFunc->token->isExpandedMacro())
continue;
if (!classScope->definedType || !hasDuplInheritedMembersRecursive(classScope->definedType, classScope->definedType).empty()) // bailout for shadowed member variables
continue;
if (baseFunc->functionScope) {
bool isSameCode = compareTokenRanges(baseFunc->argDef, baseFunc->argDef->link(), func.argDef, func.argDef->link()); // function arguments
if (isSameCode) {

46
test/testclass.cpp
View File

@ -8347,12 +8347,20 @@ private:
const Settings settings = settingsBuilder().severity(Severity::style).build();
Preprocessor preprocessor(settings);
// Raw tokens..
std::vector<std::string> files(1, "test.cpp");
std::istringstream istr(code);
const simplecpp::TokenList tokens1(istr, files, files[0]);
// Preprocess..
simplecpp::TokenList tokens2(files);
std::map<std::string, simplecpp::TokenList*> filedata;
simplecpp::preprocess(tokens2, tokens1, files, filedata, simplecpp::DUI());
// Tokenize..
Tokenizer tokenizer(&settings, this, &preprocessor);
std::istringstream istr(code);
ASSERT_LOC(tokenizer.tokenize(istr, "test.cpp"), file, line);
Tokenizer tokenizer(&settings, this);
tokenizer.createTokens(std::move(tokens2));
ASSERT_LOC(tokenizer.simplifyTokens1(""), file, line);
// Check..
CheckClass checkClass(&tokenizer, &settings, this);
@ -8465,6 +8473,36 @@ private:
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());
checkUselessOverride("struct B : std::exception {\n"
" virtual void f() { throw *this; }\n"
"};\n"
"struct D : B {\n"
" void f() override { throw *this; }\n"
"};\n");
ASSERT_EQUALS("", errout.str());
checkUselessOverride("#define MACRO virtual void f() {}\n"
"struct B {\n"
" MACRO\n"
"};\n"
"struct D : B {\n"
" MACRO\n"
"};\n");
ASSERT_EQUALS("", errout.str());
checkUselessOverride("struct B {\n"
" B() = default;\n"
" explicit B(int i) : m(i) {}\n"
" int m{};\n"
" virtual int f() const { return m; }\n"
"};\n"
"struct D : B {\n"
" explicit D(int i) : m(i) {}\n"
" int m{};\n"
" int f() const override { return m; }\n"
"};\n");
ASSERT_EQUALS("", errout.str());
}
#define checkUnsafeClassRefMember(code) checkUnsafeClassRefMember_(code, __FILE__, __LINE__)