Skip to content

Commit e8a1b2c

Browse files
ludviggunneludviggunne
authored
Fix #15105: False negative: uninitMemberVar with inherited constructor (#8937)
1 parent ebadd92 commit e8a1b2c

3 files changed

Lines changed: 242 additions & 4 deletions

File tree

‎lib/checkclass.cpp‎

Lines changed: 91 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,65 @@ void CheckClassImpl::handleUnionMembers(std::vector<Usage>& usageList)
195195
}
196196
}
197197

198+
static std::vector<const Token *> getInheritedConstructors(const Scope *scope)
199+
{
200+
std::vector<const Token *> result;
201+
202+
if (!scope || !scope->bodyStart || !scope->definedType)
203+
return result;
204+
205+
const auto isInherited = [](const Function &baseCtor, const Scope *scope) {
206+
if (baseCtor.type != FunctionType::eConstructor || baseCtor.argCount() == 0)
207+
return false;
208+
if (baseCtor.isDelete() || baseCtor.access == AccessControl::Private)
209+
return false;
210+
return std::none_of(scope->functionList.cbegin(), scope->functionList.cend(), [&](const Function &func) {
211+
return func.isConstructor() && func.argCount() == baseCtor.argCount() &&
212+
func.argsMatch(baseCtor.nestedIn, baseCtor.argDef, func.argDef, "", 0);
213+
});
214+
};
215+
216+
const std::vector<Type::BaseInfo> &bases = scope->definedType->derivedFrom;
217+
const Token *baseName = nullptr;
218+
const std::list<Function> *baseFuncs = nullptr;
219+
for (const Token *tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
220+
if (Token::Match(tok, "using %name%")) {
221+
const auto base = std::find_if(
222+
bases.cbegin(),
223+
bases.cend(),
224+
[&tok](const Type::BaseInfo &info) {
225+
return info.name == tok->strAt(1);
226+
}
227+
);
228+
if (base != bases.cend() && base->type && base->type->classScope) {
229+
baseName = tok->next();
230+
baseFuncs = &base->type->classScope->functionList;
231+
}
232+
continue;
233+
}
234+
235+
if (tok->str() == ";") {
236+
baseName = nullptr;
237+
continue;
238+
}
239+
240+
if (tok->str() == "{") {
241+
baseName = nullptr;
242+
tok = tok->link();
243+
continue;
244+
}
245+
246+
if (baseName && Token::Match(tok, "%name% :: %name% ;") &&
247+
tok->str() == baseName->str() && tok->strAt(2) == baseName->str() &&
248+
std::any_of(baseFuncs->cbegin(), baseFuncs->cend(), [&](const Function &func) {
249+
return isInherited(func, scope);
250+
}))
251+
result.push_back(tok->tokAt(2));
252+
}
253+
254+
return result;
255+
}
256+
198257
//---------------------------------------------------------------------------
199258
// ClassCheck: Check that all class constructors are ok.
200259
//---------------------------------------------------------------------------
@@ -223,9 +282,11 @@ void CheckClassImpl::constructors()
223282
});
224283
});
225284

285+
const std::vector<const Token *> inheritedCtors = getInheritedConstructors(scope);
286+
226287
// There are no constructors.
227288
std::set<const Variable*> diagVars;
228-
if (scope->numConstructors == 0 && printStyle && !usedInUnion) {
289+
if (scope->numConstructors == 0 && inheritedCtors.empty() && printStyle && !usedInUnion) {
229290
// If there is a private variable, there should be a constructor..
230291
int needInit = 0, haveInit = 0;
231292
std::vector<const Variable*> uninitVars;
@@ -339,6 +400,30 @@ void CheckClassImpl::constructors()
339400
}
340401
}
341402

403+
if (!inheritedCtors.empty()) {
404+
clearAllVar(usageList);
405+
406+
for (Usage &usage : usageList) {
407+
if (usage.var->hasDefault())
408+
usage.init = true;
409+
}
410+
411+
handleUnionMembers(usageList);
412+
413+
// Inherited constructors don't initialize members of the derived class
414+
for (const Token *baseCtorTok : inheritedCtors) {
415+
for (const Usage &usage : usageList) {
416+
if (isInitialized(usage, FunctionType::eConstructor))
417+
continue;
418+
const Variable& var = *usage.var;
419+
if (var.typeScope() && var.typeScope()->numConstructors > 0)
420+
continue;
421+
uninitVarError(baseCtorTok, false, FunctionType::eConstructor, var.scope()->className, var.name(), scope != var.scope(), false, false, true);
422+
diagVars.emplace(&var);
423+
}
424+
}
425+
}
426+
342427
if (scope->numConstructors == 0) {
343428

344429
// Mark all variables not used
@@ -1182,12 +1267,15 @@ void CheckClassImpl::noExplicitConstructorError(const Token *tok, const std::str
11821267
reportError(tok, Severity::style, "noExplicitConstructor", "$symbol:" + classname + '\n' + message + '\n' + verbose, CWE398, Certainty::normal);
11831268
}
11841269

1185-
void CheckClassImpl::uninitVarError(const Token *tok, bool isprivate, FunctionType functionType, const std::string &classname, const std::string &varname, bool derived, bool inconclusive, bool noCtor)
1270+
void CheckClassImpl::uninitVarError(const Token *tok, bool isprivate, FunctionType functionType, const std::string &classname, const std::string &varname, bool derived, bool inconclusive, bool noCtor, bool inheritedCtor)
11861271
{
11871272
std::string message("Member variable '$symbol' ");
11881273
if (noCtor)
11891274
message += "has no initializer.";
1190-
else {
1275+
else if (inheritedCtor) {
1276+
message += tok ? "is not initialized in the constructor inherited from '" + tok->str() + "'." :
1277+
"is not initialized in the inherited constructor.";
1278+
} else {
11911279
message += "is not initialized in the ";
11921280
if (functionType == FunctionType::eCopyConstructor)
11931281
message += "copy ";

‎lib/checkclass.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ class CPPCHECKLIB CheckClassImpl : public CheckImpl {
182182
void noCopyConstructorError(const Scope *scope, bool isdefault, const Token *alloc, bool inconclusive);
183183
void noOperatorEqError(const Scope *scope, bool isdefault, const Token *alloc, bool inconclusive);
184184
void noDestructorError(const Scope *scope, bool isdefault, const Token *alloc);
185-
void uninitVarError(const Token *tok, bool isprivate, FunctionType functionType, const std::string &classname, const std::string &varname, bool derived, bool inconclusive, bool noCtor = false);
185+
void uninitVarError(const Token *tok, bool isprivate, FunctionType functionType, const std::string &classname, const std::string &varname, bool derived, bool inconclusive, bool noCtor = false, bool inheritedCtor = false);
186186
void uninitVarError(const Token *tok, const std::string &classname, const std::string &varname);
187187
void missingMemberCopyError(const Token *tok, FunctionType functionType, const std::string& classname, const std::string& varname);
188188
void operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive);

‎test/testconstructors.cpp‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,18 @@ class TestConstructors : public TestFixture {
232232
TEST_CASE(uninitTemplate1); // ticket #7372
233233

234234
TEST_CASE(unknownTemplateType);
235+
236+
TEST_CASE(inheritedCtor1);
237+
TEST_CASE(inheritedCtor2);
238+
TEST_CASE(inheritedCtor3);
239+
TEST_CASE(inheritedCtor4);
240+
TEST_CASE(inheritedCtor5);
241+
TEST_CASE(inheritedCtor6);
242+
TEST_CASE(inheritedCtor7);
243+
TEST_CASE(inheritedCtor8);
244+
TEST_CASE(inheritedCtor9);
245+
TEST_CASE(inheritedCtor10);
246+
TEST_CASE(inheritedCtor11);
235247
}
236248

237249

@@ -4732,6 +4744,144 @@ class TestConstructors : public TestFixture {
47324744
ASSERT_EQUALS("", errout_str());
47334745
}
47344746

4747+
void inheritedCtor1() {
4748+
check("struct Base { Base(int); };\n"
4749+
"struct Derived : Base {\n"
4750+
" using Base::Base;\n"
4751+
" int x;\n"
4752+
"};\n");
4753+
ASSERT_EQUALS("[test.cpp:3:17]: (warning) Member variable 'Derived::x' is not initialized in the constructor inherited from 'Base'. [uninitMemberVar]\n", errout_str());
4754+
}
4755+
4756+
void inheritedCtor2() {
4757+
check("struct Base { Base(int); };\n"
4758+
"struct Derived : Base {\n"
4759+
" using Base::Base;\n"
4760+
" int x = 0;\n"
4761+
" int y{};\n"
4762+
"};\n");
4763+
ASSERT_EQUALS("", errout_str());
4764+
}
4765+
4766+
void inheritedCtor3() {
4767+
check("struct Base { Base(int); };\n"
4768+
"struct S { S(); int i; };\n"
4769+
"struct Derived : Base {\n"
4770+
" using Base::Base;\n"
4771+
" S s;\n"
4772+
"};\n");
4773+
ASSERT_EQUALS("", errout_str());
4774+
}
4775+
4776+
void inheritedCtor4() {
4777+
check("struct Base { Base(int); };\n"
4778+
"struct Derived : Base {\n"
4779+
" using Base::Base;\n"
4780+
" int x = 0;\n"
4781+
" int y;\n"
4782+
"};\n");
4783+
ASSERT_EQUALS("[test.cpp:3:17]: (warning) Member variable 'Derived::y' is not initialized in the constructor inherited from 'Base'. [uninitMemberVar]\n", errout_str());
4784+
}
4785+
4786+
void inheritedCtor5() {
4787+
check("struct Base { Base(int); };\n"
4788+
"struct Derived : Base {\n"
4789+
" using Base::Base;\n"
4790+
" Derived() : x(0) {}\n"
4791+
" int x;\n"
4792+
"};\n");
4793+
ASSERT_EQUALS("[test.cpp:3:17]: (warning) Member variable 'Derived::x' is not initialized in the constructor inherited from 'Base'. [uninitMemberVar]\n", errout_str());
4794+
}
4795+
4796+
void inheritedCtor6() {
4797+
check("struct A { A(int); };\n"
4798+
"struct B { B(double); };\n"
4799+
"struct D : A, B {\n"
4800+
" using A::A;\n"
4801+
" using B::B;\n"
4802+
" int x;\n"
4803+
"};\n");
4804+
ASSERT_EQUALS("[test.cpp:4:14]: (warning) Member variable 'D::x' is not initialized in the constructor inherited from 'A'. [uninitMemberVar]\n"
4805+
"[test.cpp:5:14]: (warning) Member variable 'D::x' is not initialized in the constructor inherited from 'B'. [uninitMemberVar]\n",
4806+
errout_str());
4807+
}
4808+
4809+
void inheritedCtor7() {
4810+
check("struct Base { void f(); };\n"
4811+
"struct Derived : Base {\n"
4812+
" using Base::f;\n"
4813+
" int x = 0;\n"
4814+
" int y;\n"
4815+
"};\n");
4816+
ASSERT_EQUALS("[test.cpp:5:9]: (warning) Member variable 'Derived::y' has no initializer. [uninitMemberVarNoCtor]\n", errout_str());
4817+
}
4818+
4819+
void inheritedCtor8() {
4820+
check("struct Base { Base(int); };\n"
4821+
"struct Derived : Base {\n"
4822+
" using Base::Base;\n"
4823+
" Derived(int i) : Base(i), x(0) {}\n"
4824+
" int x;\n"
4825+
"};\n");
4826+
ASSERT_EQUALS("", errout_str());
4827+
4828+
check("struct Base { Base(int); Base(double); };\n"
4829+
"struct Derived : Base {\n"
4830+
" using Base::Base;\n"
4831+
" Derived(int i) : Base(i), x(0) {}\n"
4832+
" int x;\n"
4833+
"};\n");
4834+
ASSERT_EQUALS("[test.cpp:3:17]: (warning) Member variable 'Derived::x' is not initialized in the constructor inherited from 'Base'. [uninitMemberVar]\n", errout_str());
4835+
}
4836+
4837+
void inheritedCtor9() {
4838+
check("struct Base { Base(); };\n"
4839+
"struct Derived : Base {\n"
4840+
" using Base::Base;\n"
4841+
" int x;\n"
4842+
"};\n");
4843+
ASSERT_EQUALS("", errout_str());
4844+
4845+
check("struct Base {};\n"
4846+
"struct Derived : Base {\n"
4847+
" using Base::Base;\n"
4848+
" int x;\n"
4849+
"};\n");
4850+
ASSERT_EQUALS("", errout_str());
4851+
4852+
check("struct Base { Base(const Base&); Base(Base&&); };\n"
4853+
"struct Derived : Base {\n"
4854+
" using Base::Base;\n"
4855+
" int x;\n"
4856+
"};\n");
4857+
ASSERT_EQUALS("", errout_str());
4858+
}
4859+
4860+
void inheritedCtor10() {
4861+
check("struct Base { Base(int) = delete; };\n"
4862+
"struct Derived : Base {\n"
4863+
" using Base::Base;\n"
4864+
" int x;\n"
4865+
"};\n");
4866+
ASSERT_EQUALS("", errout_str());
4867+
4868+
check("class Base { Base(int); };\n"
4869+
"struct Derived : Base {\n"
4870+
" using Base::Base;\n"
4871+
" int x;\n"
4872+
"};\n");
4873+
ASSERT_EQUALS("", errout_str());
4874+
}
4875+
4876+
void inheritedCtor11() {
4877+
check("namespace X { struct Base { Base(int); }; }\n"
4878+
"struct Derived : X::Base {\n"
4879+
" using X::Base::Base;\n"
4880+
" int x;\n"
4881+
"};\n");
4882+
ASSERT_EQUALS("[test.cpp:3:20]: (warning) Member variable 'Derived::x' is not initialized in the constructor inherited from 'Base'. [uninitMemberVar]\n", errout_str());
4883+
}
4884+
47354885
};
47364886

47374887
REGISTER_TEST(TestConstructors)

0 commit comments

Comments
 (0)