From f5e3dc9a381cf99b58ac5fa2a8b8478609bf3582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 17 Nov 2019 12:08:21 +0100 Subject: [PATCH] Improved fix for #8978 (False positive: Variable assigned value that is never used when assigning via iterator) --- cfg/cppcheck-cfg.rng | 17 +++++++++++++++++ cfg/std.cfg | 6 ++++++ lib/checkunusedvar.cpp | 32 ++++++++++++++++++++++++++++---- lib/library.cpp | 21 +++++++++++++++++++++ lib/library.h | 5 +++++ lib/symboldatabase.cpp | 9 +++++++++ lib/symboldatabase.h | 2 ++ man/reference-cfg-format.md | 18 ++++++++++++++++++ test/testunusedvar.cpp | 13 ++++++++++--- 9 files changed, 116 insertions(+), 7 deletions(-) diff --git a/cfg/cppcheck-cfg.rng b/cfg/cppcheck-cfg.rng index 29040a3db..9f343f776 100644 --- a/cfg/cppcheck-cfg.rng +++ b/cfg/cppcheck-cfg.rng @@ -447,6 +447,23 @@ + + + + + + + + + + + + + + + + + diff --git a/cfg/std.cfg b/cfg/std.cfg index ffa17a69f..140f790c3 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -7993,6 +7993,12 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init + + + std::insert_iterator + std::pair + + diff --git a/lib/checkunusedvar.cpp b/lib/checkunusedvar.cpp index eff8a38be..c8d48fb05 100644 --- a/lib/checkunusedvar.cpp +++ b/lib/checkunusedvar.cpp @@ -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())) - continue; + (!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,9 +1224,20 @@ 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 configuration for " + bailoutTypeName); + } + continue; + } + // warn unreadVariableError(tok, expr->expressionString(), false); + } } // varId, usage {read, write, modified} diff --git a/lib/library.cpp b/lib/library.cpp index ec04c5b5a..6015620cd 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -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(checkName, typeName)] = TypeCheck::check; + else if (checkTypeName == "suppress") + mTypeChecks[std::pair(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(check, typeName)); + return it == mTypeChecks.end() ? TypeCheck::def : it->second; +} diff --git a/lib/library.h b/lib/library.h index ee2cea17f..e97dffb25 100644 --- a/lib/library.h +++ b/lib/library.h @@ -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 xml node Error loadFunction(const tinyxml2::XMLElement * const node, const std::string &name, std::set &unknown_elements); @@ -557,6 +561,7 @@ private: std::map mPodTypes; // pod types std::map mPlatformTypes; // platform independent typedefs std::map mPlatforms; // platform dependent typedefs + std::map, TypeCheck> mTypeChecks; const ArgumentChecks * getarg(const Token *ftok, int argnr) const; diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 09613c068..cd276370f 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -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, diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index 23847144a..30592243e 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -616,6 +616,8 @@ public: return mAccess; } + std::string getTypeName() const; + private: // only symbol database can change the type friend class SymbolDatabase; diff --git a/man/reference-cfg-format.md b/man/reference-cfg-format.md index 5d688cbca..0c631b435 100644 --- a/man/reference-cfg-format.md +++ b/man/reference-cfg-format.md @@ -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 `` and `` is correct. Moreover it must point at a zero-terminated string so `` is also used. +# ``; check or suppress + +The ``configuration tells Cppcheck to show or suppress warnings for a certain type. + +Example: + + + + + + foo + bar + + + + +In the `unusedvar` checking the `foo` type will be checked. Warnings for `bar` type variables will be suppressed. + # `` Libraries can be used to define preprocessor macros as well. For example: diff --git a/test/testunusedvar.cpp b/test/testunusedvar.cpp index 2dc908f86..2a0ba8ffe 100644 --- a/test/testunusedvar.cpp +++ b/test/testunusedvar.cpp @@ -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 configuration for Fred\n", errout.str()); + + functionVariableUsage("void f() {\n" + " std::pair 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() {