Virtual destructors : Handle inheritance where
"private|protected|public" is not defined
This commit is contained in:
Daniel Marjamäki
parent
5a80bcc352
commit
057d6f1f18
239
CheckClass.cpp
239
CheckClass.cpp
|
|
@ -308,69 +308,69 @@ void CheckClass::ClassChecking_VarList_Initialize(const TOKEN *tok1, const TOKEN
|
|||
//---------------------------------------------------------------------------
|
||||
|
||||
void CheckClass::CheckConstructors()
|
||||
{
|
||||
const char pattern_class[] = "class %var% {";
|
||||
{
|
||||
const char pattern_class[] = "class %var% {";
|
||||
|
||||
// Locate class
|
||||
const TOKEN *tok1 = TOKEN::findmatch( _tokenizer->tokens(), pattern_class );
|
||||
// Locate class
|
||||
const TOKEN *tok1 = TOKEN::findmatch( _tokenizer->tokens(), pattern_class );
|
||||
while (tok1)
|
||||
{
|
||||
const char *className[2];
|
||||
className[0] = tok1->strAt( 1 );
|
||||
className[1] = 0;
|
||||
|
||||
// TODO: handling of private constructors should be improved.
|
||||
bool hasPrivateConstructor = false;
|
||||
{
|
||||
int indentlevel = 0;
|
||||
bool isPrivate = true;
|
||||
for ( const TOKEN *tok = tok1; tok; tok = tok->next )
|
||||
{
|
||||
// Indentation
|
||||
if ( tok->str() == "{" )
|
||||
++indentlevel;
|
||||
|
||||
else if ( tok->str() == "}" )
|
||||
{
|
||||
--indentlevel;
|
||||
if (indentlevel <= 0)
|
||||
break;
|
||||
}
|
||||
|
||||
// Parse class contents (indentlevel == 1)..
|
||||
if ( indentlevel == 1 )
|
||||
{
|
||||
// What section are we in.. private/non-private
|
||||
if ( tok->str() == "private:" )
|
||||
isPrivate = true;
|
||||
else if ( tok->str() == "protected:" || tok->str() == "public:" )
|
||||
isPrivate = false;
|
||||
|
||||
// Is there a private constructor?
|
||||
else if ( isPrivate && TOKEN::Match(tok, "%var1% (", className) )
|
||||
{
|
||||
hasPrivateConstructor = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if ( hasPrivateConstructor )
|
||||
{
|
||||
// TODO: Handle private constructors.
|
||||
// Right now to avoid false positives I just bail out
|
||||
tok1 = TOKEN::findmatch( tok1->next, pattern_class );
|
||||
continue;
|
||||
}
|
||||
{
|
||||
const char *className[2];
|
||||
className[0] = tok1->strAt( 1 );
|
||||
className[1] = 0;
|
||||
|
||||
// TODO: handling of private constructors should be improved.
|
||||
bool hasPrivateConstructor = false;
|
||||
{
|
||||
int indentlevel = 0;
|
||||
bool isPrivate = true;
|
||||
for ( const TOKEN *tok = tok1; tok; tok = tok->next )
|
||||
{
|
||||
// Indentation
|
||||
if ( tok->str() == "{" )
|
||||
++indentlevel;
|
||||
|
||||
else if ( tok->str() == "}" )
|
||||
{
|
||||
--indentlevel;
|
||||
if (indentlevel <= 0)
|
||||
break;
|
||||
}
|
||||
|
||||
// Parse class contents (indentlevel == 1)..
|
||||
if ( indentlevel == 1 )
|
||||
{
|
||||
// What section are we in.. private/non-private
|
||||
if ( tok->str() == "private:" )
|
||||
isPrivate = true;
|
||||
else if ( tok->str() == "protected:" || tok->str() == "public:" )
|
||||
isPrivate = false;
|
||||
|
||||
// Is there a private constructor?
|
||||
else if ( isPrivate && TOKEN::Match(tok, "%var1% (", className) )
|
||||
{
|
||||
hasPrivateConstructor = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if ( hasPrivateConstructor )
|
||||
{
|
||||
// TODO: Handle private constructors.
|
||||
// Right now to avoid false positives I just bail out
|
||||
tok1 = TOKEN::findmatch( tok1->next, pattern_class );
|
||||
continue;
|
||||
}
|
||||
|
||||
// Are there a class constructor?
|
||||
const TOKEN *constructor_token = TOKEN::findmatch( tok1, "%any% %var1% (", className );
|
||||
while ( TOKEN::Match( constructor_token, "~" ) )
|
||||
constructor_token = TOKEN::findmatch( constructor_token->next, "%any% %var1% (", className );
|
||||
|
||||
// There are no constructor.
|
||||
if ( ! constructor_token )
|
||||
const TOKEN *constructor_token = TOKEN::findmatch( tok1, "%any% %var1% (", className );
|
||||
while ( TOKEN::Match( constructor_token, "~" ) )
|
||||
constructor_token = TOKEN::findmatch( constructor_token->next, "%any% %var1% (", className );
|
||||
|
||||
// There are no constructor.
|
||||
if ( ! constructor_token )
|
||||
{
|
||||
// If "--style" has been given, give a warning
|
||||
if ( _settings._checkCodingStyle )
|
||||
|
|
@ -409,9 +409,9 @@ void CheckClass::CheckConstructors()
|
|||
// Check if any variables are uninitialized
|
||||
for (struct VAR *var = varlist; var; var = var->next)
|
||||
{
|
||||
// Is it a static member variable?
|
||||
std::ostringstream pattern;
|
||||
pattern << className[0] << "::" << var->name << "=";
|
||||
// Is it a static member variable?
|
||||
std::ostringstream pattern;
|
||||
pattern << className[0] << "::" << var->name << "=";
|
||||
if (TOKEN::findmatch(_tokenizer->tokens(), pattern.str().c_str()))
|
||||
continue;
|
||||
|
||||
|
|
@ -639,7 +639,7 @@ void CheckClass::CheckMemset()
|
|||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
|
||||
|
|
@ -658,58 +658,71 @@ void CheckClass::CheckOperatorEq1()
|
|||
ostr << _tokenizer->fileLine(tok) << ": 'operator=' should return something";
|
||||
_errorLogger->reportErr(ostr.str());
|
||||
}
|
||||
}
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
|
||||
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// A destructor in a base class should be virtual
|
||||
//---------------------------------------------------------------------------
|
||||
}
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
void CheckClass::virtualDestructor()
|
||||
{
|
||||
const TOKEN *derived = TOKEN::findmatch(_tokenizer->tokens(), "class %var% : public|protected|private %var%");
|
||||
while (derived)
|
||||
{
|
||||
// Iterate through each base class...
|
||||
for ( derived = derived->tokAt(3); TOKEN::Match(derived, "public|protected|private %var%"); derived = derived->tokAt(3) )
|
||||
|
||||
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// A destructor in a base class should be virtual
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
void CheckClass::virtualDestructor()
|
||||
{
|
||||
const char pattern_classdecl[] = "class %var% : %var%";
|
||||
|
||||
const TOKEN *derived = TOKEN::findmatch(_tokenizer->tokens(), pattern_classdecl);
|
||||
while (derived)
|
||||
{
|
||||
// Iterate through each base class...
|
||||
derived = derived->tokAt(3);
|
||||
while ( TOKEN::Match(derived, "%var%") )
|
||||
{
|
||||
// Name of base class..
|
||||
const char *baseName[2];
|
||||
baseName[0] = derived->strAt(1);
|
||||
// What kind of inheritance is it.. public|protected|private
|
||||
if ( TOKEN::Match( derived, "public|protected|private" ) )
|
||||
derived = derived->next;
|
||||
|
||||
// Name of base class..
|
||||
const char *baseName[2];
|
||||
baseName[0] = derived->strAt(0);
|
||||
baseName[1] = 0;
|
||||
|
||||
// Find the destructor for the base class.
|
||||
const TOKEN *base = TOKEN::findmatch(_tokenizer->tokens(), "%any% ~ %var1% (", baseName);
|
||||
while (TOKEN::Match(base, "::"))
|
||||
base = TOKEN::findmatch(base->next, "%any% ~ %var1% (", baseName);
|
||||
|
||||
// Check that there is a destructor..
|
||||
if ( ! base )
|
||||
{
|
||||
// Is the class declaration available?
|
||||
base = TOKEN::findmatch(_tokenizer->tokens(), "class %var1% :|{", baseName);
|
||||
if ( base )
|
||||
{
|
||||
std::ostringstream errmsg;
|
||||
errmsg << _tokenizer->fileLine(base) << ": Base class " << baseName[0] << " doesn't have a virtual destructor";
|
||||
_errorLogger->reportErr(errmsg.str());
|
||||
}
|
||||
}
|
||||
|
||||
// There is a destructor. Check that it's virtual..
|
||||
else if ( ! TOKEN::Match(base, "virtual") )
|
||||
{
|
||||
std::ostringstream errmsg;
|
||||
errmsg << _tokenizer->fileLine(base) << ": The destructor for the base class " << baseName[0] << " is not virtual";
|
||||
_errorLogger->reportErr(errmsg.str());
|
||||
}
|
||||
}
|
||||
|
||||
derived = TOKEN::findmatch(derived, "class %var% : public %var%");
|
||||
}
|
||||
}
|
||||
//---------------------------------------------------------------------------
|
||||
// Update derived so it's ready for the next loop.
|
||||
derived = derived->next;
|
||||
if ( TOKEN::Match(derived, ",") )
|
||||
derived = derived->next;
|
||||
|
||||
// Find the destructor declaration for the base class.
|
||||
const TOKEN *base = TOKEN::findmatch(_tokenizer->tokens(), "%any% ~ %var1% (", baseName);
|
||||
while (TOKEN::Match(base, "::"))
|
||||
base = TOKEN::findmatch(base->next, "%any% ~ %var1% (", baseName);
|
||||
|
||||
// Check that there is a destructor..
|
||||
if ( ! base )
|
||||
{
|
||||
// Is the class declaration available?
|
||||
base = TOKEN::findmatch(_tokenizer->tokens(), "class %var1% :|{", baseName);
|
||||
if ( base )
|
||||
{
|
||||
std::ostringstream errmsg;
|
||||
errmsg << _tokenizer->fileLine(base) << ": Base class " << baseName[0] << " doesn't have a virtual destructor";
|
||||
_errorLogger->reportErr(errmsg.str());
|
||||
}
|
||||
}
|
||||
|
||||
// There is a destructor. Check that it's virtual..
|
||||
else if ( ! TOKEN::Match(base, "virtual") )
|
||||
{
|
||||
std::ostringstream errmsg;
|
||||
errmsg << _tokenizer->fileLine(base) << ": The destructor for the base class " << baseName[0] << " is not virtual";
|
||||
_errorLogger->reportErr(errmsg.str());
|
||||
}
|
||||
}
|
||||
|
||||
// Goto next class
|
||||
derived = TOKEN::findmatch(derived, pattern_classdecl);
|
||||
}
|
||||
}
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
|
|
|
|||
109
testclass.cpp
109
testclass.cpp
|
|
@ -35,58 +35,67 @@ public:
|
|||
private:
|
||||
|
||||
void run()
|
||||
{
|
||||
TEST_CASE( virtualDestructor1 );
|
||||
TEST_CASE( virtualDestructor2 );
|
||||
TEST_CASE( virtualDestructor3 );
|
||||
TEST_CASE( virtualDestructor4 );
|
||||
{
|
||||
TEST_CASE( virtualDestructor1 ); // Base class not found => no error
|
||||
TEST_CASE( virtualDestructor2 ); // Base class doesn't have a destructor
|
||||
TEST_CASE( virtualDestructor3 ); // Base class has a destructor, but it's not virtual
|
||||
}
|
||||
|
||||
// Check that base classes have virtual destructors
|
||||
void checkVirtualDestructor(const char code[])
|
||||
{
|
||||
// Tokenize..
|
||||
Tokenizer tokenizer;
|
||||
std::istringstream istr(code);
|
||||
tokenizer.tokenize( istr, "test.cpp" );
|
||||
tokenizer.simplifyTokenList();
|
||||
|
||||
// Clear the error log
|
||||
errout.str("");
|
||||
|
||||
// Check..
|
||||
Settings settings;
|
||||
CheckClass checkClass( &tokenizer, settings, this );
|
||||
checkClass.virtualDestructor();
|
||||
}
|
||||
|
||||
|
||||
void virtualDestructor1()
|
||||
{
|
||||
checkVirtualDestructor("class Derived : public Base { };");
|
||||
ASSERT_EQUALS( std::string(""), errout.str() );
|
||||
}
|
||||
|
||||
void virtualDestructor2()
|
||||
{
|
||||
checkVirtualDestructor("class Base { };\n"
|
||||
"class Derived : public Base { };");
|
||||
ASSERT_EQUALS( std::string("[test.cpp:1]: Base class Base doesn't have a virtual destructor\n"), errout.str() );
|
||||
}
|
||||
|
||||
void virtualDestructor3()
|
||||
{
|
||||
checkVirtualDestructor("class Base { public: ~Base(); };\n"
|
||||
"class Derived : public Base { };");
|
||||
ASSERT_EQUALS( std::string("[test.cpp:1]: The destructor for the base class Base is not virtual\n"), errout.str() );
|
||||
}
|
||||
|
||||
void virtualDestructor4()
|
||||
{
|
||||
checkVirtualDestructor("class Base { public: ~Base(); };\n"
|
||||
"class Derived : private Fred, public Base { };");
|
||||
ASSERT_EQUALS( std::string("[test.cpp:1]: The destructor for the base class Base is not virtual\n"), errout.str() );
|
||||
}
|
||||
// Check that base classes have virtual destructors
|
||||
void checkVirtualDestructor(const char code[])
|
||||
{
|
||||
// Tokenize..
|
||||
Tokenizer tokenizer;
|
||||
std::istringstream istr(code);
|
||||
tokenizer.tokenize( istr, "test.cpp" );
|
||||
tokenizer.simplifyTokenList();
|
||||
|
||||
// Clear the error log
|
||||
errout.str("");
|
||||
|
||||
// Check..
|
||||
Settings settings;
|
||||
CheckClass checkClass( &tokenizer, settings, this );
|
||||
checkClass.virtualDestructor();
|
||||
}
|
||||
|
||||
|
||||
void virtualDestructor1()
|
||||
{
|
||||
// Base class not found
|
||||
|
||||
checkVirtualDestructor("class Derived : public Base { };");
|
||||
ASSERT_EQUALS( std::string(""), errout.str() );
|
||||
|
||||
checkVirtualDestructor("class Derived : Base { };");
|
||||
ASSERT_EQUALS( std::string(""), errout.str() );
|
||||
}
|
||||
|
||||
void virtualDestructor2()
|
||||
{
|
||||
// Base class doesn't have a destructor
|
||||
|
||||
checkVirtualDestructor("class Base { };\n"
|
||||
"class Derived : public Base { };");
|
||||
ASSERT_EQUALS( std::string("[test.cpp:1]: Base class Base doesn't have a virtual destructor\n"), errout.str() );
|
||||
|
||||
checkVirtualDestructor("class Base { };\n"
|
||||
"class Derived : Base { };");
|
||||
ASSERT_EQUALS( std::string("[test.cpp:1]: Base class Base doesn't have a virtual destructor\n"), errout.str() );
|
||||
}
|
||||
|
||||
void virtualDestructor3()
|
||||
{
|
||||
// Base class has a destructor, but it's not virtual
|
||||
|
||||
checkVirtualDestructor("class Base { public: ~Base(); };\n"
|
||||
"class Derived : public Base { };");
|
||||
ASSERT_EQUALS( std::string("[test.cpp:1]: The destructor for the base class Base is not virtual\n"), errout.str() );
|
||||
|
||||
checkVirtualDestructor("class Base { public: ~Base(); };\n"
|
||||
"class Derived : private Fred, public Base { };");
|
||||
ASSERT_EQUALS( std::string("[test.cpp:1]: The destructor for the base class Base is not virtual\n"), errout.str() );
|
||||
}
|
||||
};
|
||||
|
||||
REGISTER_TEST( TestClass )
|
||||
|
|
|
|||
Loading…
Reference in New Issue