From c17853949dcb91d7d9a2e1465c7b9237e15bdf77 Mon Sep 17 00:00:00 2001 From: PKEuS Date: Sat, 29 Sep 2012 11:23:30 +0200 Subject: [PATCH] Fixed scope handling problems with CheckOther::checkComparisonOfFuncReturningBool(), removed its experimental status. --- lib/checkother.cpp | 31 +++++++++------------ test/testother.cpp | 67 ++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 74 insertions(+), 24 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index be39c0a24..7a808d38a 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2245,54 +2245,47 @@ void CheckOther::misusedScopeObjectError(const Token *tok, const std::string& va void CheckOther::checkComparisonOfFuncReturningBool() { - // FIXME: This checking is "experimental" because of the false positives - // when self checking lib/preprocessor.cpp (#2617) - if (!_settings->experimental) - return; - if (!_settings->isEnabled("style")) return; if (!_tokenizer->isCPP()) return; + const SymbolDatabase * const symbolDatabase = _tokenizer->getSymbolDatabase(); + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { if (tok->previous() && tok->type() == Token::eComparisonOp && tok->str() != "==" && tok->str() != "!=") { const Token *first_token; bool first_token_func_of_type_bool = false; - bool second_token_func_of_type_bool = false; if (Token::simpleMatch(tok->previous(), ")")) { first_token = tok->previous()->link()->previous(); } else { first_token = tok->previous(); } - - std::string const first_token_name = first_token->str(); - if (first_token->isName()&& isFunction(first_token->str(), _tokenizer->tokens())) { - const Token *fToken = NULL; // TODO: locate function token - if (fToken &&fToken->previous() && fToken->previous()->str() == "bool") { + if (Token::Match(first_token, "%var% (") && !Token::Match(first_token->previous(), "::|.")) { + const Function* func = symbolDatabase->findFunctionByName(first_token->str(), first_token->scope()); + if (func && func->tokenDef && func->tokenDef->strAt(-1) == "bool") { first_token_func_of_type_bool = true; } } Token *second_token = tok->next(); + bool second_token_func_of_type_bool = false; while (second_token->str()=="!") { second_token = second_token->next(); } - std::string const second_token_name = second_token->str(); - if (second_token->isName()&& isFunction(second_token->str(), _tokenizer->tokens())) { - const Token *fToken = NULL; // TODO: locate function token - if (fToken &&fToken->previous() && fToken->previous()->str() == "bool") { + if (Token::Match(second_token, "%var% (") && !Token::Match(second_token->previous(), "::|.")) { + const Function* func = symbolDatabase->findFunctionByName(second_token->str(), second_token->scope()); + if (func && func->tokenDef && func->tokenDef->strAt(-1) == "bool") { second_token_func_of_type_bool = true; } } + if ((first_token_func_of_type_bool == true) && (second_token_func_of_type_bool == true)) { comparisonOfTwoFuncsReturningBoolError(first_token->next(), first_token->str(), second_token->str()); - } - if ((first_token_func_of_type_bool == true) && (second_token_func_of_type_bool == false)) { + } else if (first_token_func_of_type_bool == true) { comparisonOfFuncReturningBoolError(first_token->next(), first_token->str()); - } - if ((first_token_func_of_type_bool == false) && (second_token_func_of_type_bool == true)) { + } else if (second_token_func_of_type_bool == true) { comparisonOfFuncReturningBoolError(second_token->previous(), second_token->str()); } } diff --git a/test/testother.cpp b/test/testother.cpp index a1bb0048f..452aa3acf 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -148,6 +148,7 @@ private: TEST_CASE(checkComparisonOfFuncReturningBool3); TEST_CASE(checkComparisonOfFuncReturningBool4); TEST_CASE(checkComparisonOfFuncReturningBool5); + TEST_CASE(checkComparisonOfFuncReturningBool6); TEST_CASE(checkComparisonOfBoolWithBool); @@ -3992,7 +3993,7 @@ private: " else\n" " return true;\n" "}\n"); - TODO_ASSERT_EQUALS("[test.cpp:3]: (style) Comparison of two functions returning boolean value using relational (<, >, <= or >=) operator.\n", "", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (style) Comparison of two functions returning boolean value using relational (<, >, <= or >=) operator.\n", errout.str()); } void checkComparisonOfFuncReturningBool2() { @@ -4010,7 +4011,7 @@ private: " else\n" " return false;\n" "}\n"); - TODO_ASSERT_EQUALS("[test.cpp:4]: (style) Comparison of a function returning boolean value using relational (<, >, <= or >=) operator.\n", "", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (style) Comparison of a function returning boolean value using relational (<, >, <= or >=) operator.\n", errout.str()); } void checkComparisonOfFuncReturningBool3() { @@ -4027,7 +4028,7 @@ private: " else\n" " return false;\n" "}\n"); - TODO_ASSERT_EQUALS("[test.cpp:3]: (style) Comparison of a function returning boolean value using relational (<, >, <= or >=) operator.\n", "", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (style) Comparison of a function returning boolean value using relational (<, >, <= or >=) operator.\n", errout.str()); } void checkComparisonOfFuncReturningBool4() { @@ -4052,7 +4053,7 @@ private: " else\n" " return false;\n" "}\n"); - TODO_ASSERT_EQUALS("[test.cpp:4]: (style) Comparison of a function returning boolean value using relational (<, >, <= or >=) operator.\n", "", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (style) Comparison of a function returning boolean value using relational (<, >, <= or >=) operator.\n", errout.str()); } void checkComparisonOfFuncReturningBool5() { @@ -4076,7 +4077,63 @@ private: " else\n" " return true;\n" "}\n"); - TODO_ASSERT_EQUALS("[test.cpp:3]: (style) Comparison of two functions returning boolean value using relational (<, >, <= or >=) operator.\n", "", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (style) Comparison of two functions returning boolean value using relational (<, >, <= or >=) operator.\n", errout.str()); + } + + void checkComparisonOfFuncReturningBool6() { + check("int compare1(int temp);\n" + "namespace Foo {\n" + " bool compare1(int temp);\n" + "}\n" + "void f(){\n" + " int temp = 4;\n" + " if(compare1() > compare2()){\n" + " printf(\"foo\");\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("namespace Foo {\n" + " bool compare1(int temp);\n" + "}\n" + "int compare1(int temp);\n" + "void f(){\n" + " int temp = 4;\n" + " if(compare1() > compare2()){\n" + " printf(\"foo\");\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("int compare1(int temp);\n" + "namespace Foo {\n" + " bool compare1(int temp);\n" + " void f(){\n" + " if(compare1() > compare2()){\n" + " printf(\"foo\");\n" + " }\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (style) Comparison of a function returning boolean value using relational (<, >, <= or >=) operator.\n", errout.str()); + + check("int compare1(int temp);\n" + "namespace Foo {\n" + " bool compare1(int temp);\n" + " void f(){\n" + " if(::compare1() > compare2()){\n" + " printf(\"foo\");\n" + " }\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("bool compare1(int temp);\n" + "void f(){\n" + " if(foo.compare1() > compare2()){\n" + " printf(\"foo\");\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void checkComparisonOfBoolWithBool() {