Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions .selfcheck_unused_suppressions
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 0 additions & 7 deletions cli/cmdlineparser.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,6 @@ class CmdLineParser {
return mPathNames;
}

/**
* Return if help is shown to user.
*/
bool getShowHelp() const {
return mShowHelp;
}

/**
* Return if we should exit after printing version, help etc.
*/
Expand Down
24 changes: 14 additions & 10 deletions lib/checkunusedfunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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" */
/*
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
}
}
5 changes: 3 additions & 2 deletions lib/checkunusedfunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class CPPCHECKLIB CheckUnusedFunctions : public Check {

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 {}
Expand All @@ -84,7 +84,7 @@ class CPPCHECKLIB CheckUnusedFunctions : public Check {
* 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() {
Expand All @@ -98,6 +98,7 @@ class CPPCHECKLIB CheckUnusedFunctions : public Check {
struct CPPCHECKLIB FunctionUsage {
std::string filename;
unsigned int lineNumber{};
unsigned int fileIndex{};
bool usedSameFile{};
bool usedOtherFile{};
};
Expand Down
9 changes: 9 additions & 0 deletions lib/library.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,46 +107,55 @@ class CPPCHECKLIB Library {
/** 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);
return af ? af->groupId : 0;
}

/** 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;
mRealloc[functionname].reallocArg = reallocArg;
}

/** add noreturn function setting */
// cppcheck-suppress unusedFunction - test-only
void setnoreturn(const std::string& funcname, bool noreturn) {
mNoReturn[funcname] = noreturn ? FalseTrueMaybe::True : FalseTrueMaybe::False;
}
Expand Down
2 changes: 2 additions & 0 deletions lib/symboldatabase.h
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ class CPPCHECKLIB Variable {
* Is variable in a namespace.
* @return true if in a namespace, false if not
*/
// cppcheck-suppress unusedFunction
bool isNamespace() const {
return mAccess == AccessControl::Namespace;
}
Expand Down Expand Up @@ -1356,6 +1357,7 @@ class CPPCHECKLIB SymbolDatabase {
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();
}
Expand Down
7 changes: 2 additions & 5 deletions lib/templatesimplifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down Expand Up @@ -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());
Expand Down
5 changes: 1 addition & 4 deletions lib/templatesimplifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,8 @@ class CPPCHECKLIB TemplateSimplifier {
/**
* 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"
Expand Down
4 changes: 4 additions & 0 deletions lib/token.h
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,7 @@ class CPPCHECKLIB Token {
void isSigned(const bool sign) {
setFlag(fIsSigned, sign);
}
// cppcheck-suppress unusedFunction
chrchr-github marked this conversation as resolved.
Show resolved Hide resolved
bool isPointerCompare() const {
return getFlag(fIsPointerCompare);
}
Expand Down Expand Up @@ -549,6 +550,7 @@ class CPPCHECKLIB Token {
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;
}
Expand Down Expand Up @@ -691,6 +693,7 @@ class CPPCHECKLIB Token {
setFlag(fIsInitComma, b);
}

// cppcheck-suppress unusedFunction
bool isBitfield() const {
return mImpl->mBits > 0;
}
Expand Down Expand Up @@ -962,6 +965,7 @@ class CPPCHECKLIB Token {
options.files = true;
return options;
}
// cppcheck-suppress unusedFunction
static stringifyOptions forDebugVarId() {
stringifyOptions options = forDebug();
options.varid = true;
Expand Down
3 changes: 1 addition & 2 deletions lib/tokenize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
//---------------------------------------------------------------------------

Expand Down
12 changes: 0 additions & 12 deletions lib/tokenize.h
Original file line number Diff line number Diff line change
Expand Up @@ -606,12 +606,6 @@ class CPPCHECKLIB Tokenizer {
bool operatorEnd(const Token * tok) const;

public:

/** Was there templates in the code? */
bool codeWithTemplates() const {
return mCodeWithTemplates;
}

const SymbolDatabase *getSymbolDatabase() const {
return mSymbolDatabase;
}
Expand Down Expand Up @@ -724,12 +718,6 @@ class CPPCHECKLIB Tokenizer {
/** 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
*/
Expand Down
6 changes: 1 addition & 5 deletions lib/tokenlist.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ class CPPCHECKLIB TokenList {
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;

Expand Down Expand Up @@ -205,7 +201,7 @@ class CPPCHECKLIB TokenList {
std::vector<std::string> mOrigFiles;

/** settings */
const Settings* mSettings{};
const Settings* const mSettings{};

/** File is known to be C/C++ code */
bool mIsC{};
Expand Down
1 change: 1 addition & 0 deletions lib/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
25 changes: 25 additions & 0 deletions test/cli/test-other.py
Original file line number Diff line number Diff line change
Expand Up @@ -444,3 +444,28 @@ def test_addon_result(tmpdir):
'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)
1 change: 1 addition & 0 deletions test/helpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading
Loading