Fixed #1132 (Detection of misused scope objects in functions)
Emits error in the form: [useless_lock.cpp:18]: (error) instance of "Lock" object destroyed immediately ...if an instance of a class or struct is unnamed and therefore destroyed straight after creation. Only checks for misused scope objects within functions. Optimised isIdentifierObjectType() by memoizing.
This commit is contained in:
parent
50c2fa9ab0
commit
6e0ef3eda2
|
@ -3842,6 +3842,52 @@ void CheckOther::checkMathFunctions()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
bool CheckOther::isIdentifierObjectType(const Token * const tok)
|
||||||
|
{
|
||||||
|
const std::string identifier = tok->tokAt(1)->str();
|
||||||
|
const MemoizeIsClassResultsIterator found = isClassresults.find(identifier);
|
||||||
|
if (found != isClassresults.end())
|
||||||
|
{
|
||||||
|
return found->second;
|
||||||
|
}
|
||||||
|
|
||||||
|
const std::string classDef = std::string("class|struct ") + identifier;
|
||||||
|
const bool result = Token::findmatch(_tokenizer->tokens(), classDef.c_str());
|
||||||
|
isClassresults.insert(std::make_pair(identifier, result));
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
void CheckOther::checkMisusedScopedObject()
|
||||||
|
{
|
||||||
|
bool withinFuntion = false;
|
||||||
|
unsigned int depth = 0;
|
||||||
|
|
||||||
|
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
|
||||||
|
{
|
||||||
|
withinFuntion |= Token::Match(tok, ") const| {");
|
||||||
|
if (withinFuntion)
|
||||||
|
{
|
||||||
|
if (tok->str() == "{")
|
||||||
|
{
|
||||||
|
++depth;
|
||||||
|
}
|
||||||
|
else if (tok->str() == "}")
|
||||||
|
{
|
||||||
|
--depth;
|
||||||
|
withinFuntion &= depth > 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (withinFuntion && Token::Match(tok, "[;{}] %var% (") && isIdentifierObjectType(tok))
|
||||||
|
{
|
||||||
|
tok = tok->next();
|
||||||
|
misusedScopeObjectError(tok, tok->str());
|
||||||
|
tok = tok->next();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
void CheckOther::postIncrement()
|
void CheckOther::postIncrement()
|
||||||
{
|
{
|
||||||
|
@ -4091,3 +4137,8 @@ void CheckOther::selfAssignmentError(const Token *tok, const std::string &varnam
|
||||||
"selfAssignment", "Redundant assignment of \"" + varname + "\" to itself");
|
"selfAssignment", "Redundant assignment of \"" + varname + "\" to itself");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void CheckOther::misusedScopeObjectError(const Token *tok, const std::string& varname)
|
||||||
|
{
|
||||||
|
reportError(tok, Severity::error,
|
||||||
|
"unusedScopedObject", "instance of \"" + varname + "\" object destroyed immediately");
|
||||||
|
}
|
||||||
|
|
|
@ -87,6 +87,7 @@ public:
|
||||||
|
|
||||||
// New type of check: Check execution paths
|
// New type of check: Check execution paths
|
||||||
checkOther.executionPaths();
|
checkOther.executionPaths();
|
||||||
|
checkOther.checkMisusedScopedObject();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@ -183,6 +184,9 @@ public:
|
||||||
/** @brief %Check for assigning a variable to itself*/
|
/** @brief %Check for assigning a variable to itself*/
|
||||||
void checkSelfAssignment();
|
void checkSelfAssignment();
|
||||||
|
|
||||||
|
/** @brief %Check for objects that are destroyed immediately */
|
||||||
|
void checkMisusedScopedObject();
|
||||||
|
|
||||||
// Error messages..
|
// Error messages..
|
||||||
void cstyleCastError(const Token *tok);
|
void cstyleCastError(const Token *tok);
|
||||||
void dangerousUsageStrtolError(const Token *tok);
|
void dangerousUsageStrtolError(const Token *tok);
|
||||||
|
@ -209,6 +213,7 @@ public:
|
||||||
void fflushOnInputStreamError(const Token *tok, const std::string &varname);
|
void fflushOnInputStreamError(const Token *tok, const std::string &varname);
|
||||||
void redundantAssignmentInSwitchError(const Token *tok, const std::string &varname);
|
void redundantAssignmentInSwitchError(const Token *tok, const std::string &varname);
|
||||||
void selfAssignmentError(const Token *tok, const std::string &varname);
|
void selfAssignmentError(const Token *tok, const std::string &varname);
|
||||||
|
void misusedScopeObjectError(const Token *tok, const std::string &varname);
|
||||||
|
|
||||||
void getErrorMessages()
|
void getErrorMessages()
|
||||||
{
|
{
|
||||||
|
@ -222,6 +227,7 @@ public:
|
||||||
zerodivError(0);
|
zerodivError(0);
|
||||||
mathfunctionCallError(0);
|
mathfunctionCallError(0);
|
||||||
fflushOnInputStreamError(0, "stdin");
|
fflushOnInputStreamError(0, "stdin");
|
||||||
|
misusedScopeObjectError(NULL, "varname");
|
||||||
|
|
||||||
// style
|
// style
|
||||||
cstyleCastError(0);
|
cstyleCastError(0);
|
||||||
|
@ -337,6 +343,18 @@ private:
|
||||||
|
|
||||||
return varname;
|
return varname;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @brief query type of identifier
|
||||||
|
* @param tok Token of the identifier
|
||||||
|
* @return true if the identifier is of type 'class' or 'struct',
|
||||||
|
* false otherwise.
|
||||||
|
*/
|
||||||
|
bool isIdentifierObjectType(const Token* const tok);
|
||||||
|
|
||||||
|
typedef std::map<std::string, bool> MemoizeIsClassResults;
|
||||||
|
typedef MemoizeIsClassResults::const_iterator MemoizeIsClassResultsIterator;
|
||||||
|
MemoizeIsClassResults isClassresults;
|
||||||
};
|
};
|
||||||
/// @}
|
/// @}
|
||||||
//---------------------------------------------------------------------------
|
//---------------------------------------------------------------------------
|
||||||
|
|
|
@ -104,6 +104,13 @@ private:
|
||||||
TEST_CASE(selfAssignment);
|
TEST_CASE(selfAssignment);
|
||||||
TEST_CASE(testScanf1);
|
TEST_CASE(testScanf1);
|
||||||
TEST_CASE(testScanf2);
|
TEST_CASE(testScanf2);
|
||||||
|
|
||||||
|
TEST_CASE(trac1132);
|
||||||
|
TEST_CASE(testMisusedScopeObjectDoesNotPickFunction);
|
||||||
|
TEST_CASE(testMisusedScopeObjectPicksClass);
|
||||||
|
TEST_CASE(testMisusedScopeObjectPicksStruct);
|
||||||
|
TEST_CASE(testMisusedScopeObjectDoesNotPickIf);
|
||||||
|
TEST_CASE(testMisusedScopeObjectDoesNotPickConstructorDeclaration);
|
||||||
}
|
}
|
||||||
|
|
||||||
void check(const char code[])
|
void check(const char code[])
|
||||||
|
@ -134,6 +141,7 @@ private:
|
||||||
checkOther.checkFflushOnInputStream();
|
checkOther.checkFflushOnInputStream();
|
||||||
checkOther.checkSelfAssignment();
|
checkOther.checkSelfAssignment();
|
||||||
checkOther.invalidScanf();
|
checkOther.invalidScanf();
|
||||||
|
checkOther.checkMisusedScopedObject();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@ -2997,6 +3005,101 @@ private:
|
||||||
ASSERT_EQUALS("[test.cpp:6]: (style) scanf without field width limits can crash with huge input data\n"
|
ASSERT_EQUALS("[test.cpp:6]: (style) scanf without field width limits can crash with huge input data\n"
|
||||||
"[test.cpp:7]: (style) scanf without field width limits can crash with huge input data\n", errout.str());
|
"[test.cpp:7]: (style) scanf without field width limits can crash with huge input data\n", errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void trac1132()
|
||||||
|
{
|
||||||
|
errout.str("");
|
||||||
|
|
||||||
|
std::istringstream code("#include <iostream>\n"
|
||||||
|
"class Lock\n"
|
||||||
|
"{\n"
|
||||||
|
"public:\n"
|
||||||
|
" Lock(int i)\n"
|
||||||
|
" {\n"
|
||||||
|
" std::cout << \"Lock \" << i << std::endl;\n"
|
||||||
|
" }\n"
|
||||||
|
" ~Lock()\n"
|
||||||
|
" {\n"
|
||||||
|
" std::cout << \"~Lock\" << std::endl;\n"
|
||||||
|
" }\n"
|
||||||
|
"};\n"
|
||||||
|
"int main()\n"
|
||||||
|
"{\n"
|
||||||
|
" Lock(123);\n"
|
||||||
|
" std::cout << \"hello\" << std::endl;\n"
|
||||||
|
" return 0;\n"
|
||||||
|
"}\n"
|
||||||
|
);
|
||||||
|
|
||||||
|
Tokenizer tokenizer;
|
||||||
|
tokenizer.tokenize(code, "trac1132.cpp");
|
||||||
|
tokenizer.simplifyTokenList();
|
||||||
|
|
||||||
|
Settings settings;
|
||||||
|
|
||||||
|
CheckOther checkOther(&tokenizer, &settings, this);
|
||||||
|
checkOther.checkMisusedScopedObject();
|
||||||
|
|
||||||
|
ASSERT_EQUALS("[trac1132.cpp:16]: (error) instance of \"Lock\" object destroyed immediately\n", errout.str());
|
||||||
|
}
|
||||||
|
|
||||||
|
void testMisusedScopeObjectDoesNotPickFunction()
|
||||||
|
{
|
||||||
|
check("int main ( )\n"
|
||||||
|
"{\n"
|
||||||
|
" CouldBeFunction ( 123 ) ;\n"
|
||||||
|
" return 0 ;\n"
|
||||||
|
"}\n"
|
||||||
|
);
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
}
|
||||||
|
|
||||||
|
void testMisusedScopeObjectPicksClass()
|
||||||
|
{
|
||||||
|
check("class NotAFunction ;\n"
|
||||||
|
"int function ( )\n"
|
||||||
|
"{\n"
|
||||||
|
" NotAFunction ( 123 );\n"
|
||||||
|
" return 0 ;\n"
|
||||||
|
"}\n"
|
||||||
|
);
|
||||||
|
ASSERT_EQUALS("[test.cpp:4]: (error) instance of \"NotAFunction\" object destroyed immediately\n", errout.str());
|
||||||
|
}
|
||||||
|
|
||||||
|
void testMisusedScopeObjectPicksStruct()
|
||||||
|
{
|
||||||
|
check("struct NotAClass;\n"
|
||||||
|
"bool func ( )\n"
|
||||||
|
"{\n"
|
||||||
|
" NotAClass ( 123 ) ;\n"
|
||||||
|
" return true ;\n"
|
||||||
|
"}\n"
|
||||||
|
);
|
||||||
|
ASSERT_EQUALS("[test.cpp:4]: (error) instance of \"NotAClass\" object destroyed immediately\n", errout.str());
|
||||||
|
}
|
||||||
|
|
||||||
|
void testMisusedScopeObjectDoesNotPickIf()
|
||||||
|
{
|
||||||
|
check("bool func( int a , int b , int c )\n"
|
||||||
|
"{\n"
|
||||||
|
" if ( a > b ) return c == a ;\n"
|
||||||
|
" return b == a ;\n"
|
||||||
|
"}\n"
|
||||||
|
);
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
}
|
||||||
|
|
||||||
|
void testMisusedScopeObjectDoesNotPickConstructorDeclaration()
|
||||||
|
{
|
||||||
|
check("class Something : public SomthingElse\n"
|
||||||
|
"{\n"
|
||||||
|
"public:\n"
|
||||||
|
"~Something ( ) ;\n"
|
||||||
|
"Something ( ) ;\n"
|
||||||
|
"}\n"
|
||||||
|
);
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
REGISTER_TEST(TestOther)
|
REGISTER_TEST(TestOther)
|
||||||
|
|
Loading…
Reference in New Issue