From a12fd4ff5ea49ae887dff303a6590ac8fb5c37da Mon Sep 17 00:00:00 2001
From: Reijo Tomperi <aggro80@users.sourceforge.net>
Date: Wed, 6 May 2009 23:22:26 +0300
Subject: [PATCH] Fix ticket #282 (protected destructor - false positive)
 http://apps.sourceforge.net/trac/cppcheck/ticket/282

---
 src/checkclass.cpp | 39 ++++++++++++++++++++++++++++++++++++---
 test/testclass.cpp | 18 ++++++++++++++++++
 2 files changed, 54 insertions(+), 3 deletions(-)

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[])
     {