From 659a55ffb92dda3bd2707b4964f62be592602b89 Mon Sep 17 00:00:00 2001 From: wmayer Date: Wed, 19 Jan 2022 18:00:04 +0100 Subject: [PATCH] Base: Exception handling: * Harmonize FreeCAD with Python exception types * Implement AbortException::getPyExceptionType() to avoid handling it in client code * Remove catch block for plain C strings --- src/App/Application.cpp | 20 ++++----- src/Base/Exception.cpp | 38 +++++++++------- src/Base/Exception.h | 43 ++++++++++--------- src/Base/Interpreter.h | 10 ++--- src/Base/PyObjectBase.h | 19 ++++---- src/Mod/Sketcher/Gui/ViewProviderSketch.cpp | 8 ++-- .../templateClassPyExport.py | 36 ++-------------- 7 files changed, 75 insertions(+), 99 deletions(-) diff --git a/src/App/Application.cpp b/src/App/Application.cpp index 9919b5447c..8b84f1f4ad 100644 --- a/src/App/Application.cpp +++ b/src/App/Application.cpp @@ -178,8 +178,8 @@ using namespace std; ParameterManager *App::Application::_pcSysParamMngr; ParameterManager *App::Application::_pcUserParamMngr; -Base::ConsoleObserverStd *Application::_pConsoleObserverStd =0; -Base::ConsoleObserverFile *Application::_pConsoleObserverFile =0; +Base::ConsoleObserverStd *Application::_pConsoleObserverStd = nullptr; +Base::ConsoleObserverFile *Application::_pConsoleObserverFile = nullptr; AppExport std::map Application::mConfig; BaseExport extern PyObject* Base::BaseExceptionFreeCADError; @@ -262,7 +262,7 @@ Application::Application(std::map &mConfig) PyModuleDef_HEAD_INIT, "__FreeCADConsole__", Console_doc, -1, ConsoleSingleton::Methods, - NULL, NULL, NULL, NULL + nullptr, nullptr, nullptr, nullptr }; PyObject* pConsoleModule = PyModule_Create(&ConsoleModuleDef); @@ -289,11 +289,11 @@ Application::Application(std::map &mConfig) PyDict_SetItemString(modules, "__FreeCADBase__", pBaseModule); } - Base::BaseExceptionFreeCADError = PyErr_NewException("Base.FreeCADError", PyExc_RuntimeError, NULL); + Base::BaseExceptionFreeCADError = PyErr_NewException("Base.FreeCADError", PyExc_RuntimeError, nullptr); Py_INCREF(Base::BaseExceptionFreeCADError); PyModule_AddObject(pBaseModule, "FreeCADError", Base::BaseExceptionFreeCADError); - Base::BaseExceptionFreeCADAbort = PyErr_NewException("Base.FreeCADAbort", PyExc_BaseException, NULL); + Base::BaseExceptionFreeCADAbort = PyErr_NewException("Base.FreeCADAbort", PyExc_BaseException, nullptr); Py_INCREF(Base::BaseExceptionFreeCADAbort); PyModule_AddObject(pBaseModule, "FreeCADAbort", Base::BaseExceptionFreeCADAbort); @@ -343,7 +343,7 @@ Application::Application(std::map &mConfig) PyModuleDef_HEAD_INIT, "Units", "The Unit API", -1, Base::UnitsApi::Methods, - NULL, NULL, NULL, NULL + nullptr, nullptr, nullptr, nullptr }; PyObject* pUnitsModule = PyModule_Create(&UnitsModuleDef); Base::Interpreter().addType(&Base::QuantityPy ::Type,pUnitsModule,"Quantity"); @@ -1790,7 +1790,7 @@ void my_se_translator_filter(unsigned int code, EXCEPTION_POINTERS* pExp) throw Base::AccessViolation(); case EXCEPTION_FLT_DIVIDE_BY_ZERO: case EXCEPTION_INT_DIVIDE_BY_ZERO: - //throw Base::DivisionByZeroError("Division by zero!"); + //throw Base::ZeroDivisionError("Division by zero!"); Base::Console().Error("SEH exception (%u): Division by zero\n", code); return; } @@ -1837,7 +1837,7 @@ void Application::init(int argc, char ** argv) } } -void Application::initTypes(void) +void Application::initTypes() { // Base types Base::Type ::init(); @@ -2019,8 +2019,8 @@ void Application::initTypes(void) new ExceptionProducer; new ExceptionProducer; new ExceptionProducer; - new ExceptionProducer; - new ExceptionProducer; + new ExceptionProducer; + new ExceptionProducer; new ExceptionProducer; new ExceptionProducer; new ExceptionProducer; diff --git a/src/Base/Exception.cpp b/src/Base/Exception.cpp index fa5cf40c0c..39ce4e582f 100644 --- a/src/Base/Exception.cpp +++ b/src/Base/Exception.cpp @@ -39,7 +39,7 @@ using namespace Base; TYPESYSTEM_SOURCE(Base::Exception,Base::BaseClass) -Exception::Exception(void) +Exception::Exception() : _line(0) , _isTranslatable(false) , _isReported(false) @@ -48,7 +48,8 @@ Exception::Exception(void) } Exception::Exception(const Exception &inst) - : _sErrMsg(inst._sErrMsg) + : BaseClass(inst) + , _sErrMsg(inst._sErrMsg) , _file(inst._file) , _line(inst._line) , _function(inst._function) @@ -82,7 +83,7 @@ Exception &Exception::operator=(const Exception &inst) return *this; } -const char* Exception::what(void) const throw() +const char* Exception::what() const throw() { return _sErrMsg.c_str(); } @@ -105,7 +106,7 @@ void Exception::ReportException (void) const } } -PyObject * Exception::getPyObject(void) +PyObject * Exception::getPyObject() { Py::Dict edict; edict.setItem("sclassname", Py::String(typeid(*this).name())); @@ -180,6 +181,11 @@ const char* AbortException::what() const throw() return Exception::what(); } +PyObject * AbortException::getPyExceptionType() const +{ + return BaseExceptionFreeCADAbort; +} + // --------------------------------------------------------- @@ -297,7 +303,7 @@ const char* FileException::what() const throw() return _sErrMsgAndFileName.c_str(); } -void FileException::ReportException (void) const +void FileException::ReportException () const { if (!_isReported) { const char *msg; @@ -315,7 +321,7 @@ void FileException::ReportException (void) const } } -PyObject * FileException::getPyObject(void) +PyObject * FileException::getPyObject() { Py::Dict edict(Exception::getPyObject(), true); edict.setItem("filename", Py::String(this->file.fileName())); @@ -324,7 +330,7 @@ PyObject * FileException::getPyObject(void) void FileException::setPyObject( PyObject * pydict) { - if (pydict!=NULL) { + if (pydict!=nullptr) { Exception::setPyObject(pydict); Py::Dict edict(pydict); @@ -659,43 +665,43 @@ PyObject *NotImplementedError::getPyExceptionType() const { // --------------------------------------------------------- -DivisionByZeroError::DivisionByZeroError() +ZeroDivisionError::ZeroDivisionError() : Exception() { } -DivisionByZeroError::DivisionByZeroError(const char * sMessage) +ZeroDivisionError::ZeroDivisionError(const char * sMessage) : Exception(sMessage) { } -DivisionByZeroError::DivisionByZeroError(const std::string& sMessage) +ZeroDivisionError::ZeroDivisionError(const std::string& sMessage) : Exception(sMessage) { } -PyObject *DivisionByZeroError::getPyExceptionType() const { +PyObject *ZeroDivisionError::getPyExceptionType() const { return PyExc_ZeroDivisionError; } // --------------------------------------------------------- -ReferencesError::ReferencesError() +ReferenceError::ReferenceError() : Exception() { } -ReferencesError::ReferencesError(const char * sMessage) +ReferenceError::ReferenceError(const char * sMessage) : Exception(sMessage) { } -ReferencesError::ReferencesError(const std::string& sMessage) +ReferenceError::ReferenceError(const std::string& sMessage) : Exception(sMessage) { } -PyObject *ReferencesError::getPyExceptionType() const { +PyObject *ReferenceError::getPyExceptionType() const { return PyExc_ReferenceError; } @@ -871,7 +877,7 @@ SignalException::SignalException() SignalException::~SignalException() { - sigaction (SIGSEGV, &old_action, NULL); + sigaction (SIGSEGV, &old_action, nullptr); #ifdef _DEBUG std::cout << "Restore old signal handler" << std::endl; #endif diff --git a/src/Base/Exception.h b/src/Base/Exception.h index 0aff3c9e1f..aa59677a49 100644 --- a/src/Base/Exception.h +++ b/src/Base/Exception.h @@ -28,11 +28,12 @@ #include #include #include -#include -#include +#include #include "FileInfo.h" #include "BaseClass.h" +typedef struct _object PyObject; + /* MACROS FOR THROWING EXCEPTIONS */ /// the macros do NOT mark any message for translation @@ -96,10 +97,10 @@ public: Exception &operator=(const Exception &inst); - virtual const char* what(void) const throw(); + virtual const char* what() const throw(); /// Reports exception. It includes a mechanism to only report an exception once. - virtual void ReportException (void) const; + virtual void ReportException () const; inline void setMessage(const char * sMessage); inline void setMessage(const std::string& sMessage); @@ -121,7 +122,7 @@ public: inline void setReported(bool reported) { _isReported = reported; } /// returns a Python dictionary containing the exception data - virtual PyObject * getPyObject(void); + virtual PyObject * getPyObject(); /// returns sets the exception data from a Python dictionary virtual void setPyObject( PyObject * pydict); @@ -138,7 +139,7 @@ protected: * This way, the file, line, and function are automatically inserted. */ Exception(const char * sMessage); Exception(const std::string& sMessage); - Exception(void); + Exception(); Exception(const Exception &inst); protected: @@ -168,6 +169,8 @@ public: virtual ~AbortException() throw() {} /// Description of the exception virtual const char* what() const throw(); + /// returns the corresponding python exception type + virtual PyObject * getPyExceptionType() const; }; /** @@ -234,7 +237,7 @@ class BaseExport FileException : public Exception { public: /// With massage and file name - FileException(const char * sMessage, const char * sFileName=0); + FileException(const char * sMessage, const char * sFileName=nullptr); /// With massage and file name FileException(const char * sMessage, const FileInfo& File); /// standard construction @@ -248,11 +251,11 @@ public: /// Description of the exception virtual const char* what() const throw() override; /// Report generation - virtual void ReportException (void) const override; + virtual void ReportException () const override; /// Get file name for use with translatable message std::string getFileName() const; /// returns a Python dictionary containing the exception data - virtual PyObject * getPyObject(void) override; + virtual PyObject * getPyObject() override; /// returns sets the exception data from a Python dictionary virtual void setPyObject( PyObject * pydict) override; @@ -262,7 +265,7 @@ protected: // necessary for what() legacy behaviour as it returns a buffer that // can not be of a temporary object to be destroyed at end of what() std::string _sErrMsgAndFileName; - void setFileName(const char * sFileName=0); + void setFileName(const char * sFileName=nullptr); }; /** @@ -523,15 +526,15 @@ public: * The ZeroDivisionError can be used to indicate a division by zero. * @author Werner Mayer */ -class BaseExport DivisionByZeroError : public Exception +class BaseExport ZeroDivisionError : public Exception { public: /// Construction - DivisionByZeroError(); - DivisionByZeroError(const char * sMessage); - DivisionByZeroError(const std::string& sMessage); + ZeroDivisionError(); + ZeroDivisionError(const char * sMessage); + ZeroDivisionError(const std::string& sMessage); /// Destruction - virtual ~DivisionByZeroError() throw() {} + virtual ~ZeroDivisionError() throw() {} virtual PyObject * getPyExceptionType() const override; }; @@ -539,15 +542,15 @@ public: * The ReferenceError can be used to indicate a reference counter has the wrong value. * @author Werner Mayer */ -class BaseExport ReferencesError : public Exception +class BaseExport ReferenceError : public Exception { public: /// Construction - ReferencesError(); - ReferencesError(const char * sMessage); - ReferencesError(const std::string& sMessage); + ReferenceError(); + ReferenceError(const char * sMessage); + ReferenceError(const std::string& sMessage); /// Destruction - virtual ~ReferencesError() throw() {} + virtual ~ReferenceError() throw() {} virtual PyObject * getPyExceptionType() const override; }; diff --git a/src/Base/Interpreter.h b/src/Base/Interpreter.h index 66cbe49eed..f5050f67c9 100644 --- a/src/Base/Interpreter.h +++ b/src/Base/Interpreter.h @@ -97,7 +97,7 @@ class BaseExport PyException : public Exception { public: /// constructor does the whole job - PyException(void); + PyException(); PyException(const Py::Object &obj); ~PyException() throw(); @@ -106,13 +106,13 @@ public: /// this method determines if the original exception /// can be reconstructed or not, if yes throws the reconstructed version /// if not, throws a generic PyException. - static void ThrowException(void); + static void ThrowException(); /// this function returns the stack trace - const std::string &getStackTrace(void) const {return _stackTrace;} - const std::string &getErrorType(void) const {return _errorType;} + const std::string &getStackTrace() const {return _stackTrace;} + const std::string &getErrorType() const {return _errorType;} virtual PyObject *getPyExceptionType(void) const override {return _exceptionType;} - void ReportException (void) const override; + void ReportException () const override; /// Sets the Python error indicator and an error message virtual void setPyException() const override; diff --git a/src/Base/PyObjectBase.h b/src/Base/PyObjectBase.h index 89e794fae7..a68e3c9066 100644 --- a/src/Base/PyObjectBase.h +++ b/src/Base/PyObjectBase.h @@ -135,15 +135,15 @@ inline void Assert(int expr, char *msg) // C++ assert /// return with no return value if nothing happens #define Py_Return return Py_INCREF(Py_None), Py_None /// returns an error -#define Py_Error(E, M) _Py_Error(return(NULL),E,M) +#define Py_Error(E, M) _Py_Error(return(nullptr),E,M) #define _Py_Error(R, E, M) {PyErr_SetString(E, M); R;} /// returns an error -#define Py_ErrorObj(E, O) _Py_ErrorObj(return(NULL),E,O) +#define Py_ErrorObj(E, O) _Py_ErrorObj(return(nullptr),E,O) #define _Py_ErrorObj(R, E, O) {PyErr_SetObject(E, O); R;} /// checks on a condition and returns an error on failure #define Py_Try(F) {if (!(F)) return NULL;} /// assert which returns with an error on failure -#define Py_Assert(A,E,M) {if (!(A)) {PyErr_SetString(E, M); return NULL;}} +#define Py_Assert(A,E,M) {if (!(A)) {PyErr_SetString(E, M); return nullptr;}} /// This must be the first line of each PyC++ class @@ -458,10 +458,6 @@ BaseExport extern PyObject* BaseExceptionFreeCADAbort; #define PY_TRY try #define __PY_CATCH(R) \ - catch(Base::AbortException &e) \ - { \ - _Py_ErrorObj(R,Base::BaseExceptionFreeCADAbort,e.getPyObject());\ - } \ catch(Base::Exception &e) \ { \ auto pye = e.getPyExceptionType(); \ @@ -469,7 +465,7 @@ BaseExport extern PyObject* BaseExceptionFreeCADAbort; pye = Base::BaseExceptionFreeCADError; \ _Py_ErrorObj(R,pye,e.getPyObject()); \ } \ - catch(std::exception &e) \ + catch(const std::exception &e) \ { \ _Py_Error(R,Base::BaseExceptionFreeCADError,e.what()); \ } \ @@ -492,7 +488,7 @@ BaseExport extern PyObject* BaseExceptionFreeCADAbort; # define _PY_CATCH(R) __PY_CATCH(R) #endif // DONT_CATCH_CXX_EXCEPTIONS -#define PY_CATCH _PY_CATCH(return(NULL)) +#define PY_CATCH _PY_CATCH(return(nullptr)) /** Python helper class * This class encapsulate the Decoding of UTF8 to a python object. @@ -501,8 +497,9 @@ BaseExport extern PyObject* BaseExceptionFreeCADAbort; inline PyObject * PyAsUnicodeObject(const char *str) { // Returns a new reference, don't increment it! - PyObject *p = PyUnicode_DecodeUTF8(str,strlen(str),0); - if(!p) + Py_ssize_t len = Py_SAFE_DOWNCAST(strlen(str), size_t, Py_ssize_t); + PyObject *p = PyUnicode_DecodeUTF8(str, len, nullptr); + if (!p) throw Base::UnicodeError("UTF8 conversion failure at PyAsUnicodeString()"); return p; } diff --git a/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp b/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp index 3b1d750286..45e3576dde 100644 --- a/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp +++ b/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp @@ -556,7 +556,7 @@ void ViewProviderSketch::getCoordsOnSketchPlane(const SbVec3f &point, const SbVe // line Base::Vector3d R1(point[0],point[1],point[2]),RA(normal[0],normal[1],normal[2]); if (fabs(RN*RA) < FLT_EPSILON) - throw Base::DivisionByZeroError("View direction is parallel to sketch plane"); + throw Base::ZeroDivisionError("View direction is parallel to sketch plane"); // intersection point on plane Base::Vector3d S = R1 + ((RN * (R0-R1))/(RN*RA))*RA; @@ -597,7 +597,7 @@ bool ViewProviderSketch::mouseButtonPressed(int Button, bool pressed, const SbVe getCoordsOnSketchPlane(pos,normal,x,y); snapToGrid(x, y); } - catch (const Base::DivisionByZeroError&) { + catch (const Base::ZeroDivisionError&) { return false; } @@ -1041,7 +1041,7 @@ bool ViewProviderSketch::mouseMove(const SbVec2s &cursorPos, Gui::View3DInventor getCoordsOnSketchPlane(line.getPosition(),line.getDirection(),x,y); snapToGrid(x, y); } - catch (const Base::DivisionByZeroError&) { + catch (const Base::ZeroDivisionError&) { return false; } @@ -3491,7 +3491,7 @@ double ViewProviderSketch::getRotation(SbVec3f pos0, SbVec3f pos1) const return -atan2((y1-y0),(x1-x0))*180/M_PI; } - catch (const Base::DivisionByZeroError&) { + catch (const Base::ZeroDivisionError&) { return 0; } } diff --git a/src/Tools/generateTemplates/templateClassPyExport.py b/src/Tools/generateTemplates/templateClassPyExport.py index 13fd564532..0881009b88 100644 --- a/src/Tools/generateTemplates/templateClassPyExport.py +++ b/src/Tools/generateTemplates/templateClassPyExport.py @@ -568,11 +568,6 @@ PyObject * @self.export.Name@::staticCallback_@i.Name@ (PyObject *self, PyObject - return ret; } // Please sync the following catch implementation with PY_CATCH - catch(Base::AbortException &e) - { - PyErr_SetObject(Base::BaseExceptionFreeCADAbort,e.getPyObject()); - return nullptr; - } catch(Base::Exception &e) { auto pye = e.getPyExceptionType(); @@ -581,7 +576,7 @@ PyObject * @self.export.Name@::staticCallback_@i.Name@ (PyObject *self, PyObject PyErr_SetObject(pye,e.getPyObject()); return nullptr; } - catch(std::exception &e) + catch(const std::exception &e) { PyErr_SetString(Base::BaseExceptionFreeCADError,e.what()); return nullptr; @@ -591,11 +586,6 @@ PyObject * @self.export.Name@::staticCallback_@i.Name@ (PyObject *self, PyObject // The exception text is already set return nullptr; } - catch(const char *e) - { - PyErr_SetString(Base::BaseExceptionFreeCADError,e); - return nullptr; - } #ifndef DONT_CATCH_CXX_EXCEPTIONS catch(...) { @@ -741,11 +731,6 @@ PyObject *@self.export.Name@::_getattr(const char *attr) // __getattr__ functi PyObject *r = getCustomAttributes(attr); if(r) return r; } // Please sync the following catch implementation with PY_CATCH - catch(Base::AbortException &e) - { - PyErr_SetObject(Base::BaseExceptionFreeCADAbort,e.getPyObject()); - return nullptr; - } catch(Base::Exception &e) { auto pye = e.getPyExceptionType(); @@ -754,7 +739,7 @@ PyObject *@self.export.Name@::_getattr(const char *attr) // __getattr__ functi PyErr_SetObject(pye,e.getPyObject()); return nullptr; } - catch(std::exception &e) + catch(const std::exception &e) { PyErr_SetString(Base::BaseExceptionFreeCADError,e.what()); return nullptr; @@ -764,11 +749,6 @@ PyObject *@self.export.Name@::_getattr(const char *attr) // __getattr__ functi // The exception text is already set return nullptr; } - catch(const char *e) - { - PyErr_SetString(Base::BaseExceptionFreeCADError,e); - return nullptr; - } #ifndef DONT_CATCH_CXX_EXCEPTIONS catch(...) { @@ -801,11 +781,6 @@ int @self.export.Name@::_setattr(const char *attr, PyObject *value) // __setattr else if (r == -1) return -1; } // Please sync the following catch implementation with PY_CATCH - catch(Base::AbortException &e) - { - PyErr_SetObject(Base::BaseExceptionFreeCADAbort,e.getPyObject()); - return -1; - } catch(Base::Exception &e) { auto pye = e.getPyExceptionType(); @@ -814,7 +789,7 @@ int @self.export.Name@::_setattr(const char *attr, PyObject *value) // __setattr PyErr_SetObject(pye,e.getPyObject()); return -1; } - catch(std::exception &e) + catch(const std::exception &e) { PyErr_SetString(Base::BaseExceptionFreeCADError,e.what()); return -1; @@ -824,11 +799,6 @@ int @self.export.Name@::_setattr(const char *attr, PyObject *value) // __setattr // The exception text is already set return -1; } - catch(const char *e) - { - PyErr_SetString(Base::BaseExceptionFreeCADError,e); - return -1; - } #ifndef DONT_CATCH_CXX_EXCEPTIONS catch(...) {