Fixed #4656 (New check: Detect pure virtual function calls)

lib/checkclass.cpp

@@ -33,6 +33,29 @@
 // Register CheckClass..
 namespace {
     CheckClass instance;
+
+    const char * getFunctionTypeName(
+        Function::Type type)
+    {
+        switch (type) {
+        case Function::eConstructor:
+            return "constructor";
+        case Function::eCopyConstructor:
+            return "copy constructor";
+        case Function::eDestructor:
+            return "destructor";
+        case Function::eFunction:
+            return "function";
+        case Function::eOperatorEqual:
+            return "operator=";
+        }
+        return "";
+    }
+
+    inline bool isPureWithoutBody(Function const & func)
+    {
+        return func.isPure && !func.hasBody;
+    }
 }
 
 //---------------------------------------------------------------------------
@@ -1845,3 +1868,94 @@ void CheckClass::initializerListError(const Token *tok1, const Token *tok2, cons
                 "in the same order that the members were declared prevents order dependent "
                 "initialization errors.", true);
 }
+
+void CheckClass::checkPureVirtualFunctionCall()
+{
+    const std::size_t functions = symbolDatabase->functionScopes.size();
+    std::map<const Function *, std::list<const Token *> > callsPureVirtualFunctionMap;
+    for (std::size_t i = 0; i < functions; ++i) {
+        const Scope * scope = symbolDatabase->functionScopes[i];
+        if (scope->function == 0 || !scope->function->hasBody ||
+            !(scope->function->type==Function::eConstructor ||
+              scope->function->type==Function::eCopyConstructor ||
+              scope->function->type==Function::eDestructor))
+            continue;
+
+        const std::list<const Token *> & pureVirtualFunctionCalls=callsPureVirtualFunction(*scope->function,callsPureVirtualFunctionMap);
+        for (std::list<const Token *>::const_iterator pureCallIter=pureVirtualFunctionCalls.begin();
+             pureCallIter!=pureVirtualFunctionCalls.end();
+             ++pureCallIter) {
+            const Token & pureCall=**pureCallIter;
+            std::list<const Token *> pureFuncStack;
+            pureFuncStack.push_back(&pureCall);
+            getFirstPureVirtualFunctionCallStack(callsPureVirtualFunctionMap, pureCall, pureFuncStack);
+            if (!pureFuncStack.empty())
+                callsPureVirtualFunctionError(*scope->function, pureFuncStack, pureFuncStack.back()->str());
+        }
+    }
+}
+
+const std::list<const Token *> & CheckClass::callsPureVirtualFunction(const Function & function,
+        std::map<const Function *, std::list<const Token *> > & callsPureVirtualFunctionMap)
+{
+    std::pair<std::map<const Function *, std::list<const Token *> >::iterator , bool > found=
+        callsPureVirtualFunctionMap.insert(std::pair<const Function *, std::list< const Token *> >(&function,std::list<const Token *>()));
+    std::list<const Token *> & pureFunctionCalls=found.first->second;
+    if (found.second) {
+        if (function.hasBody) {
+            for (const Token *tok = function.arg->link();
+                 tok != function.functionScope->classEnd;
+                 tok = tok->next()) {
+                const Function * callFunction=tok->function();
+                if (!callFunction ||
+                    function.nestedIn != callFunction->nestedIn ||
+                    (tok->previous() && tok->previous()->str()=="."))
+                    continue;
+
+                if (isPureWithoutBody(*callFunction)) {
+                    pureFunctionCalls.push_back(tok);
+                    continue;
+                }
+
+                const std::list<const Token *> & pureFunctionCallsOfTok=callsPureVirtualFunction(*callFunction,
+                        callsPureVirtualFunctionMap);
+                if (!pureFunctionCallsOfTok.empty()) {
+                    pureFunctionCalls.push_back(tok);
+                    continue;
+                }
+            }
+        }
+    }
+    return pureFunctionCalls;
+}
+
+void CheckClass::getFirstPureVirtualFunctionCallStack(
+    std::map<const Function *, std::list<const Token *> > & callsPureVirtualFunctionMap,
+    const Token & pureCall,
+    std::list<const Token *> & pureFuncStack)
+{
+    if (isPureWithoutBody(*pureCall.function())) {
+        pureFuncStack.push_back(pureCall.function()->token);
+        return;
+    }
+    std::map<const Function *, std::list<const Token *> >::const_iterator found=callsPureVirtualFunctionMap.find(pureCall.function());
+    if (found==callsPureVirtualFunctionMap.end() ||
+        found->second.empty()) {
+        pureFuncStack.clear();
+        return;
+    }
+    const Token & firstPureCall=**found->second.begin();
+    pureFuncStack.push_back(&firstPureCall);
+    getFirstPureVirtualFunctionCallStack(callsPureVirtualFunctionMap, firstPureCall, pureFuncStack);
+}
+
+void CheckClass::callsPureVirtualFunctionError(
+    const Function & scopeFunction,
+    const std::list<const Token *> & tokStack,
+    const std::string &purefuncname)
+{
+    const char * scopeFunctionTypeName=getFunctionTypeName(scopeFunction.type);
+    reportError(tokStack, Severity::warning, "pureVirtualCall", "Call of pure virtual function '" + purefuncname + "' in " + scopeFunctionTypeName + ".\n"
+                "Call of pure virtual function '" + purefuncname + "' in " + scopeFunctionTypeName + ". The call will fail during runtime.");
+}
+

lib/checkclass.h

@@ -73,6 +73,7 @@ public:
         checkClass.virtualDestructor();
         checkClass.checkConst();
         checkClass.copyconstructors();
+        checkClass.checkPureVirtualFunctionCall();
     }
 
 
@@ -118,6 +119,9 @@ public:
 
     void copyconstructors();
 
+    /** @brief call of pure virtual funcion */
+    void checkPureVirtualFunctionCall();
+
 private:
     const SymbolDatabase *symbolDatabase;
 
@@ -141,6 +145,7 @@ private:
     void checkConstError2(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &funcname, bool suggestStatic);
     void initializerListError(const Token *tok1,const Token *tok2, const std::string & classname, const std::string &varname);
     void suggestInitializationList(const Token *tok, const std::string& varname);
+    void callsPureVirtualFunctionError(const Function & scopeFunction, const std::list<const Token *> & tokStack, const std::string &purefuncname);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
         CheckClass c(0, settings, errorLogger);
@@ -184,7 +189,8 @@ private:
                "* Constness for member functions\n"
                "* Order of initializations\n"
                "* Suggest usage of initialization list\n"
-               "* Suspicious subtraction from 'this'\n";
+               "* Suspicious subtraction from 'this'\n"
+               "* Call of pure virtual function in constructor/desctructor\n";
     }
 
     // operatorEqRetRefThis helper function
@@ -251,6 +257,27 @@ private:
      */
     void initializeVarList(const Function &func, std::list<const Function *> &callstack, const Scope *scope, std::vector<Usage> &usage);
 
+    /**
+     * @brief gives a list of tokens where pure virtual functions are called directly or indirectly
+     * @param function function to be checked
+     * @param callsPureVirtualFunctionMap map of results for already checked functions
+     * @return list of tokens where pure virtual functions are called
+     */
+    const std::list<const Token *> & callsPureVirtualFunction(
+        const Function & function,
+        std::map<const Function *, std::list<const Token *> > & callsPureVirtualFunctionMap);
+
+    /**
+     * @brief looks for the first pure virtual function call stack
+     * @param callsPureVirtualFunctionMap map of results obtained from callsPureVirtualFunction
+     * @param pureCall token where pure virtual function is called directly or indirectly
+     * @param[in,out] pureFuncStack list to append the stack
+     */
+    void getFirstPureVirtualFunctionCallStack(
+        std::map<const Function *, std::list<const Token *> > & callsPureVirtualFunctionMap,
+        const Token & pureCall,
+        std::list<const Token *> & pureFuncStack);
+
     static bool canNotCopy(const Scope *scope);
 };
 /// @}

test/testclass.cpp

@@ -172,6 +172,10 @@ private:
         TEST_CASE(initializerListUsage);
 
         TEST_CASE(forwardDeclaration); // ticket #4290/#3190
+
+        TEST_CASE(pureVirtualFunctionCall);
+        TEST_CASE(pureVirtualFunctionCallOtherClass);
+        TEST_CASE(pureVirtualFunctionCallWithBody);
     }
 
     void checkCopyConstructor(const char code[]) {
@@ -5616,6 +5620,146 @@ private:
                    "class foo;\n");
         ASSERT_EQUALS("", errout.str());
     }
+
+    void checkPureVirtualFunctionCall(const char code[], const Settings *s = 0, bool inconclusive = true) {
+        // Clear the error log
+        errout.str("");
+
+        // Check..
+        Settings settings;
+        if (s)
+            settings = *s;
+        else
+            settings.addEnabled("style");
+        settings.inconclusive = inconclusive;
+
+        // Tokenize..
+        Tokenizer tokenizer(&settings, this);
+        std::istringstream istr(code);
+        tokenizer.tokenize(istr, "test.cpp");
+        tokenizer.simplifyTokenList();
+
+        CheckClass checkClass(&tokenizer, &settings, this);
+        checkClass.checkPureVirtualFunctionCall();
+    }
+
+    void pureVirtualFunctionCall() {
+        checkPureVirtualFunctionCall("class A\n"
+                                     "{\n"
+                                     "    virtual void pure()=0;\n"
+                                     "    A();\n"
+                                     "};\n"
+                                     "A::A()\n"
+                                     "{pure();}\n");
+        ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:3]: (warning) Call of pure virtual function 'pure' in constructor.\n", errout.str());
+
+        checkPureVirtualFunctionCall("class A\n"
+                                     "{\n"
+                                     "    virtual int pure()=0;\n"
+                                     "    A();\n"
+                                     "    int m;\n"
+                                     "};\n"
+                                     "A::A():m(A::pure())\n"
+                                     "{}\n");
+        TODO_ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:3]: (warning) Call of pure virtual function 'pure' in constructor.\n", "", errout.str());
+
+        checkPureVirtualFunctionCall("class A\n"
+                                     " {\n"
+                                     "    virtual void pure()=0; \n"
+                                     "    virtual ~A(); \n"
+                                     "    int m; \n"
+                                     "};\n"
+                                     "A::~A()\n"
+                                     "{pure();}\n");
+        ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:3]: (warning) Call of pure virtual function 'pure' in destructor.\n", errout.str());
+
+        checkPureVirtualFunctionCall("class A\n"
+                                     " {\n"
+                                     "    virtual void pure()=0;\n"
+                                     "    void nonpure()\n"
+                                     "    {pure();}\n"
+                                     "    A(); \n"
+                                     "};\n"
+                                     "A::A()\n"
+                                     "{nonpure();}\n");
+        ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:5] -> [test.cpp:3]: (warning) Call of pure virtual function 'pure' in constructor.\n", errout.str());
+
+        checkPureVirtualFunctionCall("class A\n"
+                                     " {\n"
+                                     "    virtual int pure()=0;\n"
+                                     "    int nonpure()\n"
+                                     "    {return pure();}\n"
+                                     "    A(); \n"
+                                     "    int m;\n"
+                                     "};\n"
+                                     "A::A():m(nonpure())\n"
+                                     "{}\n");
+        TODO_ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:5] -> [test.cpp:3]: (warning) Call of pure virtual function 'pure' in constructor.\n", "", errout.str());
+
+        checkPureVirtualFunctionCall("class A\n"
+                                     " {\n"
+                                     "    virtual void pure()=0; \n"
+                                     "    void nonpure()\n"
+                                     "    {pure();}\n"
+                                     "    virtual ~A();\n"
+                                     "    int m;\n"
+                                     "};\n"
+                                     "A::~A()\n"
+                                     "{nonpure();}\n");
+        ASSERT_EQUALS("[test.cpp:10] -> [test.cpp:5] -> [test.cpp:3]: (warning) Call of pure virtual function 'pure' in destructor.\n", errout.str());
+    }
+
+    void pureVirtualFunctionCallOtherClass() {
+        checkPureVirtualFunctionCall("class A\n"
+                                     "{\n"
+                                     "    virtual void pure()=0;\n"
+                                     "    A(const A & a);\n"
+                                     "};\n"
+                                     "A::A(const A & a)\n"
+                                     "{a.pure();}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkPureVirtualFunctionCall("class A\n"
+                                     "{\n"
+                                     "    virtual void pure()=0;\n"
+                                     "    A();\n"
+                                     "};\n"
+                                     "class B\n"
+                                     "{\n"
+                                     "    virtual void pure()=0;\n"
+                                     "};\n"
+                                     "A::A()\n"
+                                     "{B b; b.pure();}\n");
+        ASSERT_EQUALS("", errout.str());
+    }
+
+    void pureVirtualFunctionCallWithBody() {
+        checkPureVirtualFunctionCall("class A\n"
+                                     "{\n"
+                                     "    virtual void pureWithBody()=0;\n"
+                                     "    A();\n"
+                                     "};\n"
+                                     "A::A()\n"
+                                     "{pureWithBody();}\n"
+                                     "void A::pureWithBody()\n"
+                                     "{}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkPureVirtualFunctionCall("class A\n"
+                                     " {\n"
+                                     "    virtual void pureWithBody()=0;\n"
+                                     "    void nonpure()\n"
+                                     "    {pureWithBody();}\n"
+                                     "    A(); \n"
+                                     "};\n"
+                                     "A::A()\n"
+                                     "{nonpure();}\n"
+                                     "void A::pureWithBody()\n"
+                                     "{}\n");
+        ASSERT_EQUALS("", errout.str());
+
+    }
+
 };
 
 REGISTER_TEST(TestClass)