From 057d6f1f187baaec4263141115bdf9bc69271d52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 7 Dec 2008 06:31:54 +0000 Subject: [PATCH] Virtual destructors : Handle inheritance where "private|protected|public" is not defined --- CheckClass.cpp | 239 ++++++++++++++++++++++++++----------------------- testclass.cpp | 109 +++++++++++----------- 2 files changed, 185 insertions(+), 163 deletions(-) diff --git a/CheckClass.cpp b/CheckClass.cpp index 48d7ff6e2..7799b8229 100644 --- a/CheckClass.cpp +++ b/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); + } +} +//--------------------------------------------------------------------------- diff --git a/testclass.cpp b/testclass.cpp index a312c841b..0b99ebb01 100644 --- a/testclass.cpp +++ b/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 )