walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Virtual destructors: Enabled the check again. I think it is conclusive now. Ticket: #2728

This commit is contained in:
Daniel Marjamäki 2011-04-20 18:03:16 +02:00
parent a79e979ae4
commit 061eab4d22
2 changed files with 110 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
lib/checkclass.cpp
View File

@ -1118,8 +1118,6 @@ void CheckClass::virtualDestructor()
// * base class doesn't have virtual destructor
// * derived class has non-empty destructor
// * base class is deleted
if (!_settings->experimental)
return;
createSymbolDatabase();
@ -1156,6 +1154,57 @@ void CheckClass::virtualDestructor()
// Name of base class..
const std::string baseName = derivedFrom->className;
// Check for this pattern:
// 1. Base class pointer is given the address of derived class instance
// 2. Base class pointer is deleted
//
// If this pattern is not seen then bailout the checking of these base/derived classes
{
// pointer variables of type 'Base *'
std::set<unsigned int> basepointer;
// pointer variables of type 'Base *' that should not be deleted
std::set<unsigned int> dontDelete;
// No deletion of derived class instance through base class pointer found => the code is ok
bool ok = true;
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
{
// Declaring base class pointer
if (Token::simpleMatch(tok, baseName.c_str()))
{
if (Token::Match(tok->previous(), ("[;{}] " + baseName + " * %var% ;").c_str()))
basepointer.insert(tok->tokAt(2)->varId());
}
// Assign base class pointer with pointer to derived class instance
if (Token::Match(tok, "[;{}] %var% =") &&
tok->next()->varId() > 0 &&
basepointer.find(tok->next()->varId()) != basepointer.end())
{
// new derived class..
if (Token::simpleMatch(tok->tokAt(3), ("new " + derivedClass->str()).c_str()))
{
dontDelete.insert(tok->next()->varId());
}
}
// Delete base class pointer that might point at derived class
if (Token::Match(tok, "delete %var% ;") &&
tok->next()->varId() &&
dontDelete.find(tok->next()->varId()) != dontDelete.end())
{
ok = false;
break;
}
}
// No base class pointer that points at a derived class is deleted
if (ok)
continue;
}
// Find the destructor declaration for the base class.
const Function *base_destructor = derivedFrom->getDestructor();
const Token *base = 0;

74
test/testclass.cpp
View File

@ -40,6 +40,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(virtualDestructor6); // only report error if base class pointer that points at derived class is deleted
TEST_CASE(virtualDestructorProtected);
TEST_CASE(virtualDestructorInherited);
TEST_CASE(virtualDestructorTemplate);
@ -1382,7 +1383,6 @@ private:
errout.str("");
Settings settings;
settings.experimental = true;
// Tokenize..
Tokenizer tokenizer(&settings, this);
@ -1399,10 +1399,14 @@ private:
{
// Base class not found
checkVirtualDestructor("class Derived : public Base { };");
checkVirtualDestructor("class Derived : public Base { };\n"
"Base *base = new Derived;\n"
"delete base;");
ASSERT_EQUALS("", errout.str());
checkVirtualDestructor("class Derived : Base { };");
checkVirtualDestructor("class Derived : Base { };\n"
"Base *base = new Derived;\n"
"delete base;");
ASSERT_EQUALS("", errout.str());
}
@ -1411,19 +1415,27 @@ private:
// Base class doesn't have a destructor
checkVirtualDestructor("class Base { };\n"
"class Derived : public Base { public: ~Derived() { (void)11; } };");
"class Derived : public Base { public: ~Derived() { (void)11; } };"
"Base *base = new Derived;\n"
"delete base;");
ASSERT_EQUALS("[test.cpp:1]: (error) Class Base which is inherited by class Derived does not have a virtual destructor\n", errout.str());
checkVirtualDestructor("class Base { };\n"
"class Derived : protected Base { public: ~Derived() { (void)11; } };");
"class Derived : protected Base { public: ~Derived() { (void)11; } };"
"Base *base = new Derived;\n"
"delete base;");
ASSERT_EQUALS("[test.cpp:1]: (error) Class Base which is inherited by class Derived does not have a virtual destructor\n", errout.str());
checkVirtualDestructor("class Base { };\n"
"class Derived : private Base { public: ~Derived() { (void)11; } };");
"class Derived : private Base { public: ~Derived() { (void)11; } };"
"Base *base = new Derived;\n"
"delete base;");
ASSERT_EQUALS("", errout.str());
checkVirtualDestructor("class Base { };\n"
"class Derived : Base { public: ~Derived() { (void)11; } };");
"class Derived : Base { public: ~Derived() { (void)11; } };"
"Base *base = new Derived;\n"
"delete base;");
ASSERT_EQUALS("", errout.str());
}
@ -1432,15 +1444,21 @@ private:
// Base class has a destructor, but it's not virtual
checkVirtualDestructor("class Base { public: ~Base(); };\n"
"class Derived : public Base { public: ~Derived() { (void)11; } };");
"class Derived : public Base { public: ~Derived() { (void)11; } };"
"Base *base = new Derived;\n"
"delete base;");
ASSERT_EQUALS("[test.cpp:1]: (error) Class Base which is inherited by class Derived does not have a virtual destructor\n", errout.str());
checkVirtualDestructor("class Base { public: ~Base(); };\n"
"class Derived : protected Base { public: ~Derived() { (void)11; } };");
"class Derived : protected Base { public: ~Derived() { (void)11; } };"
"Base *base = new Derived;\n"
"delete base;");
ASSERT_EQUALS("[test.cpp:1]: (error) Class Base which is inherited by class Derived does not have a virtual destructor\n", errout.str());
checkVirtualDestructor("class Base { public: ~Base(); };\n"
"class Derived : private Fred, public Base { public: ~Derived() { (void)11; } };");
"class Derived : private Fred, public Base { public: ~Derived() { (void)11; } };"
"Base *base = new Derived;\n"
"delete base;");
ASSERT_EQUALS("[test.cpp:1]: (error) Class Base which is inherited by class Derived does not have a virtual destructor\n", errout.str());
}
@ -1449,11 +1467,15 @@ private:
// Derived class doesn't have a destructor => no error
checkVirtualDestructor("class Base { public: ~Base(); };\n"
"class Derived : public Base { };");
"class Derived : public Base { };"
"Base *base = new Derived;\n"
"delete base;");
ASSERT_EQUALS("", errout.str());
checkVirtualDestructor("class Base { public: ~Base(); };\n"
"class Derived : private Fred, public Base { };");
"class Derived : private Fred, public Base { };"
"Base *base = new Derived;\n"
"delete base;");
ASSERT_EQUALS("", errout.str());
}
@ -1462,11 +1484,31 @@ private:
// Derived class has empty destructor => no error
checkVirtualDestructor("class Base { public: ~Base(); };\n"
"class Derived : public Base { public: ~Derived() {} };");
"class Derived : public Base { public: ~Derived() {} };"
"Base *base = new Derived;\n"
"delete base;");
ASSERT_EQUALS("", errout.str());
checkVirtualDestructor("class Base { public: ~Base(); };\n"
"class Derived : public Base { public: ~Derived(); }; Derived::~Derived() {}");
"class Derived : public Base { public: ~Derived(); }; Derived::~Derived() {}"
"Base *base = new Derived;\n"
"delete base;");
ASSERT_EQUALS("", errout.str());
}
void virtualDestructor6()
{
// Only report error if base class pointer is deleted that
// points at derived class
checkVirtualDestructor("class Base { public: ~Base(); };\n"
"class Derived : public Base { public: ~Derived() { (void)11; } };"
"Base *base = new Derived;\n"
"delete base;");
ASSERT_EQUALS("[test.cpp:1]: (error) Class Base which is inherited by class Derived does not have a virtual destructor\n", errout.str());
checkVirtualDestructor("class Base { public: ~Base(); };\n"
"class Derived : public Base { public: ~Derived() { (void)11; } };");
ASSERT_EQUALS("", errout.str());
}
@ -1599,7 +1641,9 @@ private:
"{\n"
" public:\n"
" ~B(){int a;}\n"
"};\n");
"};\n"
"\n"
"AA<double> *p = new B; delete p;");
ASSERT_EQUALS("[test.cpp:9]: (error) Class AA<double> which is inherited by class B does not have a virtual destructor\n", errout.str());
}