CheckClass: Better handling of non-copyable classes in the noCopyConstructor check

danmar · danmar · commit 42100fdf1174 · 2018-05-01T08:33:23.000+02:00
diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp
@@ -275,6 +275,27 @@ void CheckClass::checkExplicitConstructors()
     }
 }
 
+static bool isNonCopyable(const Scope *scope, bool *unknown)
+{
+    bool u = false;
+    // check if there is base class that is not copyable
+    for (const Type::BaseInfo &baseInfo : scope->definedType->derivedFrom) {
+        if (!baseInfo.type) {
+            u = true;
+            continue;
+        }
+
+        for (const Function &func : baseInfo.type->classScope->functionList) {
+            if (func.type != Function::eCopyConstructor)
+                continue;
+            if (func.access == Private || func.isDelete())
+                return true;
+        }
+    }
+    *unknown = u;
+    return false;
+}
+
 void CheckClass::copyconstructors()
 {
     if (!_settings->isEnabled(Settings::STYLE))
@@ -317,9 +338,11 @@ void CheckClass::copyconstructors()
                 else if (func.type == Function::eDestructor)
                     hasDestructor = true;
             }
-            bool derived = !scope->definedType->derivedFrom.empty();
-            if (!hasCopyCtor && !derived)
-                noCopyConstructorError(scope, derived);
+            if (!hasCopyCtor) {
+                bool unknown = false;
+                if (!isNonCopyable(scope, &unknown))
+                    noCopyConstructorError(scope, unknown);
+            }
             if (!hasOperatorEq)
                 noOperatorEqError(scope);
             if (!hasDestructor)
diff --git a/test/testclass.cpp b/test/testclass.cpp
@@ -735,7 +735,7 @@ class TestClass : public TestFixture {
                              "   ~F();\n"
                              "   F& operator=(const F&f);\n"
                              "};");
-        ASSERT_EQUALS("", errout.str());
+        ASSERT_EQUALS("[test.cpp:1]: (style, inconclusive) class 'F' does not have a copy constructor which is recommended since the class contains a pointer to allocated memory.\n", errout.str());
 
         checkCopyConstructor("class E { E(E&); };\n" // non-copyable
                              "class F : E\n"
@@ -758,7 +758,7 @@ class TestClass : public TestFixture {
                              "   ~F();\n"
                              "   F& operator=(const F&f);\n"
                              "};");
-        TODO_ASSERT_EQUALS("[test.cpp:2]: (style) 'class F' does not have a copy constructor which is recommended since the class contains a pointer to allocated memory.\n", "", errout.str());
+        ASSERT_EQUALS("[test.cpp:2]: (style) class 'F' does not have a copy constructor which is recommended since the class contains a pointer to allocated memory.\n", errout.str());
 
         checkCopyConstructor("class F {\n"
                              "   char *p;\n"
