Improved fix for #8978 (False positive: Variable assigned value that is never used when assigning via iterator)

This commit is contained in:
Daniel Marjamäki 2019-11-17 12:08:21 +01:00
parent 4ebf54d090
commit f5e3dc9a38
9 changed files with 116 additions and 7 deletions

View File

@ -447,6 +447,23 @@
<attribute name="class-name"><ref name="DATA-EXTNAME"/></attribute>
</element>
<element name="type-checks">
<optional>
<element name="unusedvar">
<zeroOrMore>
<choice>
<element name="check">
<ref name="DATA-EXTNAME"/>
</element>
<element name="suppress">
<ref name="DATA-EXTNAME"/>
</element>
<choice>
</zeroOrMore>
</element>
</optional>
</element>
<element name="podtype">
<attribute name="name"><ref name="DATA-EXTNAME"/></attribute>
<optional>

View File

@ -7993,6 +7993,12 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init
<smart-pointer class-name="std::shared_ptr"/>
<smart-pointer class-name="std::unique_ptr"/>
<smart-pointer class-name="std::weak_ptr"/>
<type-checks>
<unusedvar>
<suppress>std::insert_iterator</suppress>
<check>std::pair</check>
</unusedvar>
</type-checks>
<podtype name="char8_t,std::char8_t" sign="u" size="1"/>
<podtype name="char16_t,std::char16_t" sign="u" size="2"/>
<podtype name="char32_t,std::char32_t" sign="u" size="4"/>

View File

@ -1185,6 +1185,7 @@ void CheckUnusedVar::checkFunctionVariableUsage()
op1tok = op1tok->astOperand1();
const Variable *op1Var = op1tok ? op1tok->variable() : nullptr;
std::string bailoutTypeName;
if (op1Var) {
if (op1Var->isReference() && op1Var->nameToken() != tok->astOperand1())
// todo: check references
@ -1200,9 +1201,21 @@ void CheckUnusedVar::checkFunctionVariableUsage()
// Bailout for unknown template classes, we have no idea what side effects such assignments have
if (mTokenizer->isCPP() &&
op1Var->isClass() &&
(!op1Var->valueType() || op1Var->valueType()->type == ValueType::Type::UNKNOWN_TYPE) &&
Token::findsimplematch(op1Var->typeStartToken(), "<", op1Var->typeEndToken()))
(!op1Var->valueType() || op1Var->valueType()->type == ValueType::Type::UNKNOWN_TYPE)) {
// Check in the library if we should bailout or not..
const std::string typeName = op1Var->getTypeName();
switch (mSettings->library.getTypeCheck("unusedvar", typeName)) {
case Library::TypeCheck::def:
if (!mSettings->checkLibrary)
continue;
bailoutTypeName = typeName;
break;
case Library::TypeCheck::check:
break;
case Library::TypeCheck::suppress:
continue;
};
}
}
// Is there a redundant assignment?
@ -1211,10 +1224,21 @@ void CheckUnusedVar::checkFunctionVariableUsage()
const Token *expr = varDecl ? varDecl : tok->astOperand1();
FwdAnalysis fwdAnalysis(mTokenizer->isCPP(), mSettings->library);
if (fwdAnalysis.unusedValue(expr, start, scope->bodyEnd))
if (fwdAnalysis.unusedValue(expr, start, scope->bodyEnd)) {
if (!bailoutTypeName.empty()) {
if (mSettings->checkLibrary && mSettings->isEnabled(Settings::INFORMATION)) {
reportError(tok,
Severity::information,
"checkLibraryCheckType",
"--check-library: Provide <type-checks><unusedvar> configuration for " + bailoutTypeName);
}
continue;
}
// warn
unreadVariableError(tok, expr->expressionString(), false);
}
}
// varId, usage {read, write, modified}
Variables variables;

View File

@ -511,6 +511,22 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc)
smartPointers.insert(className);
}
else if (nodename == "type-checks") {
for (const tinyxml2::XMLElement *checkNode = node->FirstChildElement(); checkNode; checkNode = checkNode->NextSiblingElement()) {
const std::string &checkName = checkNode->Name();
for (const tinyxml2::XMLElement *checkTypeNode = checkNode->FirstChildElement(); checkTypeNode; checkTypeNode = checkTypeNode->NextSiblingElement()) {
const std::string checkTypeName = checkTypeNode->Name();
const char *typeName = checkTypeNode->GetText();
if (!typeName)
continue;
if (checkTypeName == "check")
mTypeChecks[std::pair<std::string,std::string>(checkName, typeName)] = TypeCheck::check;
else if (checkTypeName == "suppress")
mTypeChecks[std::pair<std::string,std::string>(checkName, typeName)] = TypeCheck::suppress;
}
}
}
else if (nodename == "podtype") {
const char * const name = node->Attribute("name");
if (!name)
@ -1405,3 +1421,8 @@ CPPCHECKLIB const Library::Container * getLibraryContainer(const Token * tok)
return tok->valueType()->container;
}
Library::TypeCheck Library::getTypeCheck(const std::string &check, const std::string &typeName) const
{
auto it = mTypeChecks.find(std::pair<std::string, std::string>(check, typeName));
return it == mTypeChecks.end() ? TypeCheck::def : it->second;
}

View File

@ -478,6 +478,10 @@ public:
static bool isContainerYield(const Token * const cond, Library::Container::Yield y, const std::string& fallback="");
/** Suppress/check a type */
enum class TypeCheck { def, check, suppress };
TypeCheck getTypeCheck(const std::string &check, const std::string &typeName) const;
private:
// load a <function> xml node
Error loadFunction(const tinyxml2::XMLElement * const node, const std::string &name, std::set<std::string> &unknown_elements);
@ -557,6 +561,7 @@ private:
std::map<std::string, struct PodType> mPodTypes; // pod types
std::map<std::string, PlatformType> mPlatformTypes; // platform independent typedefs
std::map<std::string, Platform> mPlatforms; // platform dependent typedefs
std::map<std::pair<std::string,std::string>, TypeCheck> mTypeChecks;
const ArgumentChecks * getarg(const Token *ftok, int argnr) const;

View File

@ -1857,6 +1857,15 @@ const Type *Variable::smartPointerType() const
return nullptr;
}
std::string Variable::getTypeName() const
{
std::string ret;
// TODO: For known types, generate the full type name
for (const Token *typeTok = mTypeStartToken; Token::Match(typeTok, "%name%|::") && typeTok->varId() == 0; typeTok = typeTok->next())
ret += typeTok->str();
return ret;
}
Function::Function(const Tokenizer *mTokenizer,
const Token *tok,
const Scope *scope,

View File

@ -616,6 +616,8 @@ public:
return mAccess;
}
std::string getTypeName() const;
private:
// only symbol database can change the type
friend class SymbolDatabase;

View File

@ -487,6 +487,24 @@ The first argument that the function takes is a pointer. It must not be a null p
The second argument the function takes is a pointer. It must not be null. And it must point at initialized data. Using `<not-null>` and `<not-uninit>` is correct. Moreover it must point at a zero-terminated string so `<strz>` is also used.
# `<type-checks>`; check or suppress
The `<type-checks>`configuration tells Cppcheck to show or suppress warnings for a certain type.
Example:
<?xml version="1.0"?>
<def>
<type-checks>
<unusedvar>
<check>foo</check>
<suppress>bar</suppress>
</unusedvar>
</type-checks>
</def>
In the `unusedvar` checking the `foo` type will be checked. Warnings for `bar` type variables will be suppressed.
# `<define>`
Libraries can be used to define preprocessor macros as well. For example:

View File

@ -33,6 +33,8 @@ private:
void run() OVERRIDE {
settings.addEnabled("style");
settings.addEnabled("information");
settings.checkLibrary = true;
LOAD_LIB_2(settings.library, "std.cfg");
TEST_CASE(emptyclass); // #5355 - False positive: Variable is not assigned a value.
@ -3535,13 +3537,18 @@ private:
"}");
ASSERT_EQUALS("", errout.str());
// FIXME : this is probably inconclusive
functionVariableUsage("void f() {\n"
functionVariableUsage("void f() {\n" // unknown class => library configuration is needed
" Fred fred;\n"
" int *a; a = b;\n"
" fred += a;\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (style) Variable 'fred' is assigned a value that is never used.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (information) --check-library: Provide <type-checks><unusedvar> configuration for Fred\n", errout.str());
functionVariableUsage("void f() {\n"
" std::pair<int,int> fred;\n" // class with library configuration
" fred = x;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (style) Variable 'fred' is assigned a value that is never used.\n", errout.str());
}
void localvarFor() {