From 986ec42d7951343fe2a76e741a5bd1c4670803fb Mon Sep 17 00:00:00 2001 From: Alexander Mai Date: Thu, 22 Aug 2013 06:38:54 +0200 Subject: [PATCH] Fixed #4937 (false positive: Assert calls a function which may have desired side effects) --- lib/checkassert.cpp | 5 ++++- lib/symboldatabase.cpp | 1 + test/testassert.cpp | 45 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/lib/checkassert.cpp b/lib/checkassert.cpp index cb006d028..da7e51543 100644 --- a/lib/checkassert.cpp +++ b/lib/checkassert.cpp @@ -69,7 +69,10 @@ void CheckAssert::assertWithSideEffects() if (tok2->type() != Token::eAssignmentOp && tok2->type() != Token::eIncDecOp) continue; const Variable* var = tok2->previous()->variable(); if (!var || var->isLocal()) continue; - + if (var->isArgument() && + (var->isConst() || (!var->isReference() && !var->isPointer()))) + // see ticket #4937. Assigning function arguments not passed by reference is ok. + continue; std::vector returnTokens; // find all return statements for (const Token *rt = scope->classStart; rt != scope->classEnd; rt = rt->next()) { if (!Token::Match(rt, "return %any%")) continue; diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index f2bb4db17..5e81040f9 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -1610,6 +1610,7 @@ void SymbolDatabase::printVariable(const Variable *var, const char *indent) cons std::cout << indent << " isMutable: " << (var->isMutable() ? "true" : "false") << std::endl; std::cout << indent << " isStatic: " << (var->isStatic() ? "true" : "false") << std::endl; std::cout << indent << " isExtern: " << (var->isExtern() ? "true" : "false") << std::endl; + std::cout << indent << " isLocal: " << (var->isLocal() ? "true" : "false") << std::endl; std::cout << indent << " isConst: " << (var->isConst() ? "true" : "false") << std::endl; std::cout << indent << " isClass: " << (var->isClass() ? "true" : "false") << std::endl; std::cout << indent << " isArray: " << (var->isArray() ? "true" : "false") << std::endl; diff --git a/test/testassert.cpp b/test/testassert.cpp index 796b4c84b..a252d9c7b 100644 --- a/test/testassert.cpp +++ b/test/testassert.cpp @@ -89,6 +89,51 @@ private: "assert(foo() == 3); \n" ); ASSERT_EQUALS("[test.cpp:6]: (warning) Assert statement calls a function which may have desired side effects: 'foo'.\n", errout.str()); + + // Ticket #4937 "false positive: Assert calls a function which may have desired side effects" + check("struct SquarePack {\n" + " static bool isRank1Or8( Square sq ) {\n" + " sq &= 0x38;\n" + " return sq == 0 || sq == 0x38;\n" + " }\n" + "};\n" + "void foo() {\n" + " assert( !SquarePack::isRank1Or8(push2) );\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct SquarePack {\n" + " static bool isRank1Or8( Square &sq ) {\n" + " sq &= 0x38;\n" + " return sq == 0 || sq == 0x38;\n" + " }\n" + "};\n" + "void foo() {\n" + " assert( !SquarePack::isRank1Or8(push2) );\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:8]: (warning) Assert statement calls a function which may have desired side effects: 'isRank1Or8'.\n", errout.str()); + + check("struct SquarePack {\n" + " static bool isRank1Or8( Square *sq ) {\n" + " *sq &= 0x38;\n" + " return *sq == 0 || *sq == 0x38;\n" + " }\n" + "};\n" + "void foo() {\n" + " assert( !SquarePack::isRank1Or8(push2) );\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:8]: (warning) Assert statement calls a function which may have desired side effects: 'isRank1Or8'.\n", errout.str()); + + check("struct SquarePack {\n" + " static bool isRank1Or8( Square *sq ) {\n" + " sq &= 0x38;\n" + " return sq == 0 || sq == 0x38;\n" + " }\n" + "};\n" + "void foo() {\n" + " assert( !SquarePack::isRank1Or8(push2) );\n" + "}\n"); + TODO_ASSERT_EQUALS("", "[test.cpp:8]: (warning) Assert statement calls a function which may have desired side effects: 'isRank1Or8'.\n", errout.str()); } void assignmentInAssert() {