diff --git a/src/checkclass.cpp b/src/checkclass.cpp index 7dbc8b970..2f810587f 100644 --- a/src/checkclass.cpp +++ b/src/checkclass.cpp @@ -683,8 +683,9 @@ void CheckClass::virtualDestructor() // Find the destructor declaration for the base class. const Token *base = Token::findmatch(_tokenizer->tokens(), (std::string("%any% ~ ") + baseName[0] + " (").c_str()); while (Token::Match(base, "::")) - base = Token::findmatch(base->next(), (std::string("%any% ~ ") + baseName[0] + + " (").c_str()); + base = Token::findmatch(base->next(), (std::string("%any% ~ ") + baseName[0] + " (").c_str()); + const Token *reverseTok = base; while (Token::Match(base, "%var%") && !Token::Match(base, "virtual")) base = base->previous(); @@ -700,9 +701,41 @@ void CheckClass::virtualDestructor() } // There is a destructor. Check that it's virtual.. - else if (base->str() != "virtual") + else if (base->str() == "virtual") + 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.) + int indent = 0; + while (reverseTok) { - virtualDestructorError(base, baseName[0], derivedClass->str()); + if (reverseTok->str() == "public:") + { + virtualDestructorError(base, baseName[0], derivedClass->str()); + break; + } + else if (reverseTok->str() == "protected:" || + reverseTok->str() == "private:") + { + // No bug, protected/private destructor is allowed + break; + } + else if (reverseTok->str() == "{") + { + indent++; + if (indent >= 1) + { + // We have found the start of the class without any sign + // of "public :" so we can assume that the destructor is not + // public and there is no bug in the code we are checking. + break; + } + } + else if (reverseTok->str() == "}") + indent--; + + reverseTok = reverseTok->previous(); } } } diff --git a/test/testclass.cpp b/test/testclass.cpp index 0a666a261..4b883a028 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -41,6 +41,7 @@ private: TEST_CASE(virtualDestructor3); // Base class has a destructor, but it's not virtual 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(uninitVar1); TEST_CASE(uninitVarEnum); @@ -138,6 +139,23 @@ private: ASSERT_EQUALS(std::string(""), errout.str()); } + void virtualDestructorProtected() + { + // Base class has protected destructor, it makes Base *p = new Derived(); fail + // during compilation time, so error is not possible. => no error + checkVirtualDestructor("class A\n" + "{\n" + "protected:\n" + " ~A() { }\n" + "};\n" + "\n" + "class B : public A\n" + "{\n" + "public:\n" + " ~B() { int a; }\n" + "};\n"); + ASSERT_EQUALS(std::string(""), errout.str()); + } void checkUninitVar(const char code[]) {