From 132ed4b273cc4fc1abba6705231e91d8a33199b7 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Mon, 8 Apr 2024 16:59:24 +0200 Subject: [PATCH 01/23] Update checkclass.cpp --- lib/checkclass.cpp | 48 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 627f15574b7..87caed65778 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -3303,6 +3303,54 @@ void CheckClass::checkUselessOverride() } } +static const Variable* getSingleReturnVar(const Scope* scope) { + const Token* const start = scope->bodyStart->next(); + const Token* const end = Token::findsimplematch(start, ";", 1, scope->bodyEnd); + if (!end || end->next() != scope->bodyEnd) + return nullptr; + if (start->str() != "return") + return nullptr; + return start->astOperand1()->variable(); +} + +void CheckClass::checkReturnReference() +{ + if (!mSettings->severity.isEnabled(Severity::style) && !mSettings->isPremiumEnabled("returnReference")) + return; + + logChecker("CheckClass::checkReturnReference"); // style + + for (const Scope* classScope : mSymbolDatabase->classAndStructScopes) { + for (const Function& func : classScope->functionList) { + if (!func.functionScope) + continue; + if (Function::returnsPointer(&func) || Function::returnsReference(&func) || Function::returnsStandardType(&func)) + continue; + if (const Variable* var = getSingleReturnVar(func.functionScope)) { + if (!var->valueType()) + continue; + const bool isContainer = var->valueType()->type == ValueType::Type::CONTAINER && var->valueType()->container; + const bool isView = isContainer && var->valueType()->container->view; + bool warn = isContainer && !isView; + if (!warn && !isView) { + const std::size_t size = ValueFlow::getSizeOf(*var->valueType(), *mSettings); + if (size > 2 * mSettings->platform.sizeof_pointer) + warn = true; + } + if (warn) + returnReferenceError(&func, var); + } + } + } +} + +void CheckClass::returnReferenceError(const Function* func, const Variable* var) +{ + const Token* tok = func ? func->tokenDef : nullptr; + const std::string message = "Function '" + (func ? func->name() : "") + "()' should return member '" + (var ? var->name() : "") + "' by const reference."; + reportError(tok, Severity::style, "returnReference", message); +} + void CheckClass::checkThisUseAfterFree() { if (!mSettings->severity.isEnabled(Severity::warning)) From b1e615f9991b35068b7319509ad0df9efbdd385e Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Mon, 8 Apr 2024 17:00:11 +0200 Subject: [PATCH 02/23] Update checkclass.h --- lib/checkclass.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/checkclass.h b/lib/checkclass.h index 5a84e4cbdaf..da0c618adb8 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -91,6 +91,7 @@ class CPPCHECKLIB CheckClass : public Check { checkClass.checkCopyCtorAndEqOperator(); checkClass.checkOverride(); checkClass.checkUselessOverride(); + checkClass.checkReturnReference(); checkClass.checkThisUseAfterFree(); checkClass.checkUnsafeClassRefMember(); } @@ -157,6 +158,9 @@ class CPPCHECKLIB CheckClass : public Check { /** @brief Check that the overriden function is not identical to the base function */ void checkUselessOverride(); + /** @brief Check that large members are returned by reference from getter function */ + void checkReturnReference(); + /** @brief When "self pointer" is destroyed, 'this' might become invalid. */ void checkThisUseAfterFree(); @@ -208,6 +212,7 @@ class CPPCHECKLIB CheckClass : public Check { void copyCtorAndEqOperatorError(const Token *tok, const std::string &classname, bool isStruct, bool hasCopyCtor); void overrideError(const Function *funcInBase, const Function *funcInDerived); void uselessOverrideError(const Function *funcInBase, const Function *funcInDerived, bool isSameCode = false); + void returnReferenceError(const Function *func, const Variable* var); void thisUseAfterFree(const Token *self, const Token *free, const Token *use); void unsafeClassRefMemberError(const Token *tok, const std::string &varname); void checkDuplInheritedMembersRecursive(const Type* typeCurrent, const Type* typeBase); @@ -246,6 +251,9 @@ class CPPCHECKLIB CheckClass : public Check { c.selfInitializationError(nullptr, "var"); c.duplInheritedMembersError(nullptr, nullptr, "class", "class", "variable", false, false); c.copyCtorAndEqOperatorError(nullptr, "class", false, false); + c.overrideError(nullptr, nullptr); + c.uselessOverrideError(nullptr, nullptr); + c.returnReferenceError(nullptr, nullptr); c.pureVirtualFunctionCallInConstructorError(nullptr, std::list(), "f"); c.virtualFunctionCallInConstructorError(nullptr, std::list(), "f"); c.overrideError(nullptr, nullptr); From ec2c56e8ed7e3e60323d1780a38e3f705c8e4807 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Mon, 8 Apr 2024 17:03:56 +0200 Subject: [PATCH 03/23] Update checkclass.h --- lib/checkclass.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/checkclass.h b/lib/checkclass.h index da0c618adb8..0e882be6305 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -212,7 +212,7 @@ class CPPCHECKLIB CheckClass : public Check { void copyCtorAndEqOperatorError(const Token *tok, const std::string &classname, bool isStruct, bool hasCopyCtor); void overrideError(const Function *funcInBase, const Function *funcInDerived); void uselessOverrideError(const Function *funcInBase, const Function *funcInDerived, bool isSameCode = false); - void returnReferenceError(const Function *func, const Variable* var); + void returnByReferenceError(const Function *func, const Variable* var); void thisUseAfterFree(const Token *self, const Token *free, const Token *use); void unsafeClassRefMemberError(const Token *tok, const std::string &varname); void checkDuplInheritedMembersRecursive(const Type* typeCurrent, const Type* typeBase); @@ -253,7 +253,7 @@ class CPPCHECKLIB CheckClass : public Check { c.copyCtorAndEqOperatorError(nullptr, "class", false, false); c.overrideError(nullptr, nullptr); c.uselessOverrideError(nullptr, nullptr); - c.returnReferenceError(nullptr, nullptr); + c.returnByReferenceError(nullptr, nullptr); c.pureVirtualFunctionCallInConstructorError(nullptr, std::list(), "f"); c.virtualFunctionCallInConstructorError(nullptr, std::list(), "f"); c.overrideError(nullptr, nullptr); From 5699aeedd4c567141f0449397cda3a6f97e0fc3e Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Mon, 8 Apr 2024 17:04:13 +0200 Subject: [PATCH 04/23] Update checkclass.cpp --- lib/checkclass.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 87caed65778..3e9bb2ee0bb 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -3338,17 +3338,17 @@ void CheckClass::checkReturnReference() warn = true; } if (warn) - returnReferenceError(&func, var); + returnByReferenceError(&func, var); } } } } -void CheckClass::returnReferenceError(const Function* func, const Variable* var) +void CheckClass::returnByReferenceError(const Function* func, const Variable* var) { const Token* tok = func ? func->tokenDef : nullptr; const std::string message = "Function '" + (func ? func->name() : "") + "()' should return member '" + (var ? var->name() : "") + "' by const reference."; - reportError(tok, Severity::style, "returnReference", message); + reportError(tok, Severity::style, "returnByReference", message); } void CheckClass::checkThisUseAfterFree() From ffc8a92d9e0ede47c595f66c6adb9b34eee8ffbe Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Mon, 8 Apr 2024 17:09:56 +0200 Subject: [PATCH 05/23] Update checkclass.cpp --- lib/checkclass.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 3e9bb2ee0bb..6317aca9431 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -30,6 +30,7 @@ #include "tokenize.h" #include "tokenlist.h" #include "utils.h" +#include "valueflow.h" #include #include From 45b437e06f390e3f6b73f240a2f28176872d333a Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Mon, 8 Apr 2024 17:23:35 +0200 Subject: [PATCH 06/23] Update checkclass.h --- lib/checkclass.h | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/checkclass.h b/lib/checkclass.h index 0e882be6305..4808944f214 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -256,7 +256,6 @@ class CPPCHECKLIB CheckClass : public Check { c.returnByReferenceError(nullptr, nullptr); c.pureVirtualFunctionCallInConstructorError(nullptr, std::list(), "f"); c.virtualFunctionCallInConstructorError(nullptr, std::list(), "f"); - c.overrideError(nullptr, nullptr); c.thisUseAfterFree(nullptr, nullptr, nullptr); c.unsafeClassRefMemberError(nullptr, "UnsafeClass::var"); } From 08dbfe35e37a594234f1e6bff3e0ede9ee2bc9c7 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 8 Apr 2024 20:50:54 +0200 Subject: [PATCH 07/23] clang-tidy --- lib/platform.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/platform.h b/lib/platform.h index 5e4e51e71f6..6c2d17dfb9d 100644 --- a/lib/platform.h +++ b/lib/platform.h @@ -88,17 +88,17 @@ class CPPCHECKLIB Platform { nonneg int long_long_bit; /// bits in long long /** size of standard types */ - nonneg int sizeof_bool; - nonneg int sizeof_short; - nonneg int sizeof_int; - nonneg int sizeof_long; - nonneg int sizeof_long_long; - nonneg int sizeof_float; - nonneg int sizeof_double; - nonneg int sizeof_long_double; - nonneg int sizeof_wchar_t; - nonneg int sizeof_size_t; - nonneg int sizeof_pointer; + std::size_t sizeof_bool; + std::size_t sizeof_short; + std::size_t sizeof_int; + std::size_t sizeof_long; + std::size_t sizeof_long_long; + std::size_t sizeof_float; + std::size_t sizeof_double; + std::size_t sizeof_long_double; + std::size_t sizeof_wchar_t; + std::size_t sizeof_size_t; + std::size_t sizeof_pointer; char defaultSign; // unsigned:'u', signed:'s', unknown:'\0' From d2e59852cb27a65f15c9a4e55a475102ac030435 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 8 Apr 2024 20:58:20 +0200 Subject: [PATCH 08/23] const ref --- gui/application.h | 6 +++--- gui/checkstatistics.h | 4 ++-- gui/codeeditor.h | 2 +- gui/printablereport.h | 2 +- gui/projectfile.h | 36 ++++++++++++++++++------------------ gui/projectfiledialog.h | 2 +- gui/resultstree.h | 2 +- gui/translationhandler.h | 4 ++-- lib/templatesimplifier.h | 2 +- 9 files changed, 30 insertions(+), 30 deletions(-) diff --git a/gui/application.h b/gui/application.h index e6d92ee853b..d624ecf23e1 100644 --- a/gui/application.h +++ b/gui/application.h @@ -49,7 +49,7 @@ class Application { * @brief Get application name. * @return Application name. */ - QString getName() const { + const QString& getName() const { return mName; } @@ -57,7 +57,7 @@ class Application { * @brief Get application path. * @return Application path. */ - QString getPath() const { + const QString& getPath() const { return mPath; } @@ -65,7 +65,7 @@ class Application { * @brief Get application command line parameters. * @return Application command line parameters. */ - QString getParameters() const { + const QString& getParameters() const { return mParameters; } diff --git a/gui/checkstatistics.h b/gui/checkstatistics.h index 65fa7783b2f..9e064184d99 100644 --- a/gui/checkstatistics.h +++ b/gui/checkstatistics.h @@ -68,7 +68,7 @@ class CheckStatistics : public QObject { */ unsigned getCount(const QString &tool, ShowTypes::ShowType type) const; - std::set getActiveCheckers() const { + const std::set& getActiveCheckers() const { return mActiveCheckers; } @@ -82,7 +82,7 @@ class CheckStatistics : public QObject { void setCheckersReport(QString report) { mCheckersReport = std::move(report); } - QString getCheckersReport() const { + const QString& getCheckersReport() const { return mCheckersReport; } diff --git a/gui/codeeditor.h b/gui/codeeditor.h index 6e06db9881f..e301f6bf8d7 100644 --- a/gui/codeeditor.h +++ b/gui/codeeditor.h @@ -114,7 +114,7 @@ class CodeEditor : public QPlainTextEdit { mFileName = fileName; } - QString getFileName() const { + const QString& getFileName() const { return mFileName; } diff --git a/gui/printablereport.h b/gui/printablereport.h index 693356a961d..a5a4a975e5b 100644 --- a/gui/printablereport.h +++ b/gui/printablereport.h @@ -62,7 +62,7 @@ class PrintableReport : public Report { /** * @brief Returns the formatted report. */ - QString getFormattedReportText() const; + const QString& getFormattedReportText() const; private: diff --git a/gui/projectfile.h b/gui/projectfile.h index df4d94fbbaa..c2d44ac7d01 100644 --- a/gui/projectfile.h +++ b/gui/projectfile.h @@ -75,15 +75,15 @@ class ProjectFile : public QObject { * @brief Get project root path. * @return project root path. */ - QString getRootPath() const { + const QString& getRootPath() const { return mRootPath; } - QString getBuildDir() const { + const QString& getBuildDir() const { return mBuildDir; } - QString getImportProject() const { + const QString& getImportProject() const { return mImportProject; } @@ -111,7 +111,7 @@ class ProjectFile : public QObject { * @brief Get list of include directories. * @return list of directories. */ - QStringList getIncludeDirs() const { + const QStringList& getIncludeDirs() const { return ProjectFile::fromNativeSeparators(mIncludeDirs); } @@ -119,7 +119,7 @@ class ProjectFile : public QObject { * @brief Get list of defines. * @return list of defines. */ - QStringList getDefines() const { + const QStringList& getDefines() const { return mDefines; } @@ -127,7 +127,7 @@ class ProjectFile : public QObject { * @brief Get list of undefines. * @return list of undefines. */ - QStringList getUndefines() const { + const QStringList& getUndefines() const { return mUndefines; } @@ -135,7 +135,7 @@ class ProjectFile : public QObject { * @brief Get list of paths to check. * @return list of paths. */ - QStringList getCheckPaths() const { + const QStringList& getCheckPaths() const { return ProjectFile::fromNativeSeparators(mPaths); } @@ -143,7 +143,7 @@ class ProjectFile : public QObject { * @brief Get list of paths to exclude from the check. * @return list of paths. */ - QStringList getExcludedPaths() const { + const QStringList& getExcludedPaths() const { return ProjectFile::fromNativeSeparators(mExcludedPaths); } @@ -151,7 +151,7 @@ class ProjectFile : public QObject { * @brief Get list of paths to exclude from the check. * @return list of paths. */ - QStringList getVsConfigurations() const { + const QStringList& getVsConfigurations() const { return mVsConfigurations; } @@ -159,7 +159,7 @@ class ProjectFile : public QObject { * @brief Get list libraries. * @return list of libraries. */ - QStringList getLibraries() const { + const QStringList& getLibraries() const { return mLibraries; } @@ -167,11 +167,11 @@ class ProjectFile : public QObject { * @brief Get platform. * @return Current platform. If it ends with .xml then it is a file. Otherwise it must match one of the return values from @sa cppcheck::Platform::toString() ("win32A", "unix32", ..) */ - QString getPlatform() const { + const QString& getPlatform() const { return mPlatform; } - QString getProjectName() const { + const QString& getProjectName() const { return mProjectName; } @@ -183,7 +183,7 @@ class ProjectFile : public QObject { * @brief Get "raw" suppressions. * @return list of suppressions. */ - QList getSuppressions() const { + const QList& getSuppressions() const { return mSuppressions; } @@ -191,7 +191,7 @@ class ProjectFile : public QObject { * @brief Get list addons. * @return list of addons. */ - QStringList getAddons() const { + const QStringList& getAddons() const { return mAddons; } @@ -224,7 +224,7 @@ class ProjectFile : public QObject { mClangTidy = c; } - QStringList getTags() const { + const QStringList& getTags() const { return mTags; } @@ -248,7 +248,7 @@ class ProjectFile : public QObject { * @brief Get filename for the project file. * @return file name. */ - QString getFilename() const { + const QString& getFilename() const { return mFilename; } @@ -361,7 +361,7 @@ class ProjectFile : public QObject { } /** @brief Get list of coding standards (checked by Cppcheck Premium). */ - QStringList getCodingStandards() const { + const QStringList& getCodingStandards() const { return mCodingStandards; } @@ -408,7 +408,7 @@ class ProjectFile : public QObject { SafeChecks safeChecks; /** Check unknown function return values */ - QStringList getCheckUnknownFunctionReturn() const { + const QStringList& getCheckUnknownFunctionReturn() const { return mCheckUnknownFunctionReturn; } /* diff --git a/gui/projectfiledialog.h b/gui/projectfiledialog.h index fe243e15137..d9edef245f5 100644 --- a/gui/projectfiledialog.h +++ b/gui/projectfiledialog.h @@ -109,7 +109,7 @@ class ProjectFileDialog : public QDialog { * @brief Return suppressions from the dialog control. * @return List of suppressions. */ - QList getSuppressions() const { + const QList& getSuppressions() const { return mSuppressions; } diff --git a/gui/resultstree.h b/gui/resultstree.h index 8d855766082..213bb7fcdaa 100644 --- a/gui/resultstree.h +++ b/gui/resultstree.h @@ -134,7 +134,7 @@ class ResultsTree : public QTreeView { * @return Directory containing source files */ - QString getCheckDirectory(); + const QString& getCheckDirectory(); /** * @brief Check if there are any visible results in view. diff --git a/gui/translationhandler.h b/gui/translationhandler.h index cd298d6ebaf..12ed2d4a6c0 100644 --- a/gui/translationhandler.h +++ b/gui/translationhandler.h @@ -69,7 +69,7 @@ class TranslationHandler : QObject { * @return List of available translations. * */ - QList getTranslations() const { + const QList& getTranslations() const { return mTranslations; } @@ -86,7 +86,7 @@ class TranslationHandler : QObject { * @return ISO 639 language code for current translation. * */ - QString getCurrentLanguage() const; + const QString& getCurrentLanguage() const; /** * @brief Get translation suggestion for the system. diff --git a/lib/templatesimplifier.h b/lib/templatesimplifier.h index e8e77507e5e..d3a7bcd6cf6 100644 --- a/lib/templatesimplifier.h +++ b/lib/templatesimplifier.h @@ -49,7 +49,7 @@ class CPPCHECKLIB TemplateSimplifier { public: explicit TemplateSimplifier(Tokenizer &tokenizer); - std::string dump() const { + const std::string& dump() const { return mDump; } From 5fd354905f67c776cf731efb09b2bdff31d73438 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 8 Apr 2024 21:01:20 +0200 Subject: [PATCH 09/23] Makefile --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index a720612033f..6751b73cb17 100644 --- a/Makefile +++ b/Makefile @@ -498,7 +498,7 @@ $(libcppdir)/checkboost.o: lib/checkboost.cpp lib/check.h lib/checkboost.h lib/c $(libcppdir)/checkbufferoverrun.o: lib/checkbufferoverrun.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/astutils.h lib/check.h lib/checkbufferoverrun.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.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/valueflow.h lib/vfvalue.h lib/xml.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkbufferoverrun.cpp -$(libcppdir)/checkclass.o: lib/checkclass.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/astutils.h lib/check.h lib/checkclass.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.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 lib/xml.h +$(libcppdir)/checkclass.o: lib/checkclass.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/astutils.h lib/check.h lib/checkclass.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.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/valueflow.h lib/vfvalue.h lib/xml.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkclass.cpp $(libcppdir)/checkcondition.o: lib/checkcondition.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checkcondition.h lib/checkother.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.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 From b953c0591f24cf5b83a9e983846838ce1bd8da82 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 8 Apr 2024 21:01:30 +0200 Subject: [PATCH 10/23] rename --- lib/checkclass.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 6317aca9431..73883116ce4 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -3314,12 +3314,12 @@ static const Variable* getSingleReturnVar(const Scope* scope) { return start->astOperand1()->variable(); } -void CheckClass::checkReturnReference() +void CheckClass::checkReturnByReference() { - if (!mSettings->severity.isEnabled(Severity::style) && !mSettings->isPremiumEnabled("returnReference")) + if (!mSettings->severity.isEnabled(Severity::performance) && !mSettings->isPremiumEnabled("returnByReference")) return; - logChecker("CheckClass::checkReturnReference"); // style + logChecker("CheckClass::checkReturnByReference"); // performance for (const Scope* classScope : mSymbolDatabase->classAndStructScopes) { for (const Function& func : classScope->functionList) { From d8b59dd886162e6cb19e5466031fc5c7adfcc8e0 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 8 Apr 2024 21:13:30 +0200 Subject: [PATCH 11/23] Add test --- lib/checkclass.h | 4 ++-- test/testclass.cpp | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/lib/checkclass.h b/lib/checkclass.h index 4808944f214..bc2a9d5116d 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -91,7 +91,7 @@ class CPPCHECKLIB CheckClass : public Check { checkClass.checkCopyCtorAndEqOperator(); checkClass.checkOverride(); checkClass.checkUselessOverride(); - checkClass.checkReturnReference(); + checkClass.checkReturnByReference(); checkClass.checkThisUseAfterFree(); checkClass.checkUnsafeClassRefMember(); } @@ -159,7 +159,7 @@ class CPPCHECKLIB CheckClass : public Check { void checkUselessOverride(); /** @brief Check that large members are returned by reference from getter function */ - void checkReturnReference(); + void checkReturnByReference(); /** @brief When "self pointer" is destroyed, 'this' might become invalid. */ void checkThisUseAfterFree(); diff --git a/test/testclass.cpp b/test/testclass.cpp index 7b628149e63..465caef5d0c 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -243,6 +243,8 @@ class TestClass : public TestFixture { TEST_CASE(ctuOneDefinitionRule); TEST_CASE(testGetFileInfo); + + TEST_CASE(returnByReference); } #define checkCopyCtorAndEqOperator(code) checkCopyCtorAndEqOperator_(code, __FILE__, __LINE__) @@ -8959,6 +8961,36 @@ class TestClass : public TestFixture { getFileInfo("struct sometype { sometype(); }; sometype::sometype() = delete;"); // don't crash } +#define checkReturnByReference(...) checkReturnByReference_(__FILE__, __LINE__, __VA_ARGS__) + void checkReturnByReference_(const char* file, int line, const char code[]) { + const Settings settings = settingsBuilder().severity(Severity::performance).library("std.cfg").build(); + + std::vector files(1, "test.cpp"); + Tokenizer tokenizer(settings, this); + PreprocessorHelper::preprocess(code, files, tokenizer); + + ASSERT_LOC(tokenizer.simplifyTokens1(""), file, line); + + // Check.. + CheckClass checkClass(&tokenizer, &settings, this); + (checkClass.checkReturnByReference)(); + } + + void returnByReference() { + checkReturnByReference("struct T { int a[10]; };" // #12546 + "struct S {" + " T t;" + " int i;" + " std::string s;" + " T getT() const { return t; }" + " int getI() const { return i; }" + " std::string getS() const { return s; }" + "};"); + ASSERT_EQUALS("[test.cpp:1]: (style) Function 'getT()' should return member 't' by const reference.\n" + "[test.cpp:1]: (style) Function 'getS()' should return member 's' by const reference.\n", + errout_str()); + } + }; REGISTER_TEST(TestClass) From 192974e571f40a3686a1b18ffa4676772c3c0699 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 8 Apr 2024 21:26:38 +0200 Subject: [PATCH 12/23] Undo --- gui/projectfile.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gui/projectfile.h b/gui/projectfile.h index c2d44ac7d01..0b5a576ffc1 100644 --- a/gui/projectfile.h +++ b/gui/projectfile.h @@ -135,7 +135,7 @@ class ProjectFile : public QObject { * @brief Get list of paths to check. * @return list of paths. */ - const QStringList& getCheckPaths() const { + QStringList getCheckPaths() const { return ProjectFile::fromNativeSeparators(mPaths); } @@ -143,7 +143,7 @@ class ProjectFile : public QObject { * @brief Get list of paths to exclude from the check. * @return list of paths. */ - const QStringList& getExcludedPaths() const { + QStringList getExcludedPaths() const { return ProjectFile::fromNativeSeparators(mExcludedPaths); } From 0040ca4cafba8ecbc5d8c91ac91421ab4b005ba5 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 8 Apr 2024 21:27:14 +0200 Subject: [PATCH 13/23] Undo --- gui/projectfile.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gui/projectfile.h b/gui/projectfile.h index 0b5a576ffc1..102b2162ce7 100644 --- a/gui/projectfile.h +++ b/gui/projectfile.h @@ -111,7 +111,7 @@ class ProjectFile : public QObject { * @brief Get list of include directories. * @return list of directories. */ - const QStringList& getIncludeDirs() const { + QStringList getIncludeDirs() const { return ProjectFile::fromNativeSeparators(mIncludeDirs); } From b516f7ae0df4425b1e3cc14d40059a66bc3457e3 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 8 Apr 2024 21:28:47 +0200 Subject: [PATCH 14/23] Fix --- gui/translationhandler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gui/translationhandler.cpp b/gui/translationhandler.cpp index 0caa3dcb1c5..173da1aca94 100644 --- a/gui/translationhandler.cpp +++ b/gui/translationhandler.cpp @@ -142,7 +142,7 @@ bool TranslationHandler::setLanguage(const QString &code) return true; } -QString TranslationHandler::getCurrentLanguage() const +const QString& TranslationHandler::getCurrentLanguage() const { return mCurrentLanguage; } From 35692950791de26ea49722d579497e3780ca2bf7 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 8 Apr 2024 21:42:52 +0200 Subject: [PATCH 15/23] Fix --- gui/printablereport.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gui/printablereport.cpp b/gui/printablereport.cpp index d9272632f45..b759c513628 100644 --- a/gui/printablereport.cpp +++ b/gui/printablereport.cpp @@ -52,7 +52,7 @@ void PrintableReport::writeError(const ErrorItem &error) mFormattedReport += "\n"; } -QString PrintableReport::getFormattedReportText() const +const QString& PrintableReport::getFormattedReportText() const { return mFormattedReport; } From c98419307589ef9e625851dbeddad943f1f89e67 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 8 Apr 2024 21:55:25 +0200 Subject: [PATCH 16/23] Fix --- gui/resultstree.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gui/resultstree.cpp b/gui/resultstree.cpp index a14a7a15351..eb61a08cb87 100644 --- a/gui/resultstree.cpp +++ b/gui/resultstree.cpp @@ -1337,7 +1337,7 @@ void ResultsTree::setCheckDirectory(const QString &dir) } -QString ResultsTree::getCheckDirectory() +const QString& ResultsTree::getCheckDirectory() { return mCheckPath; } From d0a23e69b4c770898c4925fbb8e0d2ef3c3d0c8f Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 8 Apr 2024 22:12:10 +0200 Subject: [PATCH 17/23] releasenotes --- releasenotes.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/releasenotes.txt b/releasenotes.txt index 56c7500bc41..bf159e7a5e3 100644 --- a/releasenotes.txt +++ b/releasenotes.txt @@ -2,6 +2,7 @@ Release Notes for Cppcheck 2.14 New checks: eraseIteratorOutOfBounds: warns when erase() is called on an iterator that is out of bounds +returnByReference: warns when a large class member is returned by value from a getter function Improved checking: - From 87aae2b67e23c8ffdd7577e25f277d2a2219c045 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 8 Apr 2024 22:13:18 +0200 Subject: [PATCH 18/23] Makefile --- oss-fuzz/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oss-fuzz/Makefile b/oss-fuzz/Makefile index 72ce6c43581..88c76f4e134 100644 --- a/oss-fuzz/Makefile +++ b/oss-fuzz/Makefile @@ -178,7 +178,7 @@ $(libcppdir)/checkboost.o: ../lib/checkboost.cpp ../lib/check.h ../lib/checkboos $(libcppdir)/checkbufferoverrun.o: ../lib/checkbufferoverrun.cpp ../externals/tinyxml2/tinyxml2.h ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkbufferoverrun.h ../lib/color.h ../lib/config.h ../lib/ctu.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.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/valueflow.h ../lib/vfvalue.h ../lib/xml.h $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkbufferoverrun.cpp -$(libcppdir)/checkclass.o: ../lib/checkclass.cpp ../externals/tinyxml2/tinyxml2.h ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkclass.h ../lib/color.h ../lib/config.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.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 ../lib/xml.h +$(libcppdir)/checkclass.o: ../lib/checkclass.cpp ../externals/tinyxml2/tinyxml2.h ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkclass.h ../lib/color.h ../lib/config.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.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/valueflow.h ../lib/vfvalue.h ../lib/xml.h $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkclass.cpp $(libcppdir)/checkcondition.o: ../lib/checkcondition.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkcondition.h ../lib/checkother.h ../lib/config.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.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 From 2dde48675dfd7e12d7f1b85ce7d5a287abaa057e Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 8 Apr 2024 22:18:34 +0200 Subject: [PATCH 19/23] clang-tidy --- gui/mainwindow.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gui/mainwindow.cpp b/gui/mainwindow.cpp index b42a81a232c..5a80a9d450e 100644 --- a/gui/mainwindow.cpp +++ b/gui/mainwindow.cpp @@ -1751,7 +1751,7 @@ void MainWindow::analyzeProject(const ProjectFile *projectFile, const bool check Settings::terminate(false); QFileInfo inf(projectFile->getFilename()); - const QString rootpath = projectFile->getRootPath(); + const QString& rootpath = projectFile->getRootPath(); QDir::setCurrent(inf.absolutePath()); From b7e6af603620aaf593bbfb8ddad1d9e88dea73af Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 8 Apr 2024 22:54:55 +0200 Subject: [PATCH 20/23] performance, simplify --- lib/checkclass.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 73883116ce4..25314d6be4f 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -3305,6 +3305,8 @@ void CheckClass::checkUselessOverride() } static const Variable* getSingleReturnVar(const Scope* scope) { + if (!scope) + return nullptr; const Token* const start = scope->bodyStart->next(); const Token* const end = Token::findsimplematch(start, ";", 1, scope->bodyEnd); if (!end || end->next() != scope->bodyEnd) @@ -3323,8 +3325,6 @@ void CheckClass::checkReturnByReference() for (const Scope* classScope : mSymbolDatabase->classAndStructScopes) { for (const Function& func : classScope->functionList) { - if (!func.functionScope) - continue; if (Function::returnsPointer(&func) || Function::returnsReference(&func) || Function::returnsStandardType(&func)) continue; if (const Variable* var = getSingleReturnVar(func.functionScope)) { @@ -3349,7 +3349,7 @@ void CheckClass::returnByReferenceError(const Function* func, const Variable* va { const Token* tok = func ? func->tokenDef : nullptr; const std::string message = "Function '" + (func ? func->name() : "") + "()' should return member '" + (var ? var->name() : "") + "' by const reference."; - reportError(tok, Severity::style, "returnByReference", message); + reportError(tok, Severity::performance, "returnByReference", message); } void CheckClass::checkThisUseAfterFree() From 6b5a8935b9217ed53b66d75fcd922afc3ca00847 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 8 Apr 2024 23:10:04 +0200 Subject: [PATCH 21/23] Fix --- test/testclass.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/testclass.cpp b/test/testclass.cpp index 465caef5d0c..e6fb80f002e 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -8986,8 +8986,8 @@ class TestClass : public TestFixture { " int getI() const { return i; }" " std::string getS() const { return s; }" "};"); - ASSERT_EQUALS("[test.cpp:1]: (style) Function 'getT()' should return member 't' by const reference.\n" - "[test.cpp:1]: (style) Function 'getS()' should return member 's' by const reference.\n", + ASSERT_EQUALS("[test.cpp:1]: (performance) Function 'getT()' should return member 't' by const reference.\n" + "[test.cpp:1]: (performance) Function 'getS()' should return member 's' by const reference.\n", errout_str()); } From 1ad38ac1781041f57299afe0899b3083f67f1544 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Mon, 8 Apr 2024 23:29:48 +0200 Subject: [PATCH 22/23] nullptr check --- lib/checkclass.cpp | 2 +- test/testclass.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 25314d6be4f..66ab3578560 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -3311,7 +3311,7 @@ static const Variable* getSingleReturnVar(const Scope* scope) { const Token* const end = Token::findsimplematch(start, ";", 1, scope->bodyEnd); if (!end || end->next() != scope->bodyEnd) return nullptr; - if (start->str() != "return") + if (!start->astOperand1() || start->str() != "return") return nullptr; return start->astOperand1()->variable(); } diff --git a/test/testclass.cpp b/test/testclass.cpp index e6fb80f002e..dc39d59b76f 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -8985,6 +8985,7 @@ class TestClass : public TestFixture { " T getT() const { return t; }" " int getI() const { return i; }" " std::string getS() const { return s; }" + " unknown_t f() { return; }" "};"); ASSERT_EQUALS("[test.cpp:1]: (performance) Function 'getT()' should return member 't' by const reference.\n" "[test.cpp:1]: (performance) Function 'getS()' should return member 's' by const reference.\n", From 5b3a5f0713cb5924d2025b7463f4d4690cc271e2 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Wed, 10 Apr 2024 16:58:02 +0200 Subject: [PATCH 23/23] dummy names --- lib/checkclass.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 66ab3578560..1a42f8ab820 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -3348,7 +3348,7 @@ void CheckClass::checkReturnByReference() void CheckClass::returnByReferenceError(const Function* func, const Variable* var) { const Token* tok = func ? func->tokenDef : nullptr; - const std::string message = "Function '" + (func ? func->name() : "") + "()' should return member '" + (var ? var->name() : "") + "' by const reference."; + const std::string message = "Function '" + (func ? func->name() : "func") + "()' should return member '" + (var ? var->name() : "var") + "' by const reference."; reportError(tok, Severity::performance, "returnByReference", message); }