fixed #11483 (FN unusedFunction for method with inline implementation) (#5457)

Co-authored-by: chrchr-github <78114321+chrchr-github@users.noreply.github.com>
This commit is contained in:
Oliver Stöneberg 2023-09-20 14:45:44 +02:00 committed by GitHub
parent da09580dde
commit f49fedb2ad
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 87 additions and 54 deletions

View File

@ -1,12 +1,11 @@
# we are not using all methods of their interfaces
unusedFunction:externals/tinyxml2/tinyxml2.cpp
unusedFunction:externals/simplecpp/simplecpp.cpp
unusedFunction:externals/*/*
# TODO: fix these
# false positive - # 10660
unusedFunction:gui/mainwindow.cpp
unusedFunction:gui/resultstree.cpp
unusedFunction:gui/codeeditor.cpp
unusedFunction:gui/codeeditor.*
# usage is disabled
unusedFunction:lib/symboldatabase.cpp
# false positive - #10661

View File

@ -861,7 +861,7 @@ test/testtype.o: test/testtype.cpp lib/check.h lib/checktype.h lib/color.h lib/c
test/testuninitvar.o: test/testuninitvar.cpp lib/check.h lib/checkuninitvar.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testuninitvar.cpp
test/testunusedfunctions.o: test/testunusedfunctions.cpp lib/check.h lib/checkunusedfunctions.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h
test/testunusedfunctions.o: test/testunusedfunctions.cpp externals/simplecpp/simplecpp.h lib/check.h lib/checkunusedfunctions.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testunusedfunctions.cpp
test/testunusedprivfunc.o: test/testunusedprivfunc.cpp externals/simplecpp/simplecpp.h lib/check.h lib/checkclass.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h

View File

@ -78,13 +78,6 @@ public:
return mPathNames;
}
/**
* Return if help is shown to user.
*/
bool getShowHelp() const {
return mShowHelp;
}
/**
* Return if we should exit after printing version, help etc.
*/

View File

@ -83,7 +83,7 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi
const SymbolDatabase* symbolDatabase = tokenizer.getSymbolDatabase();
for (const Scope* scope : symbolDatabase->functionScopes) {
const Function* func = scope->function;
if (!func || !func->token || scope->bodyStart->fileIndex() != 0)
if (!func || !func->token)
continue;
// Don't warn about functions that are marked by __attribute__((constructor)) or __attribute__((destructor))
@ -100,12 +100,15 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi
if (!usage.lineNumber)
usage.lineNumber = func->token->linenr();
usage.fileIndex = func->token->fileIndex();
const std::string& fileName = tokenizer.list.file(func->token);
// No filename set yet..
if (usage.filename.empty()) {
usage.filename = tokenizer.list.getSourceFilePath();
usage.filename = fileName;
}
// Multiple files => filename = "+"
else if (usage.filename != tokenizer.list.getSourceFilePath()) {
else if (usage.filename != fileName) {
//func.filename = "+";
usage.usedOtherFile |= usage.usedSameFile;
}
@ -142,7 +145,7 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi
mFunctions[markupVarToken->str()].usedOtherFile = true;
else if (markupVarToken->next()->str() == "(") {
FunctionUsage &func = mFunctions[markupVarToken->str()];
func.filename = tokenizer.list.getSourceFilePath();
func.filename = tokenizer.list.getFiles()[markupVarToken->fileIndex()];
if (func.filename.empty() || func.filename == "+")
func.usedOtherFile = true;
else
@ -256,7 +259,7 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi
if (funcname) {
const auto baseName = stripTemplateParameters(funcname->str());
FunctionUsage &func = mFunctions[baseName];
const std::string& called_from_file = tokenizer.list.getSourceFilePath();
const std::string& called_from_file = tokenizer.list.getFiles()[funcname->fileIndex()];
if (func.filename.empty() || func.filename == "+" || func.filename != called_from_file)
func.usedOtherFile = true;
@ -318,7 +321,7 @@ static bool isOperatorFunction(const std::string & funcName)
bool CheckUnusedFunctions::check(ErrorLogger * const errorLogger, const Settings& settings) const
{
using ErrorParams = std::tuple<std::string, unsigned int, std::string>;
using ErrorParams = std::tuple<std::string, unsigned int, unsigned int, std::string>;
std::vector<ErrorParams> errors; // ensure well-defined order
for (std::unordered_map<std::string, FunctionUsage>::const_iterator it = mFunctions.cbegin(); it != mFunctions.cend(); ++it) {
@ -333,7 +336,7 @@ bool CheckUnusedFunctions::check(ErrorLogger * const errorLogger, const Settings
std::string filename;
if (func.filename != "+")
filename = func.filename;
errors.emplace_back(filename, func.lineNumber, it->first);
errors.emplace_back(filename, func.fileIndex, func.lineNumber, it->first);
} else if (!func.usedOtherFile) {
/** @todo add error message "function is only used in <file> it can be static" */
/*
@ -346,17 +349,18 @@ bool CheckUnusedFunctions::check(ErrorLogger * const errorLogger, const Settings
}
std::sort(errors.begin(), errors.end());
for (const auto& e : errors)
unusedFunctionError(errorLogger, std::get<0>(e), std::get<1>(e), std::get<2>(e));
unusedFunctionError(errorLogger, std::get<0>(e), std::get<1>(e), std::get<2>(e), std::get<3>(e));
return !errors.empty();
}
void CheckUnusedFunctions::unusedFunctionError(ErrorLogger * const errorLogger,
const std::string &filename, unsigned int lineNumber,
const std::string &filename, unsigned int fileIndex, unsigned int lineNumber,
const std::string &funcname)
{
std::list<ErrorMessage::FileLocation> locationList;
if (!filename.empty()) {
locationList.emplace_back(filename, lineNumber);
locationList.back().fileIndex = fileIndex;
}
const ErrorMessage errmsg(locationList, emptyString, Severity::style, "$symbol:" + funcname + "\nThe function '$symbol' is never used.", "unusedFunction", CWE561, Certainty::normal);
@ -470,7 +474,7 @@ void CheckUnusedFunctions::analyseWholeProgram(const Settings &settings, ErrorLo
if (calls.find(functionName) == calls.end() && !isOperatorFunction(functionName)) {
const Location &loc = decl->second;
unusedFunctionError(errorLogger, loc.fileName, loc.lineNumber, functionName);
unusedFunctionError(errorLogger, loc.fileName, /*fileIndex*/ 0, loc.lineNumber, functionName);
}
}
}

View File

@ -75,7 +75,7 @@ public:
private:
void getErrorMessages(ErrorLogger *errorLogger, const Settings * /*settings*/) const override {
CheckUnusedFunctions::unusedFunctionError(errorLogger, emptyString, 0, "funcName");
CheckUnusedFunctions::unusedFunctionError(errorLogger, emptyString, 0, 0, "funcName");
}
void runChecks(const Tokenizer & /*tokenizer*/, ErrorLogger * /*errorLogger*/) override {}
@ -84,7 +84,7 @@ private:
* Dummy implementation, just to provide error for --errorlist
*/
static void unusedFunctionError(ErrorLogger * const errorLogger,
const std::string &filename, unsigned int lineNumber,
const std::string &filename, unsigned int fileIndex, unsigned int lineNumber,
const std::string &funcname);
static std::string myName() {
@ -98,6 +98,7 @@ private:
struct CPPCHECKLIB FunctionUsage {
std::string filename;
unsigned int lineNumber{};
unsigned int fileIndex{};
bool usedSameFile{};
bool usedOtherFile{};
};

View File

@ -107,22 +107,27 @@ public:
/** get reallocation id for function */
int getReallocId(const Token *tok, int arg) const;
// TODO: get rid of this
/** get allocation info for function by name (deprecated, use other alloc) */
const AllocFunc* getAllocFuncInfo(const char name[]) const {
return getAllocDealloc(mAlloc, name);
}
// TODO: get rid of this
/** get deallocation info for function by name (deprecated, use other alloc) */
const AllocFunc* getDeallocFuncInfo(const char name[]) const {
return getAllocDealloc(mDealloc, name);
}
// TODO: get rid of this
/** get allocation id for function by name (deprecated, use other alloc) */
// cppcheck-suppress unusedFunction
int allocId(const char name[]) const {
const AllocFunc* af = getAllocDealloc(mAlloc, name);
return af ? af->groupId : 0;
}
// TODO: get rid of this
/** get deallocation id for function by name (deprecated, use other alloc) */
int deallocId(const char name[]) const {
const AllocFunc* af = getAllocDealloc(mDealloc, name);
@ -130,16 +135,19 @@ public:
}
/** set allocation id for function */
// cppcheck-suppress unusedFunction - test-only
void setalloc(const std::string &functionname, int id, int arg) {
mAlloc[functionname].groupId = id;
mAlloc[functionname].arg = arg;
}
// cppcheck-suppress unusedFunction - test-only
void setdealloc(const std::string &functionname, int id, int arg) {
mDealloc[functionname].groupId = id;
mDealloc[functionname].arg = arg;
}
// cppcheck-suppress unusedFunction - test-only
void setrealloc(const std::string &functionname, int id, int arg, int reallocArg = 1) {
mRealloc[functionname].groupId = id;
mRealloc[functionname].arg = arg;
@ -147,6 +155,7 @@ public:
}
/** add noreturn function setting */
// cppcheck-suppress unusedFunction - test-only
void setnoreturn(const std::string& funcname, bool noreturn) {
mNoReturn[funcname] = noreturn ? FalseTrueMaybe::True : FalseTrueMaybe::False;
}

View File

@ -350,6 +350,7 @@ public:
* Is variable in a namespace.
* @return true if in a namespace, false if not
*/
// cppcheck-suppress unusedFunction
bool isNamespace() const {
return mAccess == AccessControl::Namespace;
}
@ -1356,6 +1357,7 @@ public:
return const_cast<Scope *>(this->findScope(tok, const_cast<const Scope *>(startScope)));
}
// cppcheck-suppress unusedFunction
bool isVarId(nonneg int varid) const {
return varid < mVariableList.size();
}

View File

@ -3724,9 +3724,7 @@ void TemplateSimplifier::printOut(const std::string & text) const
}
}
void TemplateSimplifier::simplifyTemplates(
const std::time_t maxtime,
bool &codeWithTemplates)
void TemplateSimplifier::simplifyTemplates(const std::time_t maxtime)
{
// convert "sizeof ..." to "sizeof..."
for (Token *tok = mTokenList.front(); tok; tok = tok->next()) {
@ -3796,10 +3794,9 @@ void TemplateSimplifier::simplifyTemplates(
mTemplateNamePos.clear();
}
const bool hasTemplates = getTemplateDeclarations();
getTemplateDeclarations();
if (passCount == 0) {
codeWithTemplates = hasTemplates;
mDump.clear();
for (const TokenAndName& t: mTemplateDeclarations)
mDump += t.dump(mTokenizer.list.getFiles());

View File

@ -307,11 +307,8 @@ public:
/**
* Simplify templates
* @param maxtime time when the simplification should be stopped
* @param codeWithTemplates output parameter that is set if code contains templates
*/
void simplifyTemplates(
const std::time_t maxtime,
bool &codeWithTemplates);
void simplifyTemplates(const std::time_t maxtime);
/**
* Simplify constant calculations such as "1+2" => "3"

View File

@ -441,6 +441,7 @@ public:
void isSigned(const bool sign) {
setFlag(fIsSigned, sign);
}
// cppcheck-suppress unusedFunction
bool isPointerCompare() const {
return getFlag(fIsPointerCompare);
}
@ -549,6 +550,7 @@ public:
bool getCppcheckAttribute(TokenImpl::CppcheckAttributes::Type type, MathLib::bigint &value) const {
return mImpl->getCppcheckAttribute(type, value);
}
// cppcheck-suppress unusedFunction
bool hasCppcheckAttributes() const {
return nullptr != mImpl->mCppcheckAttributes;
}
@ -691,6 +693,7 @@ public:
setFlag(fIsInitComma, b);
}
// cppcheck-suppress unusedFunction
bool isBitfield() const {
return mImpl->mBits > 0;
}
@ -962,6 +965,7 @@ public:
options.files = true;
return options;
}
// cppcheck-suppress unusedFunction
static stringifyOptions forDebugVarId() {
stringifyOptions options = forDebug();
options.varid = true;

View File

@ -4014,8 +4014,7 @@ void Tokenizer::simplifyTemplates()
const std::time_t maxTime = mSettings->templateMaxTime > 0 ? std::time(nullptr) + mSettings->templateMaxTime : 0;
mTemplateSimplifier->simplifyTemplates(
maxTime,
mCodeWithTemplates);
maxTime);
}
//---------------------------------------------------------------------------

View File

@ -606,12 +606,6 @@ private:
bool operatorEnd(const Token * tok) const;
public:
/** Was there templates in the code? */
bool codeWithTemplates() const {
return mCodeWithTemplates;
}
const SymbolDatabase *getSymbolDatabase() const {
return mSymbolDatabase;
}
@ -724,12 +718,6 @@ private:
/** unnamed count "Unnamed0", "Unnamed1", "Unnamed2", ... */
nonneg int mUnnamedCount{};
/**
* was there any templates? templates that are "unused" are
* removed from the token list
*/
bool mCodeWithTemplates{};
/**
* TimerResults
*/

View File

@ -46,10 +46,6 @@ public:
TokenList(const TokenList &) = delete;
TokenList &operator=(const TokenList &) = delete;
void setSettings(const Settings *settings) {
mSettings = settings;
}
/** @return the source file path. e.g. "file.cpp" */
const std::string& getSourceFilePath() const;
@ -205,7 +201,7 @@ private:
std::vector<std::string> mOrigFiles;
/** settings */
const Settings* mSettings{};
const Settings* const mSettings{};
/** File is known to be C/C++ code */
bool mIsC{};

View File

@ -276,6 +276,7 @@ T strToInt(const std::string& str)
* \return size of array
* */
template<typename T, int size>
// cppcheck-suppress unusedFunction - only used in conditional code
std::size_t getArrayLength(const T (& /*unused*/)[size])
{
return size;

View File

@ -444,3 +444,28 @@ typedef int MISRA_5_6_VIOLATION;
'Checking {} ...'.format(test_file)
]
assert stderr == 'test.cpp:1:1: style: msg [addon1-id]\n\n^\n'
# #11483
def test_unused_function_include(tmpdir):
test_cpp_file = os.path.join(tmpdir, 'test.cpp')
with open(test_cpp_file, 'wt') as f:
f.write("""
#include "test.h"
""")
test_h_file = os.path.join(tmpdir, 'test.h')
with open(test_h_file, 'wt') as f:
f.write("""
class A {
public:
void f() {}
// cppcheck-suppress unusedFunction
void f2() {}
};
""")
args = ['--enable=unusedFunction', '--inline-suppr', '--template={file}:{line}:{column}: {severity}:{inconclusive:inconclusive:} {message} [{id}]', test_cpp_file]
_, _, stderr = cppcheck(args)
assert stderr == "{}:4:0: style: The function 'f' is never used. [unusedFunction]\n".format(test_h_file)

View File

@ -125,6 +125,7 @@ std::string PreprocessorHelper::getcode(Preprocessor &preprocessor, const std::s
std::string ret;
try {
// TODO: also preserve location information when #include exists - enabling that will fail since #line is treated like a regular token
ret = preprocessor.getcode(tokens1, cfg, files, filedata.find("#file") != std::string::npos);
} catch (const simplecpp::Output &) {
ret.clear();

View File

@ -256,7 +256,6 @@ private:
REDIRECT;
const char * const argv[] = {"cppcheck"};
ASSERT(parser->parseFromArgs(1, argv));
ASSERT_EQUALS(true, parser->getShowHelp());
ASSERT(startsWith(GET_REDIRECT_OUTPUT, "Cppcheck - A tool for static C/C++ code analysis"));
}
@ -264,7 +263,6 @@ private:
REDIRECT;
const char * const argv[] = {"cppcheck", "-h"};
ASSERT(parser->parseFromArgs(2, argv));
ASSERT_EQUALS(true, parser->getShowHelp());
ASSERT(startsWith(GET_REDIRECT_OUTPUT, "Cppcheck - A tool for static C/C++ code analysis"));
}
@ -272,7 +270,6 @@ private:
REDIRECT;
const char * const argv[] = {"cppcheck", "--help"};
ASSERT(parser->parseFromArgs(2, argv));
ASSERT_EQUALS(true, parser->getShowHelp());
ASSERT(startsWith(GET_REDIRECT_OUTPUT, "Cppcheck - A tool for static C/C++ code analysis"));
}

View File

@ -18,7 +18,9 @@
#include "checkunusedfunctions.h"
#include "errortypes.h"
#include "helpers.h"
#include "platform.h"
#include "preprocessor.h"
#include "settings.h"
#include "fixture.h"
#include "tokenize.h"
@ -74,6 +76,8 @@ private:
TEST_CASE(entrypointsWin);
TEST_CASE(entrypointsWinU);
TEST_CASE(entrypointsUnix);
TEST_CASE(includes);
}
#define check(...) check_(__FILE__, __LINE__, __VA_ARGS__)
@ -673,6 +677,22 @@ private:
"int _fini() { }\n", cppcheck::Platform::Type::Native, &s);
ASSERT_EQUALS("", errout.str());
}
// TODO: fails because the location information is not be preserved by PreprocessorHelper::getcode()
void includes()
{
// #11483
const char inc[] = "class A {\n"
"public:\n"
" void f() {}\n"
"};";
const char code[] = R"(#include "test.h")";
ScopedFile header("test.h", inc);
Preprocessor preprocessor(settings, this);
const std::string processed = PreprocessorHelper::getcode(preprocessor, code, "", "test.cpp");
check(processed.c_str());
TODO_ASSERT_EQUALS("[test.h:3]: (style) The function 'f' is never used.\n", "[test.cpp:3]: (style) The function 'f' is never used.\n", errout.str());
}
};
REGISTER_TEST(TestUnusedFunctions)