Fix ticket #531 (false positive about missing virtual destructor)
http://sourceforge.net/apps/trac/cppcheck/ticket/531
This commit is contained in:
parent
a34d6fd2de
commit
975e7778ab
|
@ -720,17 +720,28 @@ void CheckClass::virtualDestructor()
|
||||||
if (! base)
|
if (! base)
|
||||||
{
|
{
|
||||||
// Is the class declaration available?
|
// 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)
|
if (base)
|
||||||
{
|
{
|
||||||
virtualDestructorError(base, baseName[0], derivedClass->str());
|
virtualDestructorError(base, baseName[0], derivedClass->str());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
// There is a destructor. Check that it's virtual..
|
// There is a destructor. Check that it's virtual..
|
||||||
else if (base->str() == "virtual")
|
else if (base->str() == "virtual")
|
||||||
continue;
|
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
|
// Make sure that the destructor is public (protected or private
|
||||||
// would not compile if inheritance is used in a way that would
|
// would not compile if inheritance is used in a way that would
|
||||||
// cause the bug we are trying to find here.)
|
// cause the bug we are trying to find here.)
|
||||||
|
|
|
@ -41,6 +41,7 @@ private:
|
||||||
TEST_CASE(virtualDestructor4); // Derived class doesn't have a destructor => no error
|
TEST_CASE(virtualDestructor4); // Derived class doesn't have a destructor => no error
|
||||||
TEST_CASE(virtualDestructor5); // Derived class has empty destructor => no error
|
TEST_CASE(virtualDestructor5); // Derived class has empty destructor => no error
|
||||||
TEST_CASE(virtualDestructorProtected);
|
TEST_CASE(virtualDestructorProtected);
|
||||||
|
TEST_CASE(virtualDestructorInherited);
|
||||||
|
|
||||||
TEST_CASE(uninitVar1);
|
TEST_CASE(uninitVar1);
|
||||||
TEST_CASE(uninitVarEnum);
|
TEST_CASE(uninitVarEnum);
|
||||||
|
@ -157,6 +158,100 @@ private:
|
||||||
ASSERT_EQUALS("", errout.str());
|
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[])
|
void checkUninitVar(const char code[])
|
||||||
{
|
{
|
||||||
// Tokenize..
|
// Tokenize..
|
||||||
|
|
Loading…
Reference in New Issue