From 05a6d09c5fd0c58298c75e9959d1454bead710cf Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Sun, 27 Feb 2022 19:15:19 +0100 Subject: [PATCH] Fix #10360 FP uninitMemberVar from copy constructor [inconclusive] (#3748) --- lib/checkclass.cpp | 24 +++++++++++++++----- lib/symboldatabase.cpp | 5 +++++ test/testclass.cpp | 21 ++++++++++++++++++ test/testconstructors.cpp | 46 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 6 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 897155c8b..d760c0d27 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -82,11 +82,23 @@ static const char * getFunctionTypeName(Function::Type type) return ""; } -static bool isVariableCopyNeeded(const Variable &var) +static bool isVariableCopyNeeded(const Variable &var, Function::Type type) { - return var.isPointer() || - (var.type() && var.type()->needInitialization == Type::NeedInitialization::True) || - (var.valueType() && var.valueType()->type >= ValueType::Type::CHAR); + bool isOpEqual = false; + switch (type) { + case Function::eOperatorEqual: + isOpEqual = true; + case Function::eCopyConstructor: + case Function::eMoveConstructor: + break; + default: + return true; + } + + return (!var.hasDefault() || isOpEqual) && // default init does not matter for operator= + (var.isPointer() || + (var.type() && var.type()->needInitialization == Type::NeedInitialization::True) || + (var.valueType() && var.valueType()->type >= ValueType::Type::CHAR)); } static bool isVcl(const Settings *settings) @@ -202,7 +214,7 @@ void CheckClass::constructors() const Variable& var = *usage.var; // check for C++11 initializer - if (var.hasDefault()) { + if (var.hasDefault() && func.type != Function::eOperatorEqual && func.type != Function::eCopyConstructor) { // variable still needs to be copied usage.init = true; continue; } @@ -244,7 +256,7 @@ void CheckClass::constructors() // Don't warn about unknown types in copy constructors since we // don't know if they can be copied or not.. - if ((func.type == Function::eCopyConstructor || func.type == Function::eMoveConstructor || func.type == Function::eOperatorEqual) && !isVariableCopyNeeded(var)) { + if (!isVariableCopyNeeded(var, func.type)) { if (!printInconclusive) continue; diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 0c6108b8b..c95a28cdd 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -5246,6 +5246,11 @@ const Function* Scope::findFunction(const Token *tok, bool requireConst) const if (fallback2Func) return fallback2Func; + // remove pure virtual function + matches.erase(std::remove_if(matches.begin(), matches.end(), [](const Function* m) { + return m->isPure(); + }), matches.end()); + // Only one candidate left if (matches.size() == 1) return matches[0]; diff --git a/test/testclass.cpp b/test/testclass.cpp index 892e6775f..b5b82efda 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -7233,6 +7233,27 @@ private: " A() { f(); }\n" "};\n"); ASSERT_EQUALS("", errout.str()); + + checkVirtualFunctionCall("class Base {\n" + "public:\n" + " virtual void Copy(const Base& Src) = 0;\n" + "};\n" + "class Derived : public Base {\n" + "public:\n" + " Derived() : i(0) {}\n" + " Derived(const Derived& Src);\n" + " void Copy(const Base& Src) override;\n" + " int i;\n" + "};\n" + "Derived::Derived(const Derived& Src) {\n" + " Copy(Src);\n" + "}\n" + "void Derived::Copy(const Base& Src) {\n" + " auto d = dynamic_cast(Src);\n" + " i = d.i;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:13] -> [test.cpp:9]: (style) Virtual function 'Copy' is called from copy constructor 'Derived(const Derived&Src)' at line 13. Dynamic binding is not used.\n", + errout.str()); } void pureVirtualFunctionCall() { diff --git a/test/testconstructors.cpp b/test/testconstructors.cpp index 135705719..907f9ae09 100644 --- a/test/testconstructors.cpp +++ b/test/testconstructors.cpp @@ -86,6 +86,8 @@ private: TEST_CASE(simple13); // #5498 - no constructor, c++11 assignments TEST_CASE(simple14); // #6253 template base TEST_CASE(simple15); // #8942 multiple arguments, decltype + TEST_CASE(simple16); // copy members with c++11 init + TEST_CASE(simple17); // #10360 TEST_CASE(noConstructor1); TEST_CASE(noConstructor2); @@ -497,6 +499,50 @@ private: ASSERT_EQUALS("", errout.str()); } + void simple16() { + check("struct S {\n" + " int i{};\n" + " S() = default;\n" + " S(const S& s) {}\n" + " S& operator=(const S& s) { return *this; }\n" + "};\n", /*inconclusive*/ true); + ASSERT_EQUALS("[test.cpp:4]: (warning, inconclusive) Member variable 'S::i' is not assigned in the copy constructor. Should it be copied?\n" + "[test.cpp:5]: (warning) Member variable 'S::i' is not assigned a value in 'S::operator='.\n", + errout.str()); + + check("struct S {\n" + " int i;\n" + " S() : i(0) {}\n" + " 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" + "[test.cpp:5]: (warning) Member variable 'S::i' is not assigned a value in 'S::operator='.\n", + errout.str()); + } + + void simple17() { // #10360 + check("class Base {\n" + "public:\n" + " virtual void Copy(const Base& Src) = 0;\n" + "};\n" + "class Derived : public Base {\n" + "public:\n" + " Derived() : i(0) {}\n" + " Derived(const Derived& Src);\n" + " void Copy(const Base& Src) override;\n" + " int i;\n" + "};\n" + "Derived::Derived(const Derived& Src) {\n" + " Copy(Src);\n" + "}\n" + "void Derived::Copy(const Base& Src) {\n" + " auto d = dynamic_cast(Src);\n" + " i = d.i;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void simple15() { // #8942 check("class A\n" "{\n"