From 0d4b87c71ee2231dc0b4187186aa8598d04ac44b Mon Sep 17 00:00:00 2001 From: Robert Reif Date: Sun, 14 Oct 2012 17:30:37 +0200 Subject: [PATCH] SymbolDatabase: Improved find function functionality. Taking arguments into account --- lib/checkautovariables.cpp | 74 +++++++++++++++++--------------------- lib/checkautovariables.h | 2 +- lib/symboldatabase.cpp | 45 ++++++++++++++++++++++- lib/symboldatabase.h | 8 +++++ test/testautovariables.cpp | 33 +++++++++++++++++ 5 files changed, 119 insertions(+), 43 deletions(-) diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index 8db66b0c5..d9c101bc2 100644 --- a/lib/checkautovariables.cpp +++ b/lib/checkautovariables.cpp @@ -222,55 +222,44 @@ void CheckAutoVariables::errorReturnAddressOfFunctionParameter(const Token *tok, //--------------------------------------------------------------------------- // return temporary? -bool CheckAutoVariables::returnTemporary(const Token *tok) const +bool CheckAutoVariables::returnTemporary(const Token *tok, const Scope *startScope) const { - if (!Token::Match(tok, "return %var% (") || !Token::simpleMatch(tok->linkAt(2), ") ;")) - return false; - - const std::string &funcname(tok->next()->str()); - const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); - std::list::const_iterator scope; - bool func = false; // Might it be a function call? bool retref = false; // is there such a function that returns a reference? bool retvalue = false; // is there such a function that returns a value? - for (scope = symbolDatabase->scopeList.begin(); !retref && scope != symbolDatabase->scopeList.end(); ++scope) { - if (scope->type == Scope::eFunction && scope->function && scope->function->type != Function::eConstructor && scope->function->type != Function::eCopyConstructor) { - if (scope->className == funcname) { - retref = scope->classDef->strAt(-1) == "&"; - if (!retref) { - const Token* start = scope->classDef; - while (start->previous() && !Token::Match(start->previous(), ";|}|{|public:|private:|protected:")) { - if ((start->str() == ")" || start->str() == ">") && start->link()) - start = start->link(); - start = start->previous(); - } - if (start->str() == "const") - start = start->next(); - if (start->str() == "::") - start = start->next(); + const Function *function = symbolDatabase->findFunctionByNameAndArgs(tok, startScope); + if (function) { + retref = function->tokenDef->strAt(-1) == "&"; + if (!retref) { + const Token *start = function->tokenDef; + while (start->previous() && !Token::Match(start->previous(), ";|}|{|public:|private:|protected:")) { + if ((start->str() == ")" || start->str() == ">") && start->link()) + start = start->link(); + start = start->previous(); + } + if (start->str() == "const") + start = start->next(); + if (start->str() == "::") + start = start->next(); - if (Token::simpleMatch(start, "std ::")) { - if (start->strAt(3) != "<" || !Token::simpleMatch(start->linkAt(3), "> ::")) - retvalue = true; - else - retref = true; // Assume that a reference is returned - } else { - if (symbolDatabase->isClassOrStruct(start->str())) - retvalue = true; - else - retref = true; - } - - } - func = true; + if (Token::simpleMatch(start, "std ::")) { + if (start->strAt(3) != "<" || !Token::simpleMatch(start->linkAt(3), "> ::")) + retvalue = true; + else + retref = true; // Assume that a reference is returned + } else { + if (symbolDatabase->isClassOrStruct(start->str())) + retvalue = true; + else + retref = true; } } + func = true; } - if (!func && symbolDatabase->isClassOrStruct(funcname)) + if (!func && symbolDatabase->isClassOrStruct(tok->str())) return true; return bool(!retref && retvalue); @@ -318,9 +307,12 @@ void CheckAutoVariables::returnReference() } // return reference to temporary.. - else if (returnTemporary(tok2)) { - // report error.. - errorReturnTempReference(tok2); + else if (Token::Match(tok2, "return %var% (") && + Token::simpleMatch(tok2->linkAt(2), ") ;")) { + if (returnTemporary(tok2->next(), scope)) { + // report error.. + errorReturnTempReference(tok2); + } } } } diff --git a/lib/checkautovariables.h b/lib/checkautovariables.h index 57caf66f4..62c76a780 100644 --- a/lib/checkautovariables.h +++ b/lib/checkautovariables.h @@ -73,7 +73,7 @@ private: * @param tok pointing at the "return" token * @return true if a temporary object is returned */ - bool returnTemporary(const Token *tok) const; + bool returnTemporary(const Token *tok, const Scope *scope) const; void errorReturnAddressToAutoVariable(const Token *tok); void errorReturnPointerToLocalArray(const Token *tok); diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 2a73c23ed..88fd8b7a5 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -982,9 +982,10 @@ Function* SymbolDatabase::addGlobalFunction(Scope*& scope, const Token*& tok, co { Function* function = 0; for (std::list::iterator i = scope->functionList.begin(); i != scope->functionList.end(); ++i) { - if (i->tokenDef->str() == tok->str() && Function::argsMatch(scope, i->argDef, argStart, "", 0)) + if (i->tokenDef->str() == tok->str() && Function::argsMatch(scope, i->argDef->next(), argStart->next(), "", 0)) function = &*i; } + if (!function) function = addGlobalFunctionDecl(scope, argStart, funcStart); @@ -2155,6 +2156,48 @@ const Function* SymbolDatabase::findFunctionByName(const std::string& str, const return 0; } +/** @todo This function only counts the number of arguments in the function call. + It does not take into account functions with default arguments. + It does not take into account argument types. This can be difficult because of promotion and conversion operators and casts and because the argument can also be a function call. + */ +const Function* SymbolDatabase::findFunctionByNameAndArgs(const Token *tok, const Scope *startScope) const +{ + const Scope* currScope = startScope; + while (currScope && currScope->isExecutable()) { + if (currScope->functionOf) + currScope = currScope->functionOf; + else + currScope = currScope->nestedIn; + } + while (currScope) { + for (std::list::const_iterator i = currScope->functionList.begin(); i != currScope->functionList.end(); ++i) { + if (i->tokenDef->str() == tok->str()) { + const Function *func = &*i; + if (tok->strAt(1) == "(" && tok->tokAt(2)) { + // check if function has no arguments + /** @todo check for default arguments */ + if (tok->strAt(2) == ")" && func->argCount() == 0) + return func; + + // check the arguments + unsigned int args = 0; + const Token *arg = tok->tokAt(2); + while (arg) { + /** @todo check argument type for match */ + args++; + arg = arg->nextArgument(); + } + + if (args == func->argCount()) + return func; + } + } + } + currScope = currScope->nestedIn; + } + return 0; +} + //--------------------------------------------------------------------------- const Scope* SymbolDatabase::findScopeByName(const std::string& name) const diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index 810c863f0..62475e595 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -580,6 +580,14 @@ public: const Function* findFunctionByName(const std::string& str, const Scope* startScope) const; + /** + * @brief find a function by name and arguments + * @param tok token of function call + * @param startScope scope to start looking in + * @return pointer to function if found or NULL if not found + */ + const Function* findFunctionByNameAndArgs(const Token *tok, const Scope *startScope) const; + const Scope* findScopeByName(const std::string& name) const; bool isClassOrStruct(const std::string &type) const { diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index d1c2d2a55..148b52daf 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -442,6 +442,17 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:8]: (error) Reference to temporary returned.\n", errout.str()); + // make sure scope is used in function lookup + check("class Fred {\n" + " std::string hello() {\n" + " return std::string();\n" + " }\n" + "};\n" + "std::string &f() {\n" + " return hello();\n" + "}"); + ASSERT_EQUALS("", errout.str()); + check("std::string hello() {\n" " return std::string();\n" "}\n" @@ -470,6 +481,21 @@ private: "}"); ASSERT_EQUALS("[test.cpp:7]: (error) Reference to temporary returned.\n", errout.str()); + // make sure function overloads are handled properly + check("class Foo;\n" + "Foo & hello(bool) {\n" + " static Foo foo;\n" + " return foo;\n" + "}\n" + "Foo hello() {\n" + " return Foo();\n" + "}\n" + "\n" + "Foo& f() {\n" + " return hello(true);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + check("Foo hello() {\n" " return Foo();\n" "}\n" @@ -624,6 +650,13 @@ private: " return a(12);\n" "}"); ASSERT_EQUALS("", errout.str()); + + check("std::string &a(int);\n" + "std::string a();\n" + "std::string &b() {\n" + " return a(12);\n" + "}"); + ASSERT_EQUALS("", errout.str()); }