Fix #10360 FP uninitMemberVar from copy constructor [inconclusive] (#3748)

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() ||
+    bool isOpEqual = false;
-           (var.type() && var.type()->needInitialization == Type::NeedInitialization::True) ||
+    switch (type) {
-           (var.valueType() && var.valueType()->type >= ValueType::Type::CHAR);
+    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;
 

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];

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<const Derived&>(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() {

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<const Derived&>(Src);\n"
+              "    i = d.i;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void simple15() { // #8942
         check("class A\n"
               "{\n"