Skip to content

Fix #4270: preserve Qt property accessor references for unusedFunction #8838

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.

Sign up for GitHub

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

Open
tzi4 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix #4270: preserve Qt property accessor references for unusedFunction #8838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
69effb8
Fix #4270: preserve Qt property accessor references for unusedFunction
tzi4 Sep 8, 2026
8825355
Fix Qt property CI checks
tzi4 Sep 9, 2026
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
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ $(libcppdir)/pathmatch.o: lib/pathmatch.cpp lib/config.h lib/path.h lib/pathmatc
$(libcppdir)/platform.o: lib/platform.cpp externals/tinyxml2/tinyxml2.h lib/config.h lib/mathlib.h lib/path.h lib/platform.h lib/standards.h lib/utils.h lib/xml.h
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/platform.cpp

$(libcppdir)/preprocessor.o: lib/preprocessor.cpp externals/simplecpp/simplecpp.h lib/checkers.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
$(libcppdir)/preprocessor.o: lib/preprocessor.cpp externals/simplecpp/simplecpp.h lib/checkers.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/preprocessor.h lib/settings.h lib/smallvector.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/vfvalue.h
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/preprocessor.cpp

$(libcppdir)/programmemory.o: lib/programmemory.cpp lib/astutils.h lib/calculate.h lib/checkers.h lib/config.h lib/errortypes.h lib/infer.h lib/library.h lib/mathlib.h lib/platform.h lib/programmemory.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/valueptr.h lib/vfvalue.h
Expand Down
12 changes: 9 additions & 3 deletions cfg/qt.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,16 @@
the READ/WRITE/NOTIFY parts are optional -->
<exported>
<exporter prefix="Q_PROPERTY">
<suffix>READ</suffix>
<!-- catch the element before READ if present -->
<prefix>READ</prefix>
<prefix>WRITE</prefix>
<prefix>NOTIFY</prefix>
<prefix>RESET</prefix>
<prefix>BINDABLE</prefix>
<prefix>DESIGNABLE</prefix>
<prefix>SCRIPTABLE</prefix>
<prefix>STORED</prefix>
<prefix>USER</prefix>
<prefix>EDITABLE</prefix>
</exporter>
</exported>
<!-- qml files can call connect on the c++ code -->
Expand Down Expand Up @@ -5468,7 +5473,8 @@
<define name="Q_PRIVATE_SLOT(d, signature)" value=""/>
<define name="Q_SLOTS" value=""/>
<!-- Treat as variadic macro to avoid preprocessorErrorDirective -->
<define name="Q_PROPERTY(...)" value=""/>
<define name="Q_PROPERTY(...)" value="QT_ANNOTATE_CLASS(qt_property, __VA_ARGS__)"/>
<define name="QT_ANNOTATE_CLASS(type, ...)" value="__cppcheck_qt_annotation__(&quot;cppcheck-qt-annotation&quot;, type, __VA_ARGS__)"/>
<define name="Q_Q(Class)" value="Class * const q = q_func()"/>
<define name="Q_RETURN_ARG(type, data)" value="QReturnArgument&lt;type &gt;(#type, data)"/>
<define name="Q_UNLIKELY(expr)" value="expr"/>
Expand Down
7 changes: 6 additions & 1 deletion lib/checkunusedfunctions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ static bool isRecursiveCall(const Token* ftok)
return ftok->function() && ftok->function() == Scope::nestedInFunction(ftok->scope());
}

void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const Library &library)
void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const Library &library, const std::set<std::string>& exportedFunctions)
{
const char * const FileName = tokenizer.list.getFiles().front().c_str();

Expand Down Expand Up @@ -129,6 +129,11 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const Library
}
}

for (const std::string& name : exportedFunctions) {
mFunctions[name].usedOtherFile = true;
mFunctionCalls.insert(name);
}

// Function usage..
const Token *lambdaEndToken = nullptr;
for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) {
Expand Down
2 changes: 1 addition & 1 deletion lib/checkunusedfunctions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class CPPCHECKLIB CheckUnusedFunctions {
// Parse current tokens and determine..
// * Check what functions are used
// * What functions are declared
void parseTokens(const Tokenizer &tokenizer, const Library &library);
void parseTokens(const Tokenizer &tokenizer, const Library &library, const std::set<std::string>& exportedFunctions = {});

std::string analyzerInfo(const Tokenizer &tokenizer) const;

Expand Down
18 changes: 11 additions & 7 deletions lib/cppcheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1143,7 +1143,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
filesDeleter.addFile(dumpFile);
}

std::set<unsigned long long> hashes;
std::set<std::pair<std::size_t, std::set<std::string>>> hashes;
int checkCount = 0;
bool hasValidConfig = false;
std::list<std::string> configurationError;
Expand Down Expand Up @@ -1270,9 +1270,12 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
mSuppressions.nomsg.markUnmatchedInlineSuppressionsAsChecked(tokenizer.list);
}

// Skip if we already met the same simplified token list
const std::set<std::string> exportedFunctions = mSettings.checks.isEnabled(Checks::unusedFunction) ?
preprocessor.getExportedFunctions() : std::set<std::string>{};

// Macro-only references can differ even when the simplified tokens match.
if (maxConfigs > 1) {
const std::size_t hash = tokenizer.list.calculateHash();
const auto hash = std::make_pair(tokenizer.list.calculateHash(), exportedFunctions);
if (hashes.find(hash) != hashes.end()) {
if (mSettings.debugwarnings)
purgedConfigurationMessage(file.spath(), currentConfig);
Expand All @@ -1282,7 +1285,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
}

// Check normal tokens
checkNormalTokens(tokenizer, analyzerInformation.get(), currentConfig);
checkNormalTokens(tokenizer, analyzerInformation.get(), currentConfig, exportedFunctions);
} catch (const InternalError &e) {
ErrorMessage errmsg = ErrorMessage::fromInternalError(e, &tokenizer.list, file.spath());
mErrorLogger.reportErr(errmsg);
Expand Down Expand Up @@ -1381,7 +1384,8 @@ void CppCheck::internalError(const std::string &filename, const std::string &msg
// CppCheck - A function that checks a normal token list
//---------------------------------------------------------------------------

void CppCheck::checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation* analyzerInformation, const std::string& currentConfig)
void CppCheck::checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation* analyzerInformation, const std::string& currentConfig,
const std::set<std::string>& exportedFunctions)
{
const ProgressReporter progressReporter(mErrorLogger, mSettings.reportProgress, tokenizer.list.getSourceFilePath(), "Run checkers");

Expand Down Expand Up @@ -1420,10 +1424,10 @@ void CppCheck::checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation
}

if (mSettings.checks.isEnabled(Checks::unusedFunction) && !mSettings.buildDir.empty()) {
unusedFunctionsChecker.parseTokens(tokenizer, mSettings.library);
unusedFunctionsChecker.parseTokens(tokenizer, mSettings.library, exportedFunctions);
}
if (mUnusedFunctionsCheck && mSettings.useSingleJob() && mSettings.buildDir.empty()) {
mUnusedFunctionsCheck->parseTokens(tokenizer, mSettings.library);
mUnusedFunctionsCheck->parseTokens(tokenizer, mSettings.library, exportedFunctions);
}

if (mSettings.clang) {
Expand Down
4 changes: 3 additions & 1 deletion lib/cppcheck.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <functional>
#include <list>
#include <memory>
#include <set>
#include <string>
#include <vector>

Expand Down Expand Up @@ -215,7 +216,8 @@ class CPPCHECKLIB CppCheck {
* @param tokenizer tokenizer instance
* @param analyzerInformation the analyzer information
*/
void checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation* analyzerInformation, const std::string& currentConfig);
void checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation* analyzerInformation, const std::string& currentConfig,
const std::set<std::string>& exportedFunctions = {});

/**
* Execute addons
Expand Down
222 changes: 222 additions & 0 deletions lib/preprocessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "settings.h"
#include "standards.h"
#include "suppressions.h"
#include "token.h"
#include "utils.h"

#include <algorithm>
Expand Down Expand Up @@ -893,9 +894,230 @@ simplecpp::TokenList Preprocessor::preprocess(const std::string &cfgStr, std::ve
if (!mSettings.keepComments)
tokens2.removeComments();

mExportedFunctions.clear();
mExportedLocations.clear();
if (mSettings.library.isexporter("Q_PROPERTY"))
readQtAnnotations(tokens2);

return tokens2;
}

static const simplecpp::Token* qtPropertyAttributes(const simplecpp::Token* tok)
{
Comment on lines +905 to +906

@danmar danmar Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an AI review. Take it with a grain of salt and feel free to reject it by resolving the comment.

I built the PR and confirmed the ticket example: value/setValue/resetValue/name referenced from Q_PROPERTY are no longer reported, while a really unused method still is. The 60 tests in test/cli/unused_function_test.py pass.

Design question: qtPropertyAttributes() + qtPropertyFunctions() hard-code Qt's grammar (type parsing with templates/>>, CONSTANT/FINAL/REVISION/MEMBER, ...) in the generic Preprocessor, and they are the only reason preprocessor.cpp now includes token.h (for Token::isStandardType), which pulls in many headers (see the oss-fuzz/Makefile diff). The <exporter prefix=...> list in qt.cfg already describes what matters.

As an experiment, I replaced qtPropertyFunctions() with the generic rule "a name (optionally ::-qualified or parenthesized) following an exporter prefix keyword at depth 0". All but 4 of the 60 CLI tests still pass. The 4 failures are all test_unused_functions_qt_property_keyword_names, where a type or the property itself is named READ/WRITE (class READ {}; Q_PROPERTY(READ property READ value ...)). Is handling such names worth around 60 extra lines of Qt-specific parsing in the preprocessor? In that rare case the worst outcome is that a function named like the type is treated as used. With the generic rule, qtPropertyAttributes() and the token.h include could go away, and the same code could serve any library exporter, not just Q_PROPERTY.

while (tok && (tok->str() == "const" || tok->str() == "volatile"))
tok = tok->next;
if (!tok)
return nullptr;
if (Token::isStandardType(tok->str())) {
do {
tok = tok->next;
} while (tok && Token::isStandardType(tok->str()));
} else {
if (tok->str() == "::")
tok = tok->next;
if (!tok || !tok->name)
return nullptr;
for (;;) {
tok = tok->next;
if (tok && tok->str() == "<") {
unsigned int depth = 1;
unsigned int parentheses = 0;
do {
tok = tok->next;
if (!tok)
return nullptr;
if (tok->str() == "(")
++parentheses;
else if (tok->str() == ")" && parentheses)
--parentheses;
else if (!parentheses) {
if (tok->str() == "<")
++depth;
else if (tok->str() == ">")
--depth;
else if (tok->str() == ">>") {
if (depth < 2)
return nullptr;
depth -= 2;
}
}
} while (depth);
tok = tok->next;
}
if (!tok || tok->str() != "::" || !tok->next || !tok->next->name)
break;
tok = tok->next;
}
}
while (tok && (tok->str() == "*" || tok->str() == "&" || tok->str() == "&&" ||
tok->str() == "const" || tok->str() == "volatile"))
tok = tok->next;
// The property name is not a function reference.
return tok && tok->name ? tok->next : nullptr;
}

static std::set<std::string> qtPropertyFunctions(const simplecpp::Token* tok, const Library& library)
{
std::set<std::string> functions;
tok = qtPropertyAttributes(tok);
while (tok && tok->str() != ")") {
const std::string& attribute = tok->str();
tok = tok->next;
if (attribute == "CONSTANT" || attribute == "FINAL" || attribute == "REQUIRED" ||
attribute == "VIRTUAL" || attribute == "OVERRIDE")
continue;
if (!tok)
return {};
if (attribute == "REVISION") {
if (tok->number)
tok = tok->next;
else if (tok->str() == "(") {
do {
tok = tok->next;
} while (tok && (tok->number || tok->str() == ","));
if (!tok || tok->str() != ")")
return {};
tok = tok->next;
} else
return {};
} else if (attribute == "MEMBER") {
if (!tok->name)
return {};
tok = tok->next;
} else if (library.isexportedprefix("Q_PROPERTY", attribute)) {
const bool parenthesized = tok->str() == "(";
if (parenthesized)
tok = tok->next;
if (!tok)
return {};
if (tok->str() == "::")
tok = tok->next;
if (!tok)
return {};
if (!tok->name)
return {};
std::string function = tok->str();
tok = tok->next;
while (tok && tok->str() == "::" && tok->next && tok->next->name) {
function = tok->next->str();
tok = tok->next->next;
}
if (function != "true" && function != "false" && function != "default")
functions.insert(function);
if (parenthesized) {
if (!tok || tok->str() != ")")
return {};
tok = tok->next;
}
if (tok && tok->str() == "(") {
tok = tok->next;
if (!tok || tok->str() != ")")
return {};
tok = tok->next;
}
} else
return {};
}
return tok ? functions : std::set<std::string>{};
}

void Preprocessor::readQtAnnotations(simplecpp::TokenList& tokens)
{
std::set<simplecpp::Location> locations;
for (const simplecpp::MacroUsage& usage : mMacroUsage) {
if (usage.macroName == "QT_ANNOTATE_CLASS")
locations.insert(usage.useLocation);
}
for (simplecpp::Token* tok = tokens.front(); tok;) {
if (tok->str() != "__cppcheck_qt_annotation__" ||
(tok->macro != "QT_ANNOTATE_CLASS" && locations.find(tok->location) == locations.end()) ||
!tok->next || tok->next->str() != "(") {
tok = tok->next;
continue;
}
const simplecpp::Token* tag = tok->next->next;
if (!tag || tag->str() != "\"cppcheck-qt-annotation\"" || !tag->next || tag->next->str() != ",") {
tok = tok->next;
continue;
}
const simplecpp::Token* end = tok->next;
unsigned int depth = 0;
do {
if (end->str() == "(")
++depth;
else if (end->str() == ")")
--depth;
end = end->next;
} while (end && depth);
if (depth) {
tok = tok->next;
continue;
}
const simplecpp::Token* type = tag->next->next;
if (type && type->str() == "qt_property" && type->next && type->next->str() == ",") {
const auto functions = qtPropertyFunctions(type->next->next, mSettings.library);
mExportedFunctions.insert(functions.begin(), functions.end());
mExportedLocations.insert(tok->location);
}
// Qt's default annotation hook expands to nothing. Retain that C++
// token stream after recording the property metadata, including for
// annotations other than qt_property.
while (tok != end) {
simplecpp::Token* next = tok->next;
tokens.deleteToken(tok);
tok = next;
}
}
}

std::set<std::string> Preprocessor::getExportedFunctions() const
{
std::set<simplecpp::Location> locations;
for (const simplecpp::MacroUsage& usage : mMacroUsage) {
if (mSettings.library.isexporter(usage.macroName) &&
mExportedLocations.find(usage.useLocation) == mExportedLocations.end())
locations.insert(usage.useLocation);
}

std::set<std::string> functions = mExportedFunctions;
if (locations.empty())
return functions;

// Source definitions can erase exporter macros without passing through the
// annotation hook. Inspect only invocations expanded in this configuration
// and not already handled by the hook, excluding inactive branches.
const auto collect = [&](const simplecpp::TokenList& tokens) {
for (const simplecpp::Token* tok = tokens.cfront(); tok; tok = tok->next) {
if (locations.find(tok->location) == locations.end() ||
!mSettings.library.isexporter(tok->str()) || !tok->next || tok->next->str() != "(")
continue;
if (tok->str() == "Q_PROPERTY") {
const auto accessors = qtPropertyFunctions(tok->next->next, mSettings.library);
functions.insert(accessors.begin(), accessors.end());
continue;
}
unsigned int depth = 1;
for (const simplecpp::Token* arg = tok->next->next; arg; arg = arg->next) {
if (arg->str() == "(")
++depth;
else if (arg->str() == ")") {
if (--depth == 0)
break;
} else if (depth == 1) {
if (mSettings.library.isexportedprefix(tok->str(), arg->str()) && arg->next && arg->next->name)
functions.insert(arg->next->str());
if (mSettings.library.isexportedsuffix(tok->str(), arg->str()) && arg->previous && arg->previous->name)
functions.insert(arg->previous->str());
}
}
}
};
collect(mTokens);
for (const auto& fileData : mFileCache)
collect(fileData->tokens);
return functions;
}

std::string Preprocessor::getcode(const std::string &cfgStr, std::vector<std::string> &files, const bool writeLocations)
{
simplecpp::OutputList outputList;
Expand Down
Loading
Loading