From 3a5931b417398da71ac62bdfa590d6c444c2fd63 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Mon, 7 Mar 2022 08:39:19 +0100 Subject: [PATCH] Fix #5499 C++11 default values not for all class fields and missing constructor (#3876) --- lib/checkclass.cpp | 24 ++++++++++++++++++------ lib/checkclass.h | 2 +- test/testconstructors.cpp | 26 ++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 0bcfe25e5..0dfc8b9ef 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -163,13 +163,25 @@ void CheckClass::constructors() // There are no constructors. if (scope->numConstructors == 0 && printStyle && !usedInUnion) { // If there is a private variable, there should be a constructor.. + int needInit = 0, haveInit = 0; + std::vector uninitVars; for (const Variable &var : scope->varlist) { - if (var.isPrivate() && !var.isStatic() && !var.isInit() && !var.hasDefault() && + if (var.isPrivate() && !var.isStatic() && (!var.isClass() || (var.type() && var.type()->needInitialization == Type::NeedInitialization::True))) { - noConstructorError(scope->classDef, scope->className, scope->classDef->str() == "struct"); - break; + ++needInit; + if (!var.isInit() && !var.hasDefault()) + uninitVars.emplace_back(&var); + else + ++haveInit; } } + if (needInit > haveInit) { + if (haveInit == 0) + noConstructorError(scope->classDef, scope->className, scope->classDef->str() == "struct"); + else + for (const Variable* uv : uninitVars) + uninitVarError(uv->typeStartToken(), /*isprivate*/ true, uv->scope()->className, uv->name(), /*derived*/ false, /*inconclusive*/ false, /*inConstructor*/ false); + } } if (!printWarnings) @@ -998,10 +1010,10 @@ void CheckClass::noExplicitConstructorError(const Token *tok, const std::string reportError(tok, Severity::style, "noExplicitConstructor", "$symbol:" + classname + '\n' + message + '\n' + verbose, CWE398, Certainty::normal); } -void CheckClass::uninitVarError(const Token *tok, bool isprivate, const std::string &classname, const std::string &varname, bool derived, bool inconclusive) +void CheckClass::uninitVarError(const Token *tok, bool isprivate, const std::string &classname, const std::string &varname, bool derived, bool inconclusive, bool inConstructor) { - std::string message; - message = "Member variable '$symbol' is not initialized in the constructor."; + std::string message("Member variable '$symbol' is not initialized"); + message += inConstructor ? " in the constructor." : "."; if (derived) message += " Maybe it should be initialized directly in the class " + classname + "?"; std::string id = std::string("uninit") + (derived ? "Derived" : "") + "MemberVar" + (isprivate ? "Private" : ""); diff --git a/lib/checkclass.h b/lib/checkclass.h index 97808a5ab..bc305d2a8 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -205,7 +205,7 @@ private: void noCopyConstructorError(const Scope *scope, bool isdefault, const Token *alloc, bool inconclusive); void noOperatorEqError(const Scope *scope, bool isdefault, const Token *alloc, bool inconclusive); void noDestructorError(const Scope *scope, bool isdefault, const Token *alloc); - void uninitVarError(const Token *tok, bool isprivate, const std::string &classname, const std::string &varname, bool derived, bool inconclusive); + void uninitVarError(const Token *tok, bool isprivate, const std::string &classname, const std::string &varname, bool derived, bool inconclusive, bool inConstructor = true); void missingMemberCopyError(const Token *tok, const std::string& classname, const std::string& varname); void operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive); void unusedPrivateFunctionError(const Token *tok, const std::string &classname, const std::string &funcname); diff --git a/test/testconstructors.cpp b/test/testconstructors.cpp index 271b6a518..a6e1f4ef4 100644 --- a/test/testconstructors.cpp +++ b/test/testconstructors.cpp @@ -103,6 +103,7 @@ private: TEST_CASE(noConstructor12); // #8951 - member initialization TEST_CASE(noConstructor13); // #9998 TEST_CASE(noConstructor14); // #10770 + TEST_CASE(noConstructor15); // #5499 TEST_CASE(forwardDeclaration); // ticket #4290/#3190 @@ -711,6 +712,31 @@ private: ASSERT_EQUALS("", errout.str()); } + void noConstructor15() { // #5499 + check("class C {\n" + "private:\n" + " int i1 = 0;\n" + " int i2;\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:4]: (warning) Member variable 'C::i2' is not initialized.\n", errout.str()); + + check("class C {\n" + "private:\n" + " int i1;\n" + " int i2;\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:1]: (style) The class 'C' does not declare a constructor although it has private member variables which likely require initialization.\n", errout.str()); + + check("class C {\n" + "public:\n" + " C(int i) : i1(i) {}\n" + "private:\n" + " int i1;\n" + " int i2;\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Member variable 'C::i2' is not initialized in the constructor.\n", errout.str()); + } + // ticket #4290 "False Positive: style (noConstructor): The class 'foo' does not have a constructor." // ticket #3190 "SymbolDatabase: Parse of sub class constructor fails" void forwardDeclaration() {