diff --git a/src/checkclass.cpp b/src/checkclass.cpp index c8757ec33..dc3c8cb8f 100644 --- a/src/checkclass.cpp +++ b/src/checkclass.cpp @@ -720,17 +720,28 @@ void CheckClass::virtualDestructor() if (! base) { // Is the class declaration available? - base = Token::findmatch(_tokenizer->tokens(), (std::string("class ") + baseName[0] + " :|{").c_str()); + base = Token::findmatch(_tokenizer->tokens(), (std::string("class ") + baseName[0] + " {").c_str()); if (base) { virtualDestructorError(base, baseName[0], derivedClass->str()); } + + continue; } // There is a destructor. Check that it's virtual.. else if (base->str() == "virtual") continue; + // TODO: This is just a temporary fix, better solution is needed. + // Skip situations where base class has base classes of its own, because + // some of the base classes might have virtual destructor. + // Proper solution is to check all of the base classes. If base class is not + // found or if one of the base classes has virtual destructor, error should not + // be printed. See TODO test case "virtualDestructorInherited" + if (!Token::findmatch(_tokenizer->tokens(), (std::string("class ") + baseName[0] + " {").c_str())) + continue; + // Make sure that the destructor is public (protected or private // would not compile if inheritance is used in a way that would // cause the bug we are trying to find here.) diff --git a/test/testclass.cpp b/test/testclass.cpp index fe6b0761d..9ca98aaff 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -41,6 +41,7 @@ private: TEST_CASE(virtualDestructor4); // Derived class doesn't have a destructor => no error TEST_CASE(virtualDestructor5); // Derived class has empty destructor => no error TEST_CASE(virtualDestructorProtected); + TEST_CASE(virtualDestructorInherited); TEST_CASE(uninitVar1); TEST_CASE(uninitVarEnum); @@ -157,6 +158,100 @@ private: ASSERT_EQUALS("", errout.str()); } + void virtualDestructorInherited() + { + // class A inherits virtual destructor from class Base -> no error + checkVirtualDestructor("class Base\n" + "{\n" + "public:\n" + "virtual ~Base() {}\n" + "};\n" + "class A : private Base\n" + "{\n" + "public:\n" + " ~A() { }\n" + "};\n" + "\n" + "class B : public A\n" + "{\n" + "public:\n" + " ~B() { int a; }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + // class A inherits virtual destructor from struct Base -> no error + // also notice that public is not given, but destructor is public, because + // we are using struct instead of class + checkVirtualDestructor("struct Base\n" + "{\n" + "virtual ~Base() {}\n" + "};\n" + "class A : public Base\n" + "{\n" + "};\n" + "\n" + "class B : public A\n" + "{\n" + "public:\n" + " ~B() { int a; }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + // Unknown Base class -> it could have virtual destructor, so ignore + checkVirtualDestructor("class A : private Base\n" + "{\n" + "public:\n" + " ~A() { }\n" + "};\n" + "\n" + "class B : public A\n" + "{\n" + "public:\n" + " ~B() { int a; }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + // Virtual destructor is inherited -> no error + checkVirtualDestructor("class Base2\n" + "{\n" + "virtual ~Base2() {}\n" + "};\n" + "class Base : public Base2\n" + "{\n" + "};\n" + "class A : private Base\n" + "{\n" + "public:\n" + " ~A() { }\n" + "};\n" + "\n" + "class B : public A\n" + "{\n" + "public:\n" + " ~B() { int a; }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + // class A doesn't inherit virtual destructor from class Base -> error + checkVirtualDestructor("class Base\n" + "{\n" + "public:\n" + "~Base() {}\n" + "};\n" + "class A : private Base\n" + "{\n" + "public:\n" + " ~A() { }\n" + "};\n" + "\n" + "class B : public A\n" + "{\n" + "public:\n" + " ~B() { int a; }\n" + "};\n"); + TODO_ASSERT_EQUALS("[test.cpp:7]: (error) Class A which is inherited by class B does not have a virtual destructor\n", errout.str()); + } + void checkUninitVar(const char code[]) { // Tokenize..