From 411b7573cc0b874d452e4d49c1a6bc7c3ae698a1 Mon Sep 17 00:00:00 2001 From: Ladislav Michl Date: Wed, 29 Mar 2023 15:49:45 +0200 Subject: [PATCH 1/3] Base: Use std::recursive_mutex --- src/Base/CMakeLists.txt | 2 -- src/Base/Mutex.cpp | 36 ------------------------------------ src/Base/Mutex.h | 39 --------------------------------------- src/Base/PreCompiled.h | 3 +-- src/Base/Sequencer.cpp | 31 ++++++++++++++----------------- 5 files changed, 15 insertions(+), 96 deletions(-) delete mode 100644 src/Base/Mutex.cpp delete mode 100644 src/Base/Mutex.h diff --git a/src/Base/CMakeLists.txt b/src/Base/CMakeLists.txt index 478bb59a8b..f2528e7155 100644 --- a/src/Base/CMakeLists.txt +++ b/src/Base/CMakeLists.txt @@ -243,7 +243,6 @@ SET(FreeCADBase_CPP_SRCS Matrix.cpp MatrixPyImp.cpp MemDebug.cpp - Mutex.cpp Observer.cpp Parameter.xsd Parameter.cpp @@ -314,7 +313,6 @@ SET(FreeCADBase_HPP_SRCS Interpreter.h Matrix.h MemDebug.h - Mutex.h Observer.h Parameter.h Persistence.h diff --git a/src/Base/Mutex.cpp b/src/Base/Mutex.cpp deleted file mode 100644 index e3d07c4a4e..0000000000 --- a/src/Base/Mutex.cpp +++ /dev/null @@ -1,36 +0,0 @@ -/*************************************************************************** - * Copyright (c) 2022 Werner Mayer * - * * - * This file is part of the FreeCAD CAx development system. * - * * - * This library is free software; you can redistribute it and/or * - * modify it under the terms of the GNU Library General Public * - * License as published by the Free Software Foundation; either * - * version 2 of the License, or (at your option) any later version. * - * * - * This library is distributed in the hope that it will be useful, * - * but WITHOUT ANY WARRANTY; without even the implied warranty of * - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * - * GNU Library General Public License for more details. * - * * - * You should have received a copy of the GNU Library General Public * - * License along with this library; see the file COPYING.LIB. If not, * - * write to the Free Software Foundation, Inc., 59 Temple Place, * - * Suite 330, Boston, MA 02111-1307, USA * - * * - ***************************************************************************/ - - -#include "PreCompiled.h" - -#include "Mutex.h" - - -#if QT_VERSION < QT_VERSION_CHECK(5, 14, 0) -QRecursiveMutex::QRecursiveMutex() - : QMutex(QMutex::Recursive) -{} - -QRecursiveMutex::~QRecursiveMutex() -{} -#endif diff --git a/src/Base/Mutex.h b/src/Base/Mutex.h deleted file mode 100644 index e65db63092..0000000000 --- a/src/Base/Mutex.h +++ /dev/null @@ -1,39 +0,0 @@ -/*************************************************************************** - * Copyright (c) 2022 Werner Mayer * - * * - * This file is part of the FreeCAD CAx development system. * - * * - * This library is free software; you can redistribute it and/or * - * modify it under the terms of the GNU Library General Public * - * License as published by the Free Software Foundation; either * - * version 2 of the License, or (at your option) any later version. * - * * - * This library is distributed in the hope that it will be useful, * - * but WITHOUT ANY WARRANTY; without even the implied warranty of * - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * - * GNU Library General Public License for more details. * - * * - * You should have received a copy of the GNU Library General Public * - * License along with this library; see the file COPYING.LIB. If not, * - * write to the Free Software Foundation, Inc., 59 Temple Place, * - * Suite 330, Boston, MA 02111-1307, USA * - * * - ***************************************************************************/ - - -#ifndef BASE_MUTEX_H -#define BASE_MUTEX_H - -#include -#include - -#if QT_VERSION < QT_VERSION_CHECK(5, 14, 0) -class BaseExport QRecursiveMutex: public QMutex -{ -public: - QRecursiveMutex(); - ~QRecursiveMutex(); -}; -#endif - -#endif // BASE_MUTEX_H diff --git a/src/Base/PreCompiled.h b/src/Base/PreCompiled.h index f81b2e771b..b38b44a68d 100644 --- a/src/Base/PreCompiled.h +++ b/src/Base/PreCompiled.h @@ -73,6 +73,7 @@ #include #include #include +#include #include // streams @@ -130,8 +131,6 @@ #include #include #include -#include -#include #include #include diff --git a/src/Base/Sequencer.cpp b/src/Base/Sequencer.cpp index 4d41dc0009..3ab27b74be 100644 --- a/src/Base/Sequencer.cpp +++ b/src/Base/Sequencer.cpp @@ -24,12 +24,11 @@ #include "PreCompiled.h" #ifndef _PreComp_ -#include +#include +#include #endif #include "Sequencer.h" -#include "Mutex.h" - using namespace Base; @@ -40,7 +39,7 @@ struct SequencerP // members static std::vector _instances; /**< A vector of all created instances */ static SequencerLauncher* _topLauncher; /**< The outermost launcher */ - static QRecursiveMutex mutex; /**< A mutex-locker for the launcher */ + static std::recursive_mutex mutex; /**< A mutex-locker for the launcher */ /** Sets a global sequencer object. * Access to the last registered object is performed by @see Sequencer(). */ @@ -66,7 +65,7 @@ struct SequencerP */ std::vector SequencerP::_instances; SequencerLauncher* SequencerP::_topLauncher = nullptr; -QRecursiveMutex SequencerP::mutex; +std::recursive_mutex SequencerP::mutex; } // namespace Base SequencerBase& SequencerBase::Instance() @@ -160,7 +159,7 @@ bool SequencerBase::isBlocking() const bool SequencerBase::setLocked(bool bLocked) { - QMutexLocker locker(&SequencerP::mutex); + std::lock_guard locker(SequencerP::mutex); bool old = this->_bLocked; this->_bLocked = bLocked; return old; @@ -168,19 +167,19 @@ bool SequencerBase::setLocked(bool bLocked) bool SequencerBase::isLocked() const { - QMutexLocker locker(&SequencerP::mutex); + std::lock_guard locker(SequencerP::mutex); return this->_bLocked; } bool SequencerBase::isRunning() const { - QMutexLocker locker(&SequencerP::mutex); + std::lock_guard locker(SequencerP::mutex); return (SequencerP::_topLauncher != nullptr); } bool SequencerBase::wasCanceled() const { - QMutexLocker locker(&SequencerP::mutex); + std::lock_guard locker(SequencerP::mutex); return this->_bCanceled; } @@ -236,7 +235,7 @@ void ConsoleSequencer::resetData() SequencerLauncher::SequencerLauncher(const char* pszStr, size_t steps) { - QMutexLocker locker(&SequencerP::mutex); + std::lock_guard locker(SequencerP::mutex); // Have we already an instance of SequencerLauncher created? if (!SequencerP::_topLauncher) { SequencerBase::Instance().start(pszStr, steps); @@ -246,24 +245,22 @@ SequencerLauncher::SequencerLauncher(const char* pszStr, size_t steps) SequencerLauncher::~SequencerLauncher() { - QMutexLocker locker(&SequencerP::mutex); + std::lock_guard locker(SequencerP::mutex); if (SequencerP::_topLauncher == this) { SequencerBase::Instance().stop(); - } - if (SequencerP::_topLauncher == this) { SequencerP::_topLauncher = nullptr; } } void SequencerLauncher::setText(const char* pszTxt) { - QMutexLocker locker(&SequencerP::mutex); + std::lock_guard locker(SequencerP::mutex); SequencerBase::Instance().setText(pszTxt); } bool SequencerLauncher::next(bool canAbort) { - QMutexLocker locker(&SequencerP::mutex); + std::lock_guard locker(SequencerP::mutex); if (SequencerP::_topLauncher != this) { return true; // ignore } @@ -272,13 +269,13 @@ bool SequencerLauncher::next(bool canAbort) void SequencerLauncher::setProgress(size_t pos) { - QMutexLocker locker(&SequencerP::mutex); + std::lock_guard locker(SequencerP::mutex); SequencerBase::Instance().setProgress(pos); } size_t SequencerLauncher::numberOfSteps() const { - QMutexLocker locker(&SequencerP::mutex); + std::lock_guard locker(SequencerP::mutex); return SequencerBase::Instance().numberOfSteps(); } From 3a25a66a05b57ee0940cc590a7f16b45d41be068 Mon Sep 17 00:00:00 2001 From: Ladislav Michl Date: Wed, 15 Nov 2023 10:12:42 +0100 Subject: [PATCH 2/3] Base: Do not use else before return --- src/Base/Interpreter.cpp | 5 +--- src/Base/MatrixPyImp.cpp | 8 ++--- src/Base/Parameter.cpp | 6 +--- src/Base/PlacementPyImp.cpp | 18 +++++------- src/Base/PyObjectBase.cpp | 43 ++++++++++++--------------- src/Base/PyTools.c | 58 +++++++++++++++++-------------------- src/Base/QuantityPyImp.cpp | 10 +++---- src/Base/Reader.cpp | 44 ++++++++++++---------------- src/Base/RotationPyImp.cpp | 18 +++++------- src/Base/StackWalker.cpp | 5 +--- src/Base/UnitPyImp.cpp | 24 ++++++--------- src/Base/UnitsApi.cpp | 1 + src/Base/VectorPyImp.cpp | 51 ++++++++++++-------------------- 13 files changed, 115 insertions(+), 176 deletions(-) diff --git a/src/Base/Interpreter.cpp b/src/Base/Interpreter.cpp index 9c5b7be311..b1e9870f27 100644 --- a/src/Base/Interpreter.cpp +++ b/src/Base/Interpreter.cpp @@ -475,14 +475,11 @@ void InterpreterSingleton::runFile(const char* pxFileName, bool local) if (PyErr_ExceptionMatches(PyExc_SystemExit)) { throw SystemExitException(); } - throw PyException(); } Py_DECREF(result); } - else { - throw FileException("Unknown file", pxFileName); - } + throw FileException("Unknown file", pxFileName); } bool InterpreterSingleton::loadModule(const char* psModName) diff --git a/src/Base/MatrixPyImp.cpp b/src/Base/MatrixPyImp.cpp index 8d0229b0ec..0fdf71b054 100644 --- a/src/Base/MatrixPyImp.cpp +++ b/src/Base/MatrixPyImp.cpp @@ -256,11 +256,9 @@ PyObject* MatrixPy::richCompare(PyObject* v, PyObject* w, int op) Py_INCREF(res); return res; } - else { - // This always returns False - Py_INCREF(Py_NotImplemented); - return Py_NotImplemented; - } + // This always returns False + Py_INCREF(Py_NotImplemented); + return Py_NotImplemented; } PyObject* MatrixPy::move(PyObject* args) diff --git a/src/Base/Parameter.cpp b/src/Base/Parameter.cpp index 47163d7259..54a33bf907 100644 --- a/src/Base/Parameter.cpp +++ b/src/Base/Parameter.cpp @@ -501,11 +501,7 @@ std::vector> ParameterGrp::GetGroups() /// test if this group is empty bool ParameterGrp::IsEmpty() const { - if (_pGroupNode && _pGroupNode->getFirstChild()) { - return false; - } - - return true; + return !(_pGroupNode && _pGroupNode->getFirstChild()); } /// test if a special sub group is in this group diff --git a/src/Base/PlacementPyImp.cpp b/src/Base/PlacementPyImp.cpp index fb5367b591..d57791d69f 100644 --- a/src/Base/PlacementPyImp.cpp +++ b/src/Base/PlacementPyImp.cpp @@ -148,22 +148,18 @@ PyObject* PlacementPy::richCompare(PyObject* v, PyObject* w, int op) PyErr_SetString(PyExc_TypeError, "no ordering relation is defined for Placement"); return nullptr; } - else if (op == Py_EQ) { + if (op == Py_EQ) { res = (p1 == p2) ? Py_True : Py_False; Py_INCREF(res); return res; } - else { - res = (p1 != p2) ? Py_True : Py_False; - Py_INCREF(res); - return res; - } - } - else { - // This always returns False - Py_INCREF(Py_NotImplemented); - return Py_NotImplemented; + res = (p1 != p2) ? Py_True : Py_False; + Py_INCREF(res); + return res; } + // This always returns False + Py_INCREF(Py_NotImplemented); + return Py_NotImplemented; } PyObject* PlacementPy::move(PyObject* args) diff --git a/src/Base/PyObjectBase.cpp b/src/Base/PyObjectBase.cpp index 4f00c0b4ae..0c4000ddfa 100644 --- a/src/Base/PyObjectBase.cpp +++ b/src/Base/PyObjectBase.cpp @@ -365,7 +365,7 @@ int PyObjectBase::__setattro(PyObject *obj, PyObject *attro, PyObject *value) PyErr_Format(PyExc_AttributeError, "Cannot delete attribute: '%s'", attr); return -1; } - else if (!static_cast(obj)->isValid()){ + if (!static_cast(obj)->isValid()){ PyErr_Format(PyExc_ReferenceError, "Cannot access attribute '%s' of deleted object", attr); return -1; } @@ -404,36 +404,32 @@ PyObject *PyObjectBase::_getattr(const char *attr) // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) return reinterpret_cast(Py_TYPE(this)); } - else if (streq(attr, "__members__")) { + if (streq(attr, "__members__")) { // Use __dict__ instead as __members__ is deprecated return nullptr; } - else if (streq(attr,"__dict__")) { + if (streq(attr,"__dict__")) { // Return the default dict PyTypeObject *tp = Py_TYPE(this); Py_XINCREF(tp->tp_dict); return tp->tp_dict; } - else if (streq(attr,"softspace")) { + if (streq(attr,"softspace")) { // Internal Python stuff return nullptr; } - else { - // As fallback solution use Python's default method to get generic attributes - PyObject *w{}; - PyObject *res{}; - w = PyUnicode_InternFromString(attr); - if (w) { - res = PyObject_GenericGetAttr(this, w); - Py_XDECREF(w); - return res; - } else { - // Throw an exception for unknown attributes - PyTypeObject *tp = Py_TYPE(this); - PyErr_Format(PyExc_AttributeError, "%.50s instance has no attribute '%.400s'", tp->tp_name, attr); - return nullptr; - } + // As fallback solution use Python's default method to get generic attributes + PyObject *w{}, *res{}; + w = PyUnicode_InternFromString(attr); + if (w) { + res = PyObject_GenericGetAttr(this, w); + Py_XDECREF(w); + return res; } + // Throw an exception for unknown attributes + PyTypeObject *tp = Py_TYPE(this); + PyErr_Format(PyExc_AttributeError, "%.50s instance has no attribute '%.400s'", tp->tp_name, attr); + return nullptr; } int PyObjectBase::_setattr(const char *attr, PyObject *value) @@ -449,12 +445,11 @@ int PyObjectBase::_setattr(const char *attr, PyObject *value) int res = PyObject_GenericSetAttr(this, w, value); Py_DECREF(w); return res; - } else { - // Throw an exception for unknown attributes - PyTypeObject *tp = Py_TYPE(this); - PyErr_Format(PyExc_AttributeError, "%.50s instance has no attribute '%.400s'", tp->tp_name, attr); - return -1; } + // Throw an exception for unknown attributes + PyTypeObject *tp = Py_TYPE(this); + PyErr_Format(PyExc_AttributeError, "%.50s instance has no attribute '%.400s'", tp->tp_name, attr); + return -1; } /*------------------------------ diff --git a/src/Base/PyTools.c b/src/Base/PyTools.c index 4a90a3becd..e71216a224 100644 --- a/src/Base/PyTools.c +++ b/src/Base/PyTools.c @@ -359,27 +359,25 @@ PP_Convert_Result(PyObject *presult, const char *resFormat, void *resTarget) Py_DECREF(presult); /* procedures and stmts return None */ return 0; } - else if (! PyArg_Parse(presult, resFormat, resTarget)) { /* convert Python->C */ Py_DECREF(presult); /* need not be tuple */ return -1; /* error in convert */ } - else { - if (strcmp(resFormat, "O") != 0) { /* free object unless exported */ - if (strcmp(resFormat, "s") == 0) { /* copy string: caller owns it */ - char **target = (char**) resTarget; + if (strcmp(resFormat, "O") != 0) { /* free object unless exported */ + if (strcmp(resFormat, "s") == 0) { /* copy string: caller owns it */ + char **target = (char**) resTarget; #if defined (__GNUC__) - *target = strdup(*target); + *target = strdup(*target); #else - *target = _strdup(*target); + *target = _strdup(*target); #endif - } - Py_DECREF(presult); } - return 0; /* returns 0=success, -1=failure */ - } /* if 0: C result in *resTarget */ -} /* caller must decref if fmt="O" */ - /* caller must free() if fmt="s" */ + Py_DECREF(presult); + } + return 0; /* returns 0=success, -1=failure */ + /* if 0: C result in *resTarget */ +} /* caller must decref if fmt="O" */ + /* caller must free() if fmt="s" */ int PP_Get_Global(const char *modname, const char *varname, const char *resfmt, void *cresult) @@ -430,27 +428,26 @@ int PP_DEBUG = 0; /* debug embedded code with pdb? */ const char *PP_Init(const char *modname) { Py_Initialize(); /* init python if needed */ - if (modname!=NULL) return modname; - { /* we assume here that the caller frees allocated memory */ - return "__main__"; - } + if (modname) + return modname; + /* we assume here that the caller frees allocated memory */ + return "__main__"; } int -PP_Make_Dummy_Module(const char *modname) /* namespace for strings, if no file */ -{ /* instead of sharing __main__ for all */ - PyObject *module = NULL, *dict = NULL; /* note: __main__ is created in py_init */ +PP_Make_Dummy_Module(const char *modname) /* namespace for strings, if no file */ +{ /* instead of sharing __main__ for all */ + PyObject *module = NULL, *dict = NULL; /* note: __main__ is created in py_init */ Py_Initialize(); module = PyImport_AddModule(modname); /* fetch or make, no load */ if (module == NULL) /* module not incref'd */ return -1; - else { /* module.__dict__ */ - dict = PyModule_GetDict(module); /* ['__dummy__'] = None */ - PyDict_SetItemString(dict, "__dummy__", Py_None); - PyDict_SetItemString(dict, "__builtins__", PyEval_GetBuiltins()); - return 0; - } + /* module.__dict__ */ + dict = PyModule_GetDict(module); /* ['__dummy__'] = None */ + PyDict_SetItemString(dict, "__dummy__", Py_None); + PyDict_SetItemString(dict, "__builtins__", PyEval_GetBuiltins()); + return 0; } @@ -479,17 +476,14 @@ PP_Load_Module(const char *modname) /* modname can be "package.module" for PyDict_GetItemString(PyModule_GetDict(module), "__dummy__")) { return module; /* not increfd */ } - else if (PP_RELOAD && module != NULL && PyModule_Check(module)) { module = PyImport_ReloadModule(module); /* reload file,run code */ Py_XDECREF(module); /* still on sys.modules */ return module; /* not increfd */ } - else { - module = PyImport_ImportModule(modname); /* fetch or load module */ - Py_XDECREF(module); /* still on sys.modules */ - return module; /* not increfd */ - } + module = PyImport_ImportModule(modname); /* fetch or load module */ + Py_XDECREF(module); /* still on sys.modules */ + return module; /* not increfd */ } diff --git a/src/Base/QuantityPyImp.cpp b/src/Base/QuantityPyImp.cpp index 6069342ebf..527c01e2d4 100644 --- a/src/Base/QuantityPyImp.cpp +++ b/src/Base/QuantityPyImp.cpp @@ -508,20 +508,18 @@ PyObject* QuantityPy::number_power_handler(PyObject* self, PyObject* other, PyOb return new QuantityPy(new Quantity(q)); } - else if (PyFloat_Check(other)) { + if (PyFloat_Check(other)) { Base::Quantity* a = static_cast(self)->getQuantityPtr(); double b = PyFloat_AsDouble(other); return new QuantityPy(new Quantity(a->pow(b))); } - else if (PyLong_Check(other)) { + if (PyLong_Check(other)) { Base::Quantity* a = static_cast(self)->getQuantityPtr(); double b = (double)PyLong_AsLong(other); return new QuantityPy(new Quantity(a->pow(b))); } - else { - PyErr_SetString(PyExc_TypeError, "Expected quantity or number"); - return nullptr; - } + PyErr_SetString(PyExc_TypeError, "Expected quantity or number"); + return nullptr; } PY_CATCH } diff --git a/src/Base/Reader.cpp b/src/Base/Reader.cpp index 6b6068bf12..3fb14a803c 100644 --- a/src/Base/Reader.cpp +++ b/src/Base/Reader.cpp @@ -116,12 +116,10 @@ long Base::XMLReader::getAttributeAsInteger(const char* AttrName) const if (pos != AttrMap.end()) { return atol(pos->second.c_str()); } - else { - // wrong name, use hasAttribute if not sure! - std::ostringstream msg; - msg << "XML Attribute: \"" << AttrName << "\" not found"; - throw Base::XMLAttributeError(msg.str()); - } + // wrong name, use hasAttribute if not sure! + std::ostringstream msg; + msg << "XML Attribute: \"" << AttrName << "\" not found"; + throw Base::XMLAttributeError(msg.str()); } unsigned long Base::XMLReader::getAttributeAsUnsigned(const char* AttrName) const @@ -131,12 +129,10 @@ unsigned long Base::XMLReader::getAttributeAsUnsigned(const char* AttrName) cons if (pos != AttrMap.end()) { return strtoul(pos->second.c_str(), nullptr, 10); } - else { - // wrong name, use hasAttribute if not sure! - std::ostringstream msg; - msg << "XML Attribute: \"" << AttrName << "\" not found"; - throw Base::XMLAttributeError(msg.str()); - } + // wrong name, use hasAttribute if not sure! + std::ostringstream msg; + msg << "XML Attribute: \"" << AttrName << "\" not found"; + throw Base::XMLAttributeError(msg.str()); } double Base::XMLReader::getAttributeAsFloat(const char* AttrName) const @@ -146,12 +142,10 @@ double Base::XMLReader::getAttributeAsFloat(const char* AttrName) const if (pos != AttrMap.end()) { return atof(pos->second.c_str()); } - else { - // wrong name, use hasAttribute if not sure! - std::ostringstream msg; - msg << "XML Attribute: \"" << AttrName << "\" not found"; - throw Base::XMLAttributeError(msg.str()); - } + // wrong name, use hasAttribute if not sure! + std::ostringstream msg; + msg << "XML Attribute: \"" << AttrName << "\" not found"; + throw Base::XMLAttributeError(msg.str()); } const char* Base::XMLReader::getAttribute(const char* AttrName) const @@ -161,12 +155,10 @@ const char* Base::XMLReader::getAttribute(const char* AttrName) const if (pos != AttrMap.end()) { return pos->second.c_str(); } - else { - // wrong name, use hasAttribute if not sure! - std::ostringstream msg; - msg << "XML Attribute: \"" << AttrName << "\" not found"; - throw Base::XMLAttributeError(msg.str()); - } + // wrong name, use hasAttribute if not sure! + std::ostringstream msg; + msg << "XML Attribute: \"" << AttrName << "\" not found"; + throw Base::XMLAttributeError(msg.str()); } bool Base::XMLReader::hasAttribute(const char* AttrName) const @@ -216,7 +208,7 @@ void Base::XMLReader::readElement(const char* ElementName) // thus we must stop reading on. break; } - else if (ReadType == EndDocument) { + if (ReadType == EndDocument) { // the end of the document has been reached but we still try to continue on reading throw Base::XMLParseException("End of document reached"); } @@ -276,7 +268,7 @@ void Base::XMLReader::readEndElement(const char* ElementName, int level) && (level < 0 || level == Level)) { return; } - else if (ReadType == EndDocument) { + if (ReadType == EndDocument) { // the end of the document has been reached but we still try to continue on reading throw Base::XMLParseException("End of document reached"); } diff --git a/src/Base/RotationPyImp.cpp b/src/Base/RotationPyImp.cpp index 8d0b019a1c..a4747ac6f1 100644 --- a/src/Base/RotationPyImp.cpp +++ b/src/Base/RotationPyImp.cpp @@ -271,22 +271,18 @@ PyObject* RotationPy::richCompare(PyObject* v, PyObject* w, int op) PyErr_SetString(PyExc_TypeError, "no ordering relation is defined for Rotation"); return nullptr; } - else if (op == Py_EQ) { + if (op == Py_EQ) { res = (r1 == r2) ? Py_True : Py_False; Py_INCREF(res); return res; } - else { - res = (r1 != r2) ? Py_True : Py_False; - Py_INCREF(res); - return res; - } - } - else { - // This always returns False - Py_INCREF(Py_NotImplemented); - return Py_NotImplemented; + res = (r1 != r2) ? Py_True : Py_False; + Py_INCREF(res); + return res; } + // This always returns False + Py_INCREF(Py_NotImplemented); + return Py_NotImplemented; } PyObject* RotationPy::invert(PyObject* args) diff --git a/src/Base/StackWalker.cpp b/src/Base/StackWalker.cpp index c3c4a7159c..fcc7fa4dc7 100644 --- a/src/Base/StackWalker.cpp +++ b/src/Base/StackWalker.cpp @@ -1118,10 +1118,7 @@ BOOL __stdcall StackWalker::myReadProcMem( //printf("ReadMemory: hProcess: %p, baseAddr: %p, buffer: %p, size: %d, read: %d, result: %d\n", hProcess, (LPVOID) qwBaseAddress, lpBuffer, nSize, (DWORD) st, (DWORD) bRet); return bRet; } - else - { - return s_readMemoryFunction(hProcess, qwBaseAddress, lpBuffer, nSize, lpNumberOfBytesRead, s_readMemoryFunction_UserData); - } + return s_readMemoryFunction(hProcess, qwBaseAddress, lpBuffer, nSize, lpNumberOfBytesRead, s_readMemoryFunction_UserData); } void StackWalker::OnLoadModule(LPCSTR img, LPCSTR mod, DWORD64 baseAddr, DWORD size, DWORD result, LPCSTR symType, LPCSTR pdbName, ULONGLONG fileVersion) diff --git a/src/Base/UnitPyImp.cpp b/src/Base/UnitPyImp.cpp index e8db65865a..364ffb6641 100644 --- a/src/Base/UnitPyImp.cpp +++ b/src/Base/UnitPyImp.cpp @@ -175,10 +175,8 @@ PyObject* UnitPy::number_multiply_handler(PyObject* self, PyObject* other) return new UnitPy(new Unit((*a) * (*b))); } - else { - PyErr_SetString(PyExc_TypeError, "A Unit can only be multiplied by a Unit"); - return nullptr; - } + PyErr_SetString(PyExc_TypeError, "A Unit can only be multiplied by a Unit"); + return nullptr; } PyObject* UnitPy::richCompare(PyObject* v, PyObject* w, int op) @@ -192,22 +190,18 @@ PyObject* UnitPy::richCompare(PyObject* v, PyObject* w, int op) PyErr_SetString(PyExc_TypeError, "no ordering relation is defined for Units"); return nullptr; } - else if (op == Py_EQ) { + if (op == Py_EQ) { res = (*u1 == *u2) ? Py_True : Py_False; // NOLINT Py_INCREF(res); return res; } - else { - res = (*u1 != *u2) ? Py_True : Py_False; // NOLINT - Py_INCREF(res); - return res; - } - } - else { - // This always returns False - Py_INCREF(Py_NotImplemented); - return Py_NotImplemented; + res = (*u1 != *u2) ? Py_True : Py_False; // NOLINT + Py_INCREF(res); + return res; } + // This always returns False + Py_INCREF(Py_NotImplemented); + return Py_NotImplemented; } Py::String UnitPy::getType() const diff --git a/src/Base/UnitsApi.cpp b/src/Base/UnitsApi.cpp index f0c118bd20..9984a4dee8 100644 --- a/src/Base/UnitsApi.cpp +++ b/src/Base/UnitsApi.cpp @@ -193,6 +193,7 @@ double UnitsApi::toDouble(PyObject* args, const Base::Unit& u) } throw Base::UnitsMismatchError("Wrong unit type!"); } + if (PyFloat_Check(args)) { return PyFloat_AsDouble(args); } diff --git a/src/Base/VectorPyImp.cpp b/src/Base/VectorPyImp.cpp index 8e0ea738e9..bf2b66181b 100644 --- a/src/Base/VectorPyImp.cpp +++ b/src/Base/VectorPyImp.cpp @@ -150,30 +150,24 @@ PyObject* VectorPy::number_multiply_handler(PyObject* self, PyObject* other) Py::Float mult(a * b); return Py::new_reference_to(mult); } - else if (PyNumber_Check(other)) { + if (PyNumber_Check(other)) { double b = PyFloat_AsDouble(other); return new VectorPy(a * b); } - else { - PyErr_SetString(PyExc_TypeError, "A Vector can only be multiplied by Vector or number"); - return nullptr; - } + PyErr_SetString(PyExc_TypeError, "A Vector can only be multiplied by Vector or number"); + return nullptr; } - else if (PyObject_TypeCheck(other, &(VectorPy::Type))) { + if (PyObject_TypeCheck(other, &(VectorPy::Type))) { Base::Vector3d a = static_cast(other)->value(); if (PyNumber_Check(self)) { double b = PyFloat_AsDouble(self); return new VectorPy(a * b); } - else { - PyErr_SetString(PyExc_TypeError, "A Vector can only be multiplied by Vector or number"); - return nullptr; - } - } - else { - PyErr_SetString(PyExc_TypeError, "First or second arg must be Vector"); + PyErr_SetString(PyExc_TypeError, "A Vector can only be multiplied by Vector or number"); return nullptr; } + PyErr_SetString(PyExc_TypeError, "First or second arg must be Vector"); + return nullptr; } Py_ssize_t VectorPy::sequence_length(PyObject* /*unused*/) @@ -243,13 +237,8 @@ PyObject* VectorPy::mapping_subscript(PyObject* self, PyObject* item) } return sequence_item(self, i); } - else if (PySlice_Check(item)) { - Py_ssize_t start = 0; - Py_ssize_t stop = 0; - Py_ssize_t step = 0; - Py_ssize_t slicelength = 0; - Py_ssize_t cur = 0; - Py_ssize_t i = 0; + if (PySlice_Check(item)) { + Py_ssize_t start = 0, stop = 0, step = 0, slicelength = 0, cur = 0, i = 0; PyObject* slice = item; if (PySlice_GetIndicesEx(slice, sequence_length(self), &start, &stop, &step, &slicelength) @@ -260,7 +249,7 @@ PyObject* VectorPy::mapping_subscript(PyObject* self, PyObject* item) if (slicelength <= 0) { return PyTuple_New(0); } - else if (start == 0 && step == 1 && slicelength == sequence_length(self) + if (start == 0 && step == 1 && slicelength == sequence_length(self) && PyObject_TypeCheck(self, &(VectorPy::Type))) { Base::Vector3d v = static_cast(self)->value(); Py::Tuple xyz(3); @@ -269,7 +258,7 @@ PyObject* VectorPy::mapping_subscript(PyObject* self, PyObject* item) xyz.setItem(2, Py::Float(v.z)); return Py::new_reference_to(xyz); } - else if (PyObject_TypeCheck(self, &(VectorPy::Type))) { + if (PyObject_TypeCheck(self, &(VectorPy::Type))) { Base::Vector3d v = static_cast(self)->value(); Py::Tuple xyz(static_cast(slicelength)); @@ -342,22 +331,18 @@ PyObject* VectorPy::richCompare(PyObject* v, PyObject* w, int op) PyErr_SetString(PyExc_TypeError, "no ordering relation is defined for Vector"); return nullptr; } - else if (op == Py_EQ) { + if (op == Py_EQ) { res = (v1 == v2) ? Py_True : Py_False; // NOLINT Py_INCREF(res); return res; } - else { - res = (v1 != v2) ? Py_True : Py_False; // NOLINT - Py_INCREF(res); - return res; - } - } - else { - // This always returns False - Py_INCREF(Py_NotImplemented); - return Py_NotImplemented; + res = (v1 != v2) ? Py_True : Py_False; // NOLINT + Py_INCREF(res); + return res; } + // This always returns False + Py_INCREF(Py_NotImplemented); + return Py_NotImplemented; } PyObject* VectorPy::isEqual(PyObject* args) From 839cd947c7b0600fb20716878245e6cc7b275957 Mon Sep 17 00:00:00 2001 From: Ladislav Michl Date: Sat, 20 Jan 2024 18:18:09 +0100 Subject: [PATCH 3/3] Base: Replace if else with switch statement --- src/Base/Console.h | 30 +++++------ src/Base/Persistence.cpp | 54 ++++++++++--------- src/Base/QuantityPyImp.cpp | 108 +++++++++++++++++-------------------- src/Base/Tools.cpp | 42 ++++++++------- src/Base/VectorPyImp.cpp | 2 +- 5 files changed, 115 insertions(+), 121 deletions(-) diff --git a/src/Base/Console.h b/src/Base/Console.h index ab2a2b3616..ed350bd5ca 100644 --- a/src/Base/Console.h +++ b/src/Base/Console.h @@ -579,23 +579,19 @@ public: */ bool isActive(Base::LogStyle category) const { - if (category == Base::LogStyle::Log) { - return bLog; - } - if (category == Base::LogStyle::Warning) { - return bWrn; - } - if (category == Base::LogStyle::Error) { - return bErr; - } - if (category == Base::LogStyle::Message) { - return bMsg; - } - if (category == Base::LogStyle::Critical) { - return bCritical; - } - if (category == Base::LogStyle::Notification) { - return bNotification; + switch (category) { + case Base::LogStyle::Log: + return bLog; + case Base::LogStyle::Warning: + return bWrn; + case Base::LogStyle::Error: + return bErr; + case Base::LogStyle::Message: + return bMsg; + case Base::LogStyle::Critical: + return bCritical; + case Base::LogStyle::Notification: + return bNotification; } return false; diff --git a/src/Base/Persistence.cpp b/src/Base/Persistence.cpp index 094b85767f..7dc3f7d42e 100644 --- a/src/Base/Persistence.cpp +++ b/src/Base/Persistence.cpp @@ -76,32 +76,34 @@ std::string Persistence::encodeAttribute(const std::string& str) { std::string tmp; for (char it : str) { - if (it == '<') { - tmp += "<"; - } - else if (it == '\"') { - tmp += """; - } - else if (it == '\'') { - tmp += "'"; - } - else if (it == '&') { - tmp += "&"; - } - else if (it == '>') { - tmp += ">"; - } - else if (it == '\r') { - tmp += " "; - } - else if (it == '\n') { - tmp += " "; - } - else if (it == '\t') { - tmp += " "; - } - else { - tmp += it; + switch (it) { + case '<': + tmp += "<"; + break; + case '\"': + tmp += """; + break; + case '\'': + tmp += "'"; + break; + case '&': + tmp += "&"; + break; + case '>': + tmp += ">"; + break; + case '\r': + tmp += " "; + break; + case '\n': + tmp += " "; + break; + case '\t': + tmp += " "; + break; + default: + tmp += it; + break; } } diff --git a/src/Base/QuantityPyImp.cpp b/src/Base/QuantityPyImp.cpp index 527c01e2d4..4a59496f42 100644 --- a/src/Base/QuantityPyImp.cpp +++ b/src/Base/QuantityPyImp.cpp @@ -541,35 +541,31 @@ PyObject* QuantityPy::richCompare(PyObject* v, PyObject* w, int op) const Quantity* u2 = static_cast(w)->getQuantityPtr(); PyObject* res = nullptr; - if (op == Py_NE) { - res = (!(*u1 == *u2)) ? Py_True : Py_False; - Py_INCREF(res); - return res; - } - else if (op == Py_LT) { - res = (*u1 < *u2) ? Py_True : Py_False; - Py_INCREF(res); - return res; - } - else if (op == Py_LE) { - res = (*u1 < *u2) || (*u1 == *u2) ? Py_True : Py_False; - Py_INCREF(res); - return res; - } - else if (op == Py_GT) { - res = (!(*u1 < *u2)) && (!(*u1 == *u2)) ? Py_True : Py_False; - Py_INCREF(res); - return res; - } - else if (op == Py_GE) { - res = (!(*u1 < *u2)) ? Py_True : Py_False; - Py_INCREF(res); - return res; - } - else if (op == Py_EQ) { - res = (*u1 == *u2) ? Py_True : Py_False; - Py_INCREF(res); - return res; + switch (op) { + case Py_NE: + res = (!(*u1 == *u2)) ? Py_True : Py_False; + Py_INCREF(res); + return res; + case Py_LT: + res = (*u1 < *u2) ? Py_True : Py_False; + Py_INCREF(res); + return res; + case Py_LE: + res = (*u1 < *u2) || (*u1 == *u2) ? Py_True : Py_False; + Py_INCREF(res); + return res; + case Py_GT: + res = (!(*u1 < *u2)) && (!(*u1 == *u2)) ? Py_True : Py_False; + Py_INCREF(res); + return res; + case Py_GE: + res = (!(*u1 < *u2)) ? Py_True : Py_False; + Py_INCREF(res); + return res; + case Py_EQ: + res = (*u1 == *u2) ? Py_True : Py_False; + Py_INCREF(res); + return res; } } else if (PyNumber_Check(v) && PyNumber_Check(w)) { @@ -577,35 +573,31 @@ PyObject* QuantityPy::richCompare(PyObject* v, PyObject* w, int op) double u1 = PyFloat_AsDouble(v); double u2 = PyFloat_AsDouble(w); PyObject* res = nullptr; - if (op == Py_NE) { - res = (u1 != u2) ? Py_True : Py_False; - Py_INCREF(res); - return res; - } - else if (op == Py_LT) { - res = (u1 < u2) ? Py_True : Py_False; - Py_INCREF(res); - return res; - } - else if (op == Py_LE) { - res = (u1 <= u2) ? Py_True : Py_False; - Py_INCREF(res); - return res; - } - else if (op == Py_GT) { - res = (u1 > u2) ? Py_True : Py_False; - Py_INCREF(res); - return res; - } - else if (op == Py_GE) { - res = (u1 >= u2) ? Py_True : Py_False; - Py_INCREF(res); - return res; - } - else if (op == Py_EQ) { - res = (u1 == u2) ? Py_True : Py_False; - Py_INCREF(res); - return res; + switch (op) { + case Py_NE: + res = (u1 != u2) ? Py_True : Py_False; + Py_INCREF(res); + return res; + case Py_LT: + res = (u1 < u2) ? Py_True : Py_False; + Py_INCREF(res); + return res; + case Py_LE: + res = (u1 <= u2) ? Py_True : Py_False; + Py_INCREF(res); + return res; + case Py_GT: + res = (u1 > u2) ? Py_True : Py_False; + Py_INCREF(res); + return res; + case Py_GE: + res = (u1 >= u2) ? Py_True : Py_False; + Py_INCREF(res); + return res; + case Py_EQ: + res = (u1 == u2) ? Py_True : Py_False; + Py_INCREF(res); + return res; } } diff --git a/src/Base/Tools.cpp b/src/Base/Tools.cpp index 1f315ce721..9e313be85e 100644 --- a/src/Base/Tools.cpp +++ b/src/Base/Tools.cpp @@ -264,17 +264,19 @@ std::string Base::Tools::escapeEncodeString(const std::string& s) std::string result; size_t len = s.size(); for (size_t i = 0; i < len; ++i) { - if (s.at(i) == '\\') { - result += "\\\\"; - } - else if (s.at(i) == '\"') { - result += "\\\""; - } - else if (s.at(i) == '\'') { - result += "\\\'"; - } - else { - result += s.at(i); + switch (s.at(i)) { + case '\\': + result += "\\\\"; + break; + case '\"': + result += "\\\""; + break; + case '\'': + result += "\\\'"; + break; + default: + result += s.at(i); + break; } } return result; @@ -305,14 +307,16 @@ std::string Base::Tools::escapeEncodeFilename(const std::string& s) std::string result; size_t len = s.size(); for (size_t i = 0; i < len; ++i) { - if (s.at(i) == '\"') { - result += "\\\""; - } - else if (s.at(i) == '\'') { - result += "\\\'"; - } - else { - result += s.at(i); + switch (s.at(i)) { + case '\"': + result += "\\\""; + break; + case '\'': + result += "\\\'"; + break; + default: + result += s.at(i); + break; } } return result; diff --git a/src/Base/VectorPyImp.cpp b/src/Base/VectorPyImp.cpp index bf2b66181b..80d730c39d 100644 --- a/src/Base/VectorPyImp.cpp +++ b/src/Base/VectorPyImp.cpp @@ -250,7 +250,7 @@ PyObject* VectorPy::mapping_subscript(PyObject* self, PyObject* item) return PyTuple_New(0); } if (start == 0 && step == 1 && slicelength == sequence_length(self) - && PyObject_TypeCheck(self, &(VectorPy::Type))) { + && PyObject_TypeCheck(self, &(VectorPy::Type))) { Base::Vector3d v = static_cast(self)->value(); Py::Tuple xyz(3); xyz.setItem(0, Py::Float(v.x));