From 365e7136cdd1797dec21d95611a45ed32c9750fb Mon Sep 17 00:00:00 2001 From: wmayer Date: Wed, 15 Nov 2023 13:36:51 +0100 Subject: [PATCH] fix bugprone-* * bugprone-throw-keyword-missing * bugprone-unhandled-self-assignment * bugprone-suspicious-string-compare * bugprone-reserved-identifier * bugprone-narrowing-conversions * bugprone-macro-parentheses * bugprone-implicit-widening-of-multiplication-result * bugprone-exception-escape * bugprone-copy-constructor-init --- .clang-tidy | 2 +- src/Base/Console.h | 2 ++ src/Base/ConsoleObserver.h | 15 ++++++++++----- src/Base/Converter.h | 34 +++++++++++++++++----------------- src/Base/Exception.h | 2 ++ src/Base/ExceptionFactory.cpp | 2 +- src/Base/FileInfo.cpp | 8 ++++---- src/Base/FileInfo.h | 4 ++-- src/Base/GeometryPyCXX.h | 2 +- src/Base/Interpreter.h | 2 ++ src/Base/Matrix.cpp | 6 +++--- src/Base/Matrix.h | 4 ++++ src/Base/Observer.h | 28 ++++++++++++++-------------- src/Base/Parameter.cpp | 4 ++-- src/Base/Parameter.h | 6 +++--- src/Base/ParameterPy.cpp | 12 ++++++++---- src/Base/Precision.h | 4 ++-- src/Base/Rotation.cpp | 8 +++++++- src/Base/Rotation.h | 2 +- src/Base/RotationPyImp.cpp | 2 +- src/Base/Stream.cpp | 1 + src/Base/Tools2D.cpp | 16 ++++++++-------- src/Base/Vector3D.h | 6 +++--- 23 files changed, 99 insertions(+), 73 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index b800bcb21c..38f21b8bfc 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -152,7 +152,7 @@ CheckOptions: - key: google-readability-braces-around-statements.ShortStatementLines value: '1' - key: bugprone-reserved-identifier.AllowedIdentifiers - value: '' + value: '_object;_Precision' - key: cppcoreguidelines-pro-type-member-init.IgnoreArrays value: 'false' - key: readability-else-after-return.WarnOnUnfixable diff --git a/src/Base/Console.h b/src/Base/Console.h index 79511acb14..4cc7a53f2a 100644 --- a/src/Base/Console.h +++ b/src/Base/Console.h @@ -341,6 +341,7 @@ using PyMethodDef = struct PyMethodDef; * */ +// NOLINTBEGIN(bugprone-reserved-identifier,bugprone-macro-parentheses,cppcoreguidelines-macro-usage) #define FC_LOGLEVEL_DEFAULT -1 #define FC_LOGLEVEL_ERR 0 #define FC_LOGLEVEL_WARN 1 @@ -479,6 +480,7 @@ using PyMethodDef = struct PyMethodDef; } while (0) #endif // FC_LOG_NO_TIMING +// NOLINTEND(bugprone-reserved-identifier,bugprone-macro-parentheses,cppcoreguidelines-macro-usage) // TODO: Get rid of this typedef using ConsoleMsgFlags = unsigned int; diff --git a/src/Base/ConsoleObserver.h b/src/Base/ConsoleObserver.h index 3c69faccfd..e3c23a04c9 100644 --- a/src/Base/ConsoleObserver.h +++ b/src/Base/ConsoleObserver.h @@ -132,14 +132,19 @@ ILoggerBlocker::ILoggerBlocker(const char* co, ConsoleMsgFlags msgTypes) ILoggerBlocker::~ILoggerBlocker() { + try { #ifdef FC_DEBUG - auto debug = Console().SetEnabledMsgType(conObs, msgTypesBlocked, true); - if (debug != msgTypesBlocked) { - Console().Warning("Enabled message types have been changed while ILoggerBlocker was set\n"); - } + auto debug = Console().SetEnabledMsgType(conObs, msgTypesBlocked, true); + if (debug != msgTypesBlocked) { + Console().Warning( + "Enabled message types have been changed while ILoggerBlocker was set\n"); + } #else - Console().SetEnabledMsgType(conObs, msgTypesBlocked, true); + Console().SetEnabledMsgType(conObs, msgTypesBlocked, true); #endif + } + catch (...) { + } } class BaseExport RedirectStdOutput: public std::streambuf diff --git a/src/Base/Converter.h b/src/Base/Converter.h index 69e18e6412..a524fd67a4 100644 --- a/src/Base/Converter.h +++ b/src/Base/Converter.h @@ -94,36 +94,36 @@ private: }; // type with three floats -template -_Vec make_vec(const std::tuple&& ft) +template +Vec make_vec(const std::tuple&& ft) { - using traits_type = vec_traits<_Vec>; + using traits_type = vec_traits; using float_traits_type = typename traits_type::float_type; - return _Vec(float_traits_type(std::get<0>(ft)), - float_traits_type(std::get<1>(ft)), - float_traits_type(std::get<2>(ft))); + return Vec(float_traits_type(std::get<0>(ft)), + float_traits_type(std::get<1>(ft)), + float_traits_type(std::get<2>(ft))); } // type with four floats -template -_Vec make_vec(const std::tuple&& ft) +template +Vec make_vec(const std::tuple&& ft) { - using traits_type = vec_traits<_Vec>; + using traits_type = vec_traits; using float_traits_type = typename traits_type::float_type; - return _Vec(float_traits_type(std::get<0>(ft)), - float_traits_type(std::get<1>(ft)), - float_traits_type(std::get<2>(ft)), - float_traits_type(std::get<3>(ft))); + return Vec(float_traits_type(std::get<0>(ft)), + float_traits_type(std::get<1>(ft)), + float_traits_type(std::get<2>(ft)), + float_traits_type(std::get<3>(ft))); } -template -inline _Vec1 convertTo(const _Vec2& vec) +template +inline Vec1 convertTo(const Vec2& vec) { - using traits_type = vec_traits<_Vec2>; + using traits_type = vec_traits; using float_type = typename traits_type::float_type; traits_type tt(vec); auto tuple = tt.get(); - return make_vec<_Vec1, float_type>(std::move(tuple)); + return make_vec(std::move(tuple)); } } // namespace Base diff --git a/src/Base/Exception.h b/src/Base/Exception.h index 8da3ec52ab..b5663fcca8 100644 --- a/src/Base/Exception.h +++ b/src/Base/Exception.h @@ -47,6 +47,7 @@ using PyObject = struct _object; /// have provided one) at that time it gets translated (e.g. in the UI before showing the message of /// the exception). +// NOLINTBEGIN #ifdef _MSC_VER #define THROW(exception) \ @@ -185,6 +186,7 @@ using PyObject = struct _object; ss << _msg; \ THROWM(_exception, ss.str().c_str()); \ } while (0) +// NOLINTEND namespace Base { diff --git a/src/Base/ExceptionFactory.cpp b/src/Base/ExceptionFactory.cpp index 8fe1141c91..d3816fef6d 100644 --- a/src/Base/ExceptionFactory.cpp +++ b/src/Base/ExceptionFactory.cpp @@ -33,7 +33,7 @@ ExceptionFactory* ExceptionFactory::_pcSingleton = nullptr; // NOLINT ExceptionFactory& ExceptionFactory::Instance() { if (!_pcSingleton) { - _pcSingleton = new ExceptionFactory; + _pcSingleton = new ExceptionFactory; // NOLINT } return *_pcSingleton; } diff --git a/src/Base/FileInfo.cpp b/src/Base/FileInfo.cpp index 5ed4170341..0dea57a97e 100644 --- a/src/Base/FileInfo.cpp +++ b/src/Base/FileInfo.cpp @@ -93,14 +93,14 @@ std::wstring ConvertToWideString(const std::string& string) // FileInfo -FileInfo::FileInfo(const char* _FileName) +FileInfo::FileInfo(const char* fileName) { - setFile(_FileName); + setFile(fileName); } -FileInfo::FileInfo(const std::string& _FileName) +FileInfo::FileInfo(const std::string& fileName) { - setFile(_FileName.c_str()); + setFile(fileName.c_str()); } const std::string& FileInfo::getTempPath() diff --git a/src/Base/FileInfo.h b/src/Base/FileInfo.h index 6a9a41be5a..0f120386a6 100644 --- a/src/Base/FileInfo.h +++ b/src/Base/FileInfo.h @@ -59,8 +59,8 @@ public: }; /// Construction - FileInfo(const char* _FileName = ""); - FileInfo(const std::string& _FileName); + FileInfo(const char* fileName = ""); + FileInfo(const std::string& fileName); /// Set a new file name void setFile(const char* name); /// Set a new file name diff --git a/src/Base/GeometryPyCXX.h b/src/Base/GeometryPyCXX.h index aac57cf686..c9b54ac825 100644 --- a/src/Base/GeometryPyCXX.h +++ b/src/Base/GeometryPyCXX.h @@ -152,7 +152,7 @@ public: } Vector(const Vector& ob) - : Object(*ob) + : Object(ob) { validate(); } diff --git a/src/Base/Interpreter.h b/src/Base/Interpreter.h index cad7f569b5..14d897baed 100644 --- a/src/Base/Interpreter.h +++ b/src/Base/Interpreter.h @@ -47,6 +47,7 @@ #include "Exception.h" +// NOLINTBEGIN /** Helper macro to obtain callable from an object * * @param _pyobj: PyObject pointer @@ -79,6 +80,7 @@ if (PyObject_HasAttrString(_pyobj, _name)) \ _var = Py::asObject(PyObject_GetAttrString(_pyobj, _name)); \ } while (0) +// NOLINTEND namespace Base diff --git a/src/Base/Matrix.cpp b/src/Base/Matrix.cpp index 18a2468c70..cd3c3c80c2 100644 --- a/src/Base/Matrix.cpp +++ b/src/Base/Matrix.cpp @@ -1026,9 +1026,9 @@ std::array Matrix4D::decompose() const scaleMatrix.dMtrx4D[1][1] = yScale; scaleMatrix.dMtrx4D[2][2] = zScale; // The remaining shear - residualMatrix.scale(xScale ? 1.0 / xScale : 1.0, - yScale ? 1.0 / yScale : 1.0, - zScale ? 1.0 / zScale : 1.0); + residualMatrix.scale(xScale != 0 ? 1.0 / xScale : 1.0, + yScale != 0 ? 1.0 / yScale : 1.0, + zScale != 0 ? 1.0 / zScale : 1.0); // Restore trace in shear matrix residualMatrix.setDiagonal(Vector3d(1.0, 1.0, 1.0)); // Remove values close to zero diff --git a/src/Base/Matrix.h b/src/Base/Matrix.h index c802af8dbd..d8487fe7d2 100644 --- a/src/Base/Matrix.h +++ b/src/Base/Matrix.h @@ -320,6 +320,10 @@ inline Matrix4D Matrix4D::operator*(const Matrix4D& mat) const inline Matrix4D& Matrix4D::operator=(const Matrix4D& mat) { + if (this == &mat) { + return *this; + } + for (int iz = 0; iz < 4; iz++) { for (int is = 0; is < 4; is++) { dMtrx4D[iz][is] = mat.dMtrx4D[iz][is]; diff --git a/src/Base/Observer.h b/src/Base/Observer.h index 92220939db..0f7e7932c9 100644 --- a/src/Base/Observer.h +++ b/src/Base/Observer.h @@ -46,7 +46,7 @@ class Subject; * Attach itself to the observed object. * @see FCSubject */ -template +template class Observer { public: @@ -69,14 +69,14 @@ public: * @param rcReason * \todo undocumented parameter 2 */ - virtual void OnChange(Subject<_MessageType>& rCaller, _MessageType rcReason) = 0; + virtual void OnChange(Subject& rCaller, MsgType rcReason) = 0; /** * This method need to be reimplemented from the concrete Observer * and get called by the observed class * @param rCaller a reference to the calling object */ - virtual void OnDestroy(Subject<_MessageType>& rCaller) + virtual void OnDestroy(Subject& rCaller) { (void)rCaller; } @@ -99,13 +99,13 @@ public: * Attach itself to the observed object. * @see FCObserver */ -template +template class Subject { public: - using ObserverType = Observer<_MessageType>; - using MessageType = _MessageType; - using SubjectType = Subject<_MessageType>; + using ObserverType = Observer; + using MessageType = MsgType; + using SubjectType = Subject; /** * A constructor. @@ -131,7 +131,7 @@ public: * @param ToObserv A pointer to a concrete Observer * @see Notify */ - void Attach(Observer<_MessageType>* ToObserv) + void Attach(Observer* ToObserv) { #ifdef FC_DEBUG size_t count = _ObserverSet.size(); @@ -152,7 +152,7 @@ public: * @param ToObserv A pointer to a concrete Observer * @see Notify */ - void Detach(Observer<_MessageType>* ToObserv) + void Detach(Observer* ToObserv) { #ifdef FC_DEBUG size_t count = _ObserverSet.size(); @@ -173,9 +173,9 @@ public: * Oberserver and Subject. * @see Notify */ - void Notify(_MessageType rcReason) + void Notify(MsgType rcReason) { - for (typename std::set*>::iterator Iter = _ObserverSet.begin(); + for (typename std::set*>::iterator Iter = _ObserverSet.begin(); Iter != _ObserverSet.end(); ++Iter) { try { @@ -202,10 +202,10 @@ public: * Get a observer by name if the observer reimplements the Name() mthode. * @see Observer */ - Observer<_MessageType>* Get(const char* Name) + Observer* Get(const char* Name) { const char* OName = nullptr; - for (typename std::set*>::iterator Iter = _ObserverSet.begin(); + for (typename std::set*>::iterator Iter = _ObserverSet.begin(); Iter != _ObserverSet.end(); ++Iter) { OName = (*Iter)->Name(); // get the name @@ -228,7 +228,7 @@ public: protected: /// Vector of attached observers - std::set*> _ObserverSet; + std::set*> _ObserverSet; }; // Workaround for MSVC diff --git a/src/Base/Parameter.cpp b/src/Base/Parameter.cpp index d67aedd2ce..23bb112cd1 100644 --- a/src/Base/Parameter.cpp +++ b/src/Base/Parameter.cpp @@ -754,7 +754,7 @@ std::vector ParameterGrp::GetBools(const char* sFilter) const Name = StrX(pcTemp->getAttribute(XStr("Name").unicodeForm())).c_str(); // check on filter condition if (!sFilter || Name.find(sFilter) != std::string::npos) { - if (strcmp(StrX(pcTemp->getAttribute(XStr("Value").unicodeForm())).c_str(), "1")) { + if (strcmp(StrX(pcTemp->getAttribute(XStr("Value").unicodeForm())).c_str(), "1") != 0) { vrValues.push_back(false); } else { @@ -781,7 +781,7 @@ std::vector> ParameterGrp::GetBoolMap(const char* s Name = StrX(pcTemp->getAttribute(XStr("Name").unicodeForm())).c_str(); // check on filter condition if (!sFilter || Name.find(sFilter) != std::string::npos) { - if (strcmp(StrX(pcTemp->getAttribute(XStr("Value").unicodeForm())).c_str(), "1")) { + if (strcmp(StrX(pcTemp->getAttribute(XStr("Value").unicodeForm())).c_str(), "1") != 0) { vrValues.emplace_back(Name, false); } else { diff --git a/src/Base/Parameter.h b/src/Base/Parameter.h index 4a68318460..a2afb2efed 100644 --- a/src/Base/Parameter.h +++ b/src/Base/Parameter.h @@ -29,8 +29,8 @@ * 3rd party Xerces-C++ XML parser is used to parse and write the XML. */ -#ifndef BASE__PARAMETER_H -#define BASE__PARAMETER_H +#ifndef BASE_PARAMETER_H +#define BASE_PARAMETER_H // Python stuff using PyObject = struct _object; @@ -454,4 +454,4 @@ private: BaseExport PyObject* GetPyObject(const Base::Reference& hcParamGrp); -#endif // BASE__PARAMETER_H +#endif // BASE_PARAMETER_H diff --git a/src/Base/ParameterPy.cpp b/src/Base/ParameterPy.cpp index d04cf934d7..191b71e1fe 100644 --- a/src/Base/ParameterPy.cpp +++ b/src/Base/ParameterPy.cpp @@ -251,11 +251,15 @@ ParameterGrpPy::ParameterGrpPy(const Base::Reference& rcParamGrp) ParameterGrpPy::~ParameterGrpPy() { - for (ParameterGrpObserver* obs : _observers) { - if (!obs->_target) { - _cParamGrp->Detach(obs); + try { + for (ParameterGrpObserver* obs : _observers) { + if (!obs->_target) { + _cParamGrp->Detach(obs); + } + delete obs; } - delete obs; + } + catch (...) { } } diff --git a/src/Base/Precision.h b/src/Base/Precision.h index a65ff5f7fc..8c7fbee348 100644 --- a/src/Base/Precision.h +++ b/src/Base/Precision.h @@ -187,7 +187,7 @@ public: * \param R * \return */ - static double IsInfinite(const double R) + static bool IsInfinite(const double R) { return std::fabs(R) >= (0.5 * Precision::Infinite()); } @@ -198,7 +198,7 @@ public: * \param R * \return */ - static double IsPositiveInfinite(const double R) + static bool IsPositiveInfinite(const double R) { return R >= (0.5 * Precision::Infinite()); } diff --git a/src/Base/Rotation.cpp b/src/Base/Rotation.cpp index 41a7852728..abb609141c 100644 --- a/src/Base/Rotation.cpp +++ b/src/Base/Rotation.cpp @@ -95,8 +95,12 @@ Rotation::Rotation(const Rotation& rot) this->_angle = rot._angle; } -void Rotation::operator=(const Rotation& rot) +Rotation& Rotation::operator=(const Rotation& rot) { + if (this == &rot) { + return *this; + } + this->quat[0] = rot.quat[0]; this->quat[1] = rot.quat[1]; this->quat[2] = rot.quat[2]; @@ -106,6 +110,8 @@ void Rotation::operator=(const Rotation& rot) this->_axis[1] = rot._axis[1]; this->_axis[2] = rot._axis[2]; this->_angle = rot._angle; + + return *this; } const double* Rotation::getValue() const diff --git a/src/Base/Rotation.h b/src/Base/Rotation.h index 7542c055d6..8bd8e08d17 100644 --- a/src/Base/Rotation.h +++ b/src/Base/Rotation.h @@ -141,7 +141,7 @@ public: { return quat[usIndex]; } - void operator=(const Rotation&); + Rotation& operator=(const Rotation&); Rotation& multRight(const Base::Rotation& q); Rotation& multLeft(const Base::Rotation& q); diff --git a/src/Base/RotationPyImp.cpp b/src/Base/RotationPyImp.cpp index d73555d646..f364d34bf5 100644 --- a/src/Base/RotationPyImp.cpp +++ b/src/Base/RotationPyImp.cpp @@ -674,7 +674,7 @@ PyObject* RotationPy::number_power_handler(PyObject* self, PyObject* other, PyOb double rfAngle {}; a.getRawValue(axis, rfAngle); - rfAngle *= b; + rfAngle *= double(b); a.setValue(axis, rfAngle); return new RotationPy(a); diff --git a/src/Base/Stream.cpp b/src/Base/Stream.cpp index b3121ccfee..57d3efe630 100644 --- a/src/Base/Stream.cpp +++ b/src/Base/Stream.cpp @@ -602,6 +602,7 @@ PyStreambuf::int_type PyStreambuf::underflow() return traits_type::eof(); } + // Check: bugprone-not-null-terminated-result std::memcpy(start, c.data(), c.size()); } catch (Py::Exception& e) { diff --git a/src/Base/Tools2D.cpp b/src/Base/Tools2D.cpp index 8e311a71bf..a9859eb63a 100644 --- a/src/Base/Tools2D.cpp +++ b/src/Base/Tools2D.cpp @@ -258,18 +258,18 @@ BoundBox2d Polygon2d::CalcBoundBox() const return clBB; } -static short _CalcTorsion(const double* pfLine, double fX, double fY) +static short CalcTorsion(const std::array& pfLine, double fX, double fY) { - std::array sQuad; + std::array sQuad {}; double fResX = 0.0; // Classification of both polygon points into quadrants - for (int i = 0; i < 2; i++) { - if (pfLine[i * 2] <= fX) { - sQuad[i] = (pfLine[i * 2 + 1] > fY) ? 0 : 3; + for (std::size_t i = 0; i < 2; i++) { + if (pfLine.at(i * 2) <= fX) { + sQuad[i] = (pfLine.at(i * 2 + 1) > fY) ? 0 : 3; } else { - sQuad[i] = (pfLine[i * 2 + 1] > fY) ? 1 : 2; + sQuad[i] = (pfLine.at(i * 2 + 1) > fY) ? 1 : 2; } } @@ -302,7 +302,7 @@ bool Polygon2d::Contains(const Vector2d& rclV) const // Using the number of turns method, determines // whether a point is contained within a polygon. // The sum of all turns indicates whether yes or no. - double pfTmp[4]; + std::array pfTmp; unsigned long i = 0; short sTorsion = 0; @@ -330,7 +330,7 @@ bool Polygon2d::Contains(const Vector2d& rclV) const } // Carry out a cut test and calculate the turn counter - sTorsion += _CalcTorsion(pfTmp, rclV.x, rclV.y); + sTorsion += CalcTorsion(pfTmp, rclV.x, rclV.y); } // Evaluate turn counter diff --git a/src/Base/Vector3D.h b/src/Base/Vector3D.h index c58817bf7c..e0c14dab98 100644 --- a/src/Base/Vector3D.h +++ b/src/Base/Vector3D.h @@ -276,10 +276,10 @@ inline Vector3<_Precision> operator*(_Precision fFac, const Vector3<_Precision>& return Vector3<_Precision>(rcVct.x * fFac, rcVct.y * fFac, rcVct.z * fFac); } -template -inline Vector3<_Pr1> toVector(const Vector3<_Pr2>& v) +template +inline Vector3 toVector(const Vector3& v) { - return Vector3<_Pr1>(static_cast<_Pr1>(v.x), static_cast<_Pr1>(v.y), static_cast<_Pr1>(v.z)); + return Vector3(static_cast(v.x), static_cast(v.y), static_cast(v.z)); } using Vector3f = Vector3;