From 1e9687aa8be40f949acf07f7d913d55c724eac79 Mon Sep 17 00:00:00 2001 From: ericmalenfant Date: Fri, 9 Apr 2021 01:41:59 -0400 Subject: [PATCH] Fix noCopyConstructor with multiple inheritance (#3203) --- lib/checkclass.cpp | 16 ++++++++-------- test/testclass.cpp | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 102edaf23..d910c66c7 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -298,27 +298,27 @@ void CheckClass::checkExplicitConstructors() } } -static bool isNonCopyable(const Scope *scope, bool *unknown) +static bool hasNonCopyableBase(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 || !baseInfo.type->classScope) { - u = true; + *unknown = true; continue; } - if (isNonCopyable(baseInfo.type->classScope, &u)) + if (hasNonCopyableBase(baseInfo.type->classScope, unknown)) return true; for (const Function &func : baseInfo.type->classScope->functionList) { if (func.type != Function::eCopyConstructor) continue; - if (func.access == AccessControl::Private || func.isDelete()) + if (func.access == AccessControl::Private || func.isDelete()) { + *unknown = false; return true; + } } } - *unknown = u; return false; } @@ -366,12 +366,12 @@ void CheckClass::copyconstructors() } if (!funcCopyCtor || funcCopyCtor->isDefault()) { bool unknown = false; - if (!isNonCopyable(scope, &unknown) && !unknown) + if (!hasNonCopyableBase(scope, &unknown) && !unknown) noCopyConstructorError(scope, funcCopyCtor, allocatedVars.begin()->second, unknown); } if (!funcOperatorEq || funcOperatorEq->isDefault()) { bool unknown = false; - if (!isNonCopyable(scope, &unknown) && !unknown) + if (!hasNonCopyableBase(scope, &unknown) && !unknown) noOperatorEqError(scope, funcOperatorEq, allocatedVars.begin()->second, unknown); } if (!funcDestructor || funcDestructor->isDefault()) { diff --git a/test/testclass.cpp b/test/testclass.cpp index a4b5ef34f..f570db09e 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -71,6 +71,7 @@ private: TEST_CASE(copyConstructor2); // ticket #4458 TEST_CASE(copyConstructor3); // defaulted/deleted TEST_CASE(copyConstructor4); // base class with private constructor + TEST_CASE(copyConstructor5); // multiple inheritance TEST_CASE(noOperatorEq); // class with memory management should have operator eq TEST_CASE(noDestructor); // class with memory management should have destructor @@ -908,6 +909,30 @@ private: ASSERT_EQUALS("", errout.str()); } + void copyConstructor5() { + checkCopyConstructor("class Copyable {};\n" + "\n" + "class Foo : public Copyable, public UnknownType {\n" + "public:\n" + " Foo() : m_ptr(new int) {}\n" + " ~Foo() { delete m_ptr; }\n" + "private:\n" + " int* m_ptr;\n" + "};"); + ASSERT_EQUALS("", errout.str()); + + checkCopyConstructor("class Copyable {};\n" + "\n" + "class Foo : public UnknownType, public Copyable {\n" + "public:\n" + " Foo() : m_ptr(new int) {}\n" + " ~Foo() { delete m_ptr; }\n" + "private:\n" + " int* m_ptr;\n" + "};"); + ASSERT_EQUALS("", errout.str()); + } + void noOperatorEq() { checkCopyConstructor("struct F {\n" " char* c;\n"