Skip to content

Fix couple issues with missing operatorEqVarError #4576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danmar merged 2 commits into from
Nov 5, 2022
Merged

Fix couple issues with missing operatorEqVarError #4576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a67696d
Don't assume const or static member functions can initialize member v…
gerboe-of Nov 4, 2022
5581a8f
Let base class call to operator= only assign members in that base class
gerboe-of Nov 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 27 additions & 6 deletions lib/checkclass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,21 @@ void CheckClass::assignAllVar(std::vector<Usage> &usageList)
i.assign = true;
}

void CheckClass::assignAllVarsVisibleFromScope(std::vector<Usage>& usageList, const Scope* scope)
{
for (Usage& usage : usageList) {
if (usage.var->scope() == scope)
usage.assign = true;
}

// Iterate through each base class...
for (const Type::BaseInfo& i : scope->definedType->derivedFrom) {
const Type *derivedFrom = i.type;

assignAllVarsVisibleFromScope(usageList, derivedFrom->classScope);
Copy link
Copy Markdown
Collaborator

@chrchr-github chrchr-github Nov 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like derivedFrom needs to be checked for nullptr, see http://cppcheck1.osuosl.org:8000/crash.html

}
}

void CheckClass::clearAllVar(std::vector<Usage> &usageList)
{
for (Usage & i : usageList) {
Expand All @@ -694,7 +709,7 @@ void CheckClass::clearAllVar(std::vector<Usage> &usageList)
}
}

bool CheckClass::isBaseClassFunc(const Token *tok, const Scope *scope)
bool CheckClass::isBaseClassMutableMemberFunc(const Token *tok, const Scope *scope)
{
// Iterate through each base class...
for (const Type::BaseInfo & i : scope->definedType->derivedFrom) {
Expand All @@ -705,11 +720,11 @@ bool CheckClass::isBaseClassFunc(const Token *tok, const Scope *scope)
const std::list<Function>& functionList = derivedFrom->classScope->functionList;

if (std::any_of(functionList.begin(), functionList.end(), [&](const Function& func) {
return func.tokenDef->str() == tok->str();
return func.tokenDef->str() == tok->str() && !func.isStatic() && !func.isConst();
}))
return true;

if (isBaseClassFunc(tok, derivedFrom->classScope))
if (isBaseClassMutableMemberFunc(tok, derivedFrom->classScope))
return true;
}

Expand Down Expand Up @@ -894,6 +909,12 @@ void CheckClass::initializeVarList(const Function &func, std::list<const Functio
callstack.pop_back();
}

// assume that a base class call to operator= assigns all its base members (but not more)
else if (func.tokenDef->str() == ftok->str() && isBaseClassMutableMemberFunc(ftok, scope)) {
if (member->nestedIn)
assignAllVarsVisibleFromScope(usage, member->nestedIn->definedType->classScope);
}

// there is a called member function, but it has no implementation, so we assume it initializes everything
else {
assignAllVar(usage);
Expand Down Expand Up @@ -949,16 +970,16 @@ void CheckClass::initializeVarList(const Function &func, std::list<const Functio
}
}

// there is a called member function, but it has no implementation, so we assume it initializes everything
else {
// there is a called member function, but it has no implementation, so we assume it initializes everything (if it can mutate state)
else if (!member->isConst() && !member->isStatic()) {
assignAllVar(usage);
}
}

// not member function
else {
// could be a base class virtual function, so we assume it initializes everything
if (!func.isConstructor() && isBaseClassFunc(ftok, scope)) {
if (!func.isConstructor() && isBaseClassMutableMemberFunc(ftok, scope)) {
/** @todo False Negative: we should look at the base class functions to see if they
* call any derived class virtual functions that change the derived class state
*/
Expand Down
9 changes: 8 additions & 1 deletion lib/checkclass.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ class CPPCHECKLIB CheckClass : public Check {
bool init;
};

static bool isBaseClassFunc(const Token *tok, const Scope *scope);
static bool isBaseClassMutableMemberFunc(const Token *tok, const Scope *scope);

/**
* @brief Create usage list that contains all scope members and also members
Expand Down Expand Up @@ -372,6 +372,13 @@ class CPPCHECKLIB CheckClass : public Check {
*/
static void assignAllVar(std::vector<Usage> &usageList);

/**
* @brief set all variable in list assigned, if visible from given scope
* @param usageList reference to usage vector
* @param scope scope from which usages must be visible
*/
static void assignAllVarsVisibleFromScope(std::vector<Usage> &usageList, const Scope *scope);

/**
* @brief set all variables in list not assigned and not initialized
* @param usageList reference to usage vector
Expand Down
57 changes: 57 additions & 0 deletions test/testconstructors.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ class TestConstructors : public TestFixture {
TEST_CASE(initvar_operator_eq5); // ticket #4119
TEST_CASE(initvar_operator_eq6);
TEST_CASE(initvar_operator_eq7);
TEST_CASE(initvar_operator_eq8);
TEST_CASE(initvar_same_classname); // BUG 2208157
TEST_CASE(initvar_chained_assign); // BUG 2270433
TEST_CASE(initvar_2constructors); // BUG 2270353
Expand Down Expand Up @@ -981,6 +982,30 @@ class TestConstructors : public TestFixture {
ASSERT_EQUALS("", errout.str());
}

void initvar_operator_eq8() {
check("struct B {\n"
" int b;\n"
"};\n"
"struct D1 : B {\n"
" D1& operator=(const D1& src);\n"
" int d1;\n"
"};\n"
"struct D2 : D1 {\n"
" D2& operator=(const D2& src);\n"
" int d2;\n"
"};\n"
"struct D3 : D2 {\n"
" D3& operator=(const D3& src) {\n"
" D1::operator=(src);\n"
" d3_1 = src.d3_1;\n"
" }\n"
" int d3_1;\n"
" int d3_2;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:13]: (warning) Member variable 'D3::d3_2' is not assigned a value in 'D3::operator='.\n"
"[test.cpp:13]: (warning) Member variable 'D3::d2' is not assigned a value in 'D3::operator='.\n", errout.str());
}

void initvar_same_classname() {
// Bug 2208157 - False positive: Uninitialized variable, same class name

Expand Down Expand Up @@ -3324,6 +3349,38 @@ class TestConstructors : public TestFixture {
"};");
ASSERT_EQUALS("[test.cpp:5]: (warning) Member variable 'Fred::i' is not initialized in the constructor.\n", errout.str());

// Unknown member functions and unknown static functions
check("class ABC {\n"
" static void static_base_func();\n"
" void const_base_func() const;\n"
"};\n"
"class Fred : private ABC {\n"
"public:\n"
" Fred() {\n"
" const_func();\n"
" static_func();\n"
" const_base_func();\n"
" ABC::static_base_func();\n"
" }\n"
" void const_func() const;\n"
" static void static_f();\n"
"private:\n"
" int i;\n"
"};");

// Unknown overloaded member functions
check("class Fred : private ABC {\n"
"public:\n"
" Fred() {\n"
" func();\n"
" }\n"
" void func() const;\n"
" void func();\n"
"private:\n"
" int i;\n"
"};");
ASSERT_EQUALS("", errout.str());

}

void uninitVarEnum1() {
Expand Down