From 2eed660b32d857432a8dfb3a99e0fec797e3b84f Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Sun, 13 Mar 2022 17:33:31 +0100 Subject: [PATCH] Fix #8496 Clarify warnings for uninitMemberVar (#3760) --- Makefile | 6 +++--- lib/checkclass.cpp | 31 ++++++++++++++++++++++--------- lib/checkclass.h | 17 +++++++++-------- test/testconstructors.cpp | 33 +++++++++++++++++++++------------ 4 files changed, 55 insertions(+), 32 deletions(-) diff --git a/Makefile b/Makefile index 17306c668..fbb27b23b 100644 --- a/Makefile +++ b/Makefile @@ -629,7 +629,7 @@ test/testcharvar.o: test/testcharvar.cpp lib/check.h lib/checkother.h lib/color. test/testclangimport.o: test/testclangimport.cpp lib/clangimport.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o test/testclangimport.o test/testclangimport.cpp -test/testclass.o: test/testclass.cpp externals/tinyxml2/tinyxml2.h lib/check.h lib/checkclass.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h +test/testclass.o: test/testclass.cpp externals/tinyxml2/tinyxml2.h lib/check.h lib/checkclass.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o test/testclass.o test/testclass.cpp test/testcmdlineparser.o: test/testcmdlineparser.cpp cli/cmdlineparser.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h test/redirect.h test/testsuite.h @@ -638,7 +638,7 @@ test/testcmdlineparser.o: test/testcmdlineparser.cpp cli/cmdlineparser.h lib/col test/testcondition.o: test/testcondition.cpp externals/simplecpp/simplecpp.h externals/tinyxml2/tinyxml2.h lib/check.h lib/checkcondition.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o test/testcondition.o test/testcondition.cpp -test/testconstructors.o: test/testconstructors.cpp lib/check.h lib/checkclass.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h +test/testconstructors.o: test/testconstructors.cpp lib/check.h lib/checkclass.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o test/testconstructors.o test/testconstructors.cpp test/testcppcheck.o: test/testcppcheck.cpp lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h test/testsuite.h @@ -773,7 +773,7 @@ test/testuninitvar.o: test/testuninitvar.cpp lib/check.h lib/checkuninitvar.h li test/testunusedfunctions.o: test/testunusedfunctions.cpp lib/check.h lib/checkunusedfunctions.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o test/testunusedfunctions.o test/testunusedfunctions.cpp -test/testunusedprivfunc.o: test/testunusedprivfunc.cpp externals/simplecpp/simplecpp.h lib/check.h lib/checkclass.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h +test/testunusedprivfunc.o: test/testunusedprivfunc.cpp externals/simplecpp/simplecpp.h lib/check.h lib/checkclass.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o test/testunusedprivfunc.o test/testunusedprivfunc.cpp test/testunusedvar.o: test/testunusedvar.cpp externals/simplecpp/simplecpp.h lib/check.h lib/checkunusedvar.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 0dfc8b9ef..728e2a306 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -180,7 +180,7 @@ void CheckClass::constructors() 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); + uninitVarError(uv->typeStartToken(), uv->scope()->className, uv->name()); } } @@ -303,11 +303,11 @@ void CheckClass::constructors() func.functionScope->bodyStart->link() == func.functionScope->bodyStart->next()) { // don't warn about user defined default constructor when there are other constructors if (printInconclusive) - uninitVarError(func.token, func.access == AccessControl::Private, var.scope()->className, var.name(), derived, true); + uninitVarError(func.token, func.access == AccessControl::Private, func.type, var.scope()->className, var.name(), derived, true); } else if (missingCopy) - missingMemberCopyError(func.token, var.scope()->className, var.name()); + missingMemberCopyError(func.token, func.type, var.scope()->className, var.name()); else - uninitVarError(func.token, func.access == AccessControl::Private, var.scope()->className, var.name(), derived, false); + uninitVarError(func.token, func.access == AccessControl::Private, func.type, var.scope()->className, var.name(), derived, false); } } } @@ -1010,21 +1010,34 @@ 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, bool inConstructor) +void CheckClass::uninitVarError(const Token *tok, bool isprivate, Function::Type functionType, const std::string &classname, const std::string &varname, bool derived, bool inconclusive) { - std::string message("Member variable '$symbol' is not initialized"); - message += inConstructor ? " in the constructor." : "."; + std::string ctor; + if (functionType == Function::eCopyConstructor) + ctor = "copy "; + else if (functionType == Function::eMoveConstructor) + ctor = "move "; + std::string message("Member variable '$symbol' is not initialized in the " + ctor + "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" : ""); reportError(tok, Severity::warning, id, "$symbol:" + classname + "::" + varname + "\n" + message, CWE398, inconclusive ? Certainty::inconclusive : Certainty::normal); } -void CheckClass::missingMemberCopyError(const Token *tok, const std::string& classname, const std::string& varname) +void CheckClass::uninitVarError(const Token *tok, const std::string &classname, const std::string &varname) { + const std::string message("Member variable '$symbol' is not initialized."); // report missing in-class initializer + const std::string id = std::string("uninitMemberVarPrivate"); + reportError(tok, Severity::warning, id, "$symbol:" + classname + "::" + varname + "\n" + message, CWE398, Certainty::normal); +} + +void CheckClass::missingMemberCopyError(const Token *tok, Function::Type functionType, const std::string& classname, const std::string& varname) +{ + const std::string ctor(functionType == Function::Type::eCopyConstructor ? "copy" : "move"); + const std::string action(functionType == Function::Type::eCopyConstructor ? "copied?" : "moved?"); const std::string message = "$symbol:" + classname + "::" + varname + "\n" + - "Member variable '$symbol' is not assigned in the copy constructor. Should it be copied?"; + "Member variable '$symbol' is not assigned in the " + ctor + " constructor. Should it be " + action; const char id[] = "missingMemberCopy"; reportError(tok, Severity::warning, id, message, CWE398, Certainty::inconclusive); } diff --git a/lib/checkclass.h b/lib/checkclass.h index bc305d2a8..e53c00721 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -24,6 +24,7 @@ #include "check.h" #include "config.h" #include "tokenize.h" +#include "symboldatabase.h" #include #include @@ -37,7 +38,6 @@ class Settings; class Token; class Function; class Scope; -class SymbolDatabase; class Type; class Variable; @@ -205,8 +205,9 @@ 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, bool inConstructor = true); - void missingMemberCopyError(const Token *tok, const std::string& classname, const std::string& varname); + void uninitVarError(const Token *tok, bool isprivate, Function::Type functionType, const std::string &classname, const std::string &varname, bool derived, bool inconclusive); + void uninitVarError(const Token *tok, const std::string &classname, const std::string &varname); + void missingMemberCopyError(const Token *tok, Function::Type functionType, 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); void memsetError(const Token *tok, const std::string &memfunc, const std::string &classname, const std::string &type); @@ -243,11 +244,11 @@ private: c.noCopyConstructorError(nullptr, false, nullptr, false); c.noOperatorEqError(nullptr, false, nullptr, false); c.noDestructorError(nullptr, false, nullptr); - c.uninitVarError(nullptr, false, "classname", "varname", false, false); - c.uninitVarError(nullptr, true, "classname", "varnamepriv", false, false); - c.uninitVarError(nullptr, false, "classname", "varname", true, false); - c.uninitVarError(nullptr, true, "classname", "varnamepriv", true, false); - c.missingMemberCopyError(nullptr, "classname", "varnamepriv"); + c.uninitVarError(nullptr, false, Function::eConstructor, "classname", "varname", false, false); + c.uninitVarError(nullptr, true, Function::eConstructor, "classname", "varnamepriv", false, false); + c.uninitVarError(nullptr, false, Function::eConstructor, "classname", "varname", true, false); + c.uninitVarError(nullptr, true, Function::eConstructor, "classname", "varnamepriv", true, false); + c.missingMemberCopyError(nullptr, Function::eConstructor, "classname", "varnamepriv"); c.operatorEqVarError(nullptr, "classname", emptyString, false); c.unusedPrivateFunctionError(nullptr, "classname", "funcname"); c.memsetError(nullptr, "memfunc", "classname", "class"); diff --git a/test/testconstructors.cpp b/test/testconstructors.cpp index a6e1f4ef4..d6b2688d8 100644 --- a/test/testconstructors.cpp +++ b/test/testconstructors.cpp @@ -287,8 +287,8 @@ private: " int i;\n" "};"); ASSERT_EQUALS("[test.cpp:4]: (warning) Member variable 'Fred::i' is not initialized in the constructor.\n" - "[test.cpp:5]: (warning) Member variable 'Fred::i' is not initialized in the constructor.\n" - "[test.cpp:6]: (warning) Member variable 'Fred::i' is not initialized in the constructor.\n", errout.str()); + "[test.cpp:5]: (warning) Member variable 'Fred::i' is not initialized in the copy constructor.\n" + "[test.cpp:6]: (warning) Member variable 'Fred::i' is not initialized in the move constructor.\n", errout.str()); check("struct Fred\n" "{\n" @@ -298,8 +298,8 @@ private: " int i;\n" "};"); ASSERT_EQUALS("[test.cpp:3]: (warning) Member variable 'Fred::i' is not initialized in the constructor.\n" - "[test.cpp:4]: (warning) Member variable 'Fred::i' is not initialized in the constructor.\n" - "[test.cpp:5]: (warning) Member variable 'Fred::i' is not initialized in the constructor.\n", errout.str()); + "[test.cpp:4]: (warning) Member variable 'Fred::i' is not initialized in the copy constructor.\n" + "[test.cpp:5]: (warning) Member variable 'Fred::i' is not initialized in the move constructor.\n", errout.str()); } @@ -519,7 +519,7 @@ private: " S(const S& s) {}\n" " S& operator=(const S& s) { return *this; }\n" "};\n"); - ASSERT_EQUALS("[test.cpp:4]: (warning) Member variable 'S::i' is not initialized in the constructor.\n" + ASSERT_EQUALS("[test.cpp:4]: (warning) Member variable 'S::i' is not initialized in the copy constructor.\n" "[test.cpp:5]: (warning) Member variable 'S::i' is not assigned a value in 'S::operator='.\n", errout.str()); } @@ -1460,6 +1460,15 @@ private: "Fred::Fred() { };\n" "Fred::Fred(const Fred &) { };\n", true); ASSERT_EQUALS("[test.cpp:10]: (warning, inconclusive) Member variable 'Fred::var' is not assigned in the copy constructor. Should it be copied?\n", errout.str()); + + check("class Baz {};\n" // #8496 + "class Bar {\n" + "public:\n" + " explicit Bar(Baz* pBaz = NULL) : i(0) {}\n" + " Bar(const Bar& bar) {}\n" + " int i;\n" + "};\n", true); + ASSERT_EQUALS("[test.cpp:5]: (warning) Member variable 'Bar::i' is not initialized in the copy constructor.\n", errout.str()); } void initvar_nested_constructor() { // ticket #1375 @@ -1521,7 +1530,7 @@ private: ASSERT_EQUALS("[test.cpp:20]: (warning) Member variable 'A::a' is not initialized in the constructor.\n" "[test.cpp:21]: (warning) Member variable 'B::b' is not initialized in the constructor.\n" "[test.cpp:22]: (warning) Member variable 'C::c' is not initialized in the constructor.\n" - "[test.cpp:23]: (warning) Member variable 'D::d' is not initialized in the constructor.\n", errout.str()); + "[test.cpp:23]: (warning) Member variable 'D::d' is not initialized in the copy constructor.\n", errout.str()); check("class A {\n" "public:\n" @@ -1634,7 +1643,7 @@ private: " const B& operator=(const B&){return *this;}\n" "};", true); ASSERT_EQUALS("[test.cpp:11]: (warning, inconclusive) Member variable 'B::a' is not assigned in the copy constructor. Should it be copied?\n" - "[test.cpp:12]: (warning, inconclusive) Member variable 'B::a' is not assigned in the copy constructor. Should it be copied?\n" + "[test.cpp:12]: (warning, inconclusive) Member variable 'B::a' is not assigned in the move constructor. Should it be moved?\n" "[test.cpp:13]: (warning, inconclusive) Member variable 'B::a' is not assigned a value in 'B::operator='.\n", errout.str()); @@ -1653,7 +1662,7 @@ private: " A(A &&){}\n" " const A& operator=(const A&){return *this;}\n" "};"); - ASSERT_EQUALS("[test.cpp:13]: (warning) Member variable 'A::m_SemVar' is not initialized in the constructor.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:13]: (warning) Member variable 'A::m_SemVar' is not initialized in the move constructor.\n", errout.str()); check("class B\n" "{\n" @@ -1717,7 +1726,7 @@ private: " const A& operator=(const A&){return *this;}\n" "};", true); ASSERT_EQUALS("[test.cpp:12]: (warning) Member variable 'A::m_SemVar' is not initialized in the constructor.\n" - "[test.cpp:13]: (warning) Member variable 'A::m_SemVar' is not initialized in the constructor.\n" + "[test.cpp:13]: (warning) Member variable 'A::m_SemVar' is not initialized in the copy constructor.\n" "[test.cpp:14]: (warning) Member variable 'A::m_SemVar' is not assigned a value in 'A::operator='.\n", errout.str()); } @@ -3775,7 +3784,7 @@ private: " return *this;\n" " }\n" "};"); - ASSERT_EQUALS("[test.cpp:4]: (warning) Member variable 'A::a' is not initialized in the constructor.\n" + ASSERT_EQUALS("[test.cpp:4]: (warning) Member variable 'A::a' is not initialized in the copy constructor.\n" "[test.cpp:5]: (warning) Member variable 'A::a' is not assigned a value in 'A::operator='.\n", errout.str()); } @@ -3848,7 +3857,7 @@ private: " B(B& b) { }\n" "};"); ASSERT_EQUALS("[test.cpp:4]: (warning) Member variable 'B::a' is not initialized in the constructor.\n" - "[test.cpp:5]: (warning) Member variable 'B::a' is not initialized in the constructor.\n", errout.str()); + "[test.cpp:5]: (warning) Member variable 'B::a' is not initialized in the copy constructor.\n", errout.str()); check("struct A;\n" "struct B {\n" @@ -3863,7 +3872,7 @@ private: " B(B& b) { }\n" "};"); ASSERT_EQUALS("[test.cpp:3]: (warning) Member variable 'B::a' is not initialized in the constructor.\n" - "[test.cpp:4]: (warning) Member variable 'B::a' is not initialized in the constructor.\n", errout.str()); + "[test.cpp:4]: (warning) Member variable 'B::a' is not initialized in the copy constructor.\n", errout.str()); check("struct B {\n" " const int a;\n"