From 21661de66a889d4c8b4973c8313f59938396dea1 Mon Sep 17 00:00:00 2001 From: "M. Gronle" Date: Wed, 3 Feb 2021 22:36:14 +0100 Subject: [PATCH] refactoring of breakpoint management during debugging. If a breakpoint is considered to be in an empty or comment line, an error text is printed and the breakpoints is deleted. --- CMakeLists.txt | 2 +- Qitom/python/pythonEngine.cpp | 322 +++++++++++++++++++++++-------- Qitom/python/pythonEngine.h | 13 +- Qitom/ui/dialogEditBreakpoint.ui | 8 +- itoDebugger.py | 10 +- 5 files changed, 263 insertions(+), 92 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f88aa268d..a0c6ce1cf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -21,7 +21,7 @@ cmake_minimum_required(VERSION 3.1) project(itom) - + message(STATUS "--------------- PROJECT itom -----------------") set(CMAKE_CXX_STANDARD 11) diff --git a/Qitom/python/pythonEngine.cpp b/Qitom/python/pythonEngine.cpp index bcb6cbf92..2211ca440 100644 --- a/Qitom/python/pythonEngine.cpp +++ b/Qitom/python/pythonEngine.cpp @@ -1863,11 +1863,7 @@ ito::RetVal PythonEngine::debugFunction(PyObject *callable, PyObject *argTuple, } //!< submit all breakpoints - ito::RetVal retValueBp = submitAllBreakpointsToDebugger(); - if (retValueBp.containsError()) - { - std::cerr << retValueBp.errorMessage() << "\n" << std::endl; - } + submitAllBreakpointsToDebugger(); //!< setup connections for live-changes in breakpoints setupBreakPointDebugConnections(); @@ -1979,11 +1975,7 @@ ito::RetVal PythonEngine::debugFile(const QString &pythonFileName) } //!< submit all breakpoints - ito::RetVal retValueBp = submitAllBreakpointsToDebugger(); - if (retValueBp.containsError()) - { - std::cerr << retValueBp.errorMessage() << "\n" << std::endl; - } + submitAllBreakpointsToDebugger(); //!< setup connections for live-changes in breakpoints setupBreakPointDebugConnections(); @@ -2094,11 +2086,7 @@ ito::RetVal PythonEngine::debugString(const QString &command) } //!< submit all breakpoints - ito::RetVal retValueBp = submitAllBreakpointsToDebugger(); - if (retValueBp.containsError()) - { - std::cerr << retValueBp.errorMessage() << "\n" << std::endl; - } + submitAllBreakpointsToDebugger(); //!< setup connections for live-changes in breakpoints setupBreakPointDebugConnections(); @@ -2410,83 +2398,136 @@ void PythonEngine::pythonCodeCheck(const QString &code, const QString &filename, } } -//---------------------------------------------------------------------------------------------------------------------------------- +//------------------------------------------------------------------------------------- //!< submits all breakpoints to the debugger. This should be called before code is debugged. -ito::RetVal PythonEngine::submitAllBreakpointsToDebugger() +void PythonEngine::submitAllBreakpointsToDebugger() { //when calling this method, the Python GIL must already be locked - QList bp = bpModel->getBreakpoints(); - QList::iterator it; + QModelIndexList bpIndices = bpModel->getAllBreakPointIndizes(); + QModelIndexList bpIndicesToDelete; int pyBpNumber; - QModelIndex modelIndex; ito::RetVal retVal; - ito::RetVal retValTemp; - for (it = bp.begin(); it != bp.end(); ++it) + foreach(const QModelIndex &idx, bpIndices) { - if (it->pythonDbgBpNumber == -1) + const BreakPointItem &bp = bpModel->getBreakPoint(idx); + + if (bp.pythonDbgBpNumber == -1) { - retValTemp = pythonAddBreakpoint(it->filename, it->lineIdx, it->enabled, it->temporary, it->condition, it->ignoreCount, pyBpNumber); - if (retValTemp == ito::retOk) + retVal = pythonAddBreakpoint(bp, pyBpNumber); + + if (retVal == ito::retOk) { - bpModel->setPyBpNumber(*it, pyBpNumber); + bpModel->setPyBpNumber(bp, pyBpNumber); } else { - bpModel->setPyBpNumber(*it, -1); - retVal += retValTemp; + if (retVal.hasErrorMessage()) + { + std::cerr << retVal.errorMessage() << "\n"; + } + + if (retVal.errorCode() == DbgErrorInvalidBp) + { + bpIndicesToDelete << idx; + std::cerr << "The breakpoint will be deleted.\n" << std::endl; + } + else + { + std::cerr << std::endl; + } + + bpModel->setPyBpNumber(bp, -1); } } } - return retVal; + if (bpIndicesToDelete.size() > 0) + { + bpModel->deleteBreakPoints(bpIndicesToDelete); + } } -//---------------------------------------------------------------------------------------------------------------------------------- -ito::RetVal PythonEngine::pythonAddBreakpoint(const QString &filename, const int lineno, const bool enabled, const bool temporary, const QString &condition, const int ignoreCount, int &pyBpNumber) +//------------------------------------------------------------------------------------- +ito::RetVal PythonEngine::pythonAddBreakpoint( + const BreakPointItem &breakpoint, + int &pyBpNumber) { RetVal retval; //when calling this method, the Python GIL must already be locked - PyObject *result = NULL; - + PyObject *result = nullptr; + int lineNo = breakpoint.lineIdx + 1; pyBpNumber = -1; - if (itomDbgInstance == NULL) + if (itomDbgInstance == nullptr) { - retval += RetVal(retError, 0, "Debugger not available"); + retval += RetVal(retError, DbgErrorNo, "Debugger not available"); } else { - PyObject *PyEnabled = enabled ? Py_True : Py_False; - PyObject *PyTemporary = temporary ? Py_True : Py_False; + PyObject *pyEnabled = breakpoint.enabled ? Py_True : Py_False; + PyObject *pyTemporary = breakpoint.temporary ? Py_True : Py_False; - if (condition == "") + if (breakpoint.condition == "") { - result = PyObject_CallMethod(itomDbgInstance, "addNewBreakPoint", "siOOOi", filename.toUtf8().data(), lineno+1, PyEnabled, PyTemporary, Py_None, ignoreCount); + result = PyObject_CallMethod( + itomDbgInstance, + "addNewBreakPoint", + "siOOOi", + breakpoint.filename.toUtf8().data(), + lineNo, + pyEnabled, + pyTemporary, + Py_None, + breakpoint.ignoreCount); } else { - result = PyObject_CallMethod(itomDbgInstance, "addNewBreakPoint", "siOOsi", filename.toUtf8().data(), lineno+1, PyEnabled, PyTemporary, condition.toLatin1().data(), ignoreCount); + result = PyObject_CallMethod( + itomDbgInstance, + "addNewBreakPoint", + "siOOsi", + breakpoint.filename.toUtf8().data(), + lineNo, + pyEnabled, + pyTemporary, + breakpoint.condition.toLatin1().data(), + breakpoint.ignoreCount); } - if (result == NULL) + if (result == nullptr) { //this is an exception case that should not occur under normal circumstances - std::cerr << tr("Adding breakpoint to file '%1', line %2 failed in Python debugger.").arg(filename).arg(lineno + 1).toLatin1().constData() << "\n" << std::endl; - printPythonErrorWithoutTraceback(); //traceback is sense-less, since the traceback is in itoDebugger.py only! - retval += RetVal(retError, 0, tr("Adding breakpoint to file '%1', line %2 failed in Python debugger.").arg(filename).arg(lineno + 1).toLatin1().constData()); + std::cerr << tr("Adding breakpoint to file '%1', line %2 failed in Python debugger.") + .arg(breakpoint.filename).arg(lineNo).toLatin1().constData() << "\n" << std::endl; + + //traceback is sense-less, since the traceback is in itoDebugger.py only! + printPythonErrorWithoutTraceback(); PyErr_Clear(); + + retval += RetVal( + retError, + DbgErrorOther, + tr("Adding breakpoint to file '%1', line %2 failed in Python debugger.") + .arg(breakpoint.filename).arg(lineNo).toLatin1().constData()); } else { if (PyLong_Check(result)) { + // ok long retNumber = PyLong_AsLong(result); + if (retNumber < 0) { pyBpNumber = -1; - retval += RetVal::format(retError, 0, tr("Adding breakpoint to file '%s', line %i failed in Python debugger (invalid breakpoint id).").toLatin1().data(), - filename.toLatin1().data(), lineno + 1); + retval += RetVal::format( + retError, + DbgErrorOther, + tr("Adding breakpoint to file '%s', line %i failed in Python debugger (invalid breakpoint id).").toLatin1().data(), + breakpoint.filename.toLatin1().data(), + lineNo + ); } else { @@ -2498,87 +2539,154 @@ ito::RetVal PythonEngine::pythonAddBreakpoint(const QString &filename, const int { bool ok; QByteArray error = PythonQtConversion::PyObjGetString(result, true, ok).toLatin1(); + if (ok) { - retval += RetVal(retError, 0, error.data()); + if (error.startsWith("_")) + { + error = error.remove(0, 1); + retval += RetVal(retError, DbgErrorOther, error.constData()); + } + else + { + retval += RetVal(retError, DbgErrorInvalidBp, error.constData()); + } } else { - retval += RetVal::format(retError, 0, tr("Adding breakpoint to file '%s', line %i in Python debugger returned unknown error string").toLatin1().data(), - filename.toLatin1().data(), lineno + 1); + retval += RetVal::format( + retError, + DbgErrorOther, + tr("Adding breakpoint to file '%s', line %i in Python debugger returned unknown error string.").toLatin1().data(), + breakpoint.filename.toLatin1().data(), + lineNo + ); } } } Py_XDECREF(result); - result = NULL; + result = nullptr; } return retval; } -//---------------------------------------------------------------------------------------------------------------------------------- -ito::RetVal PythonEngine::pythonEditBreakpoint(const int pyBpNumber, const QString &filename, const int lineno, const bool enabled, const bool temporary, const QString &condition, const int ignoreCount) +//------------------------------------------------------------------------------------- +ito::RetVal PythonEngine::pythonEditBreakpoint(const int pyBpNumber, const BreakPointItem &newBreakpoint) { RetVal retval; //when calling this method, the Python GIL must already be locked - PyObject *result = NULL; + PyObject *result = nullptr; + int lineno = newBreakpoint.lineIdx + 1; - if (itomDbgInstance == NULL) + if (itomDbgInstance == nullptr) { - retval += RetVal(retError, 0, tr("Debugger not available").toLatin1().data()); + retval += RetVal(retError, DbgErrorOther, tr("Debugger not available").toLatin1().data()); } else if (pyBpNumber >= 0) { - PyObject *PyEnabled = enabled ? Py_True : Py_False; - PyObject *PyTemporary = temporary ? Py_True : Py_False; - - if (condition == "") - { - result = PyObject_CallMethod(itomDbgInstance, "editBreakPoint", "isiOOOi", pyBpNumber, filename.toUtf8().data(), lineno+1, PyEnabled, PyTemporary, Py_None, ignoreCount); + PyObject *pyEnabled = newBreakpoint.enabled ? Py_True : Py_False; + PyObject *pyTemporary = newBreakpoint.temporary ? Py_True : Py_False; + + if (newBreakpoint.condition == "") + { + result = PyObject_CallMethod( + itomDbgInstance, + "editBreakPoint", + "isiOOOi", + pyBpNumber, + newBreakpoint.filename.toUtf8().data(), + lineno, + pyEnabled, + pyTemporary, + Py_None, + newBreakpoint.ignoreCount + ); } else { - result = PyObject_CallMethod(itomDbgInstance, "editBreakPoint", "isiOOsi", pyBpNumber, filename.toUtf8().data(), lineno+1, PyEnabled, PyTemporary, condition.toLatin1().data(), ignoreCount); + result = PyObject_CallMethod( + itomDbgInstance, + "editBreakPoint", + "isiOOsi", + pyBpNumber, + newBreakpoint.filename.toUtf8().data(), + lineno, + pyEnabled, + pyTemporary, + newBreakpoint.condition.toLatin1().data(), + newBreakpoint.ignoreCount + ); } - if (result == NULL) + if (result == nullptr) { //this is an exception case that should not occure under normal circumstances std::cerr << "Error while editing breakpoint in debugger." << "\n" << std::endl; + printPythonErrorWithoutTraceback(); //traceback is sense-less, since the traceback is in itoDebugger.py only! - retval += RetVal(retError, 0, tr("Exception raised while editing breakpoint in debugger.").toLatin1().data()); + PyErr_Clear(); + + retval += RetVal( + retError, + DbgErrorOther, + tr("Exception raised while editing breakpoint in debugger.").toLatin1().data() + ); } else if (PyLong_Check(result)) { long val = PyLong_AsLong(result); + if (val != 0) { - retval += RetVal::format(retError, 0, tr("Editing breakpoint (file '%s', line %i) in Python debugger returned error code %i").toLatin1().data(), - filename.toLatin1().data(), lineno + 1, val); + retval += RetVal::format( + retError, DbgErrorOther, + tr("Editing breakpoint (file '%s', line %i) in Python debugger returned error code %i").toLatin1().data(), + newBreakpoint.filename.toLatin1().data(), + lineno, + val + ); } } else { bool ok; QByteArray error = PythonQtConversion::PyObjGetString(result, true, ok).toLatin1(); + if (ok) { - retval += RetVal(retError, 0, error.data()); + if (error.startsWith("_")) + { + error = error.remove(0, 1); + retval += RetVal(retError, DbgErrorOther, error.constData()); + } + else + { + retval += RetVal(retError, DbgErrorInvalidBp, error.constData()); + } } else { - retval += RetVal::format(retError, 0, tr("Editing breakpoint (file '%s', line %i) in Python debugger returned unknown error string").toLatin1().data(), - filename.toLatin1().data(), lineno + 1); + retval += RetVal::format( + retError, + DbgErrorOther, + tr("Editing breakpoint (file '%s', line %i) in Python debugger returned unknown error string").toLatin1().data(), + newBreakpoint.filename.toLatin1().data(), + lineno); } } Py_XDECREF(result); - result = NULL; + result = nullptr; } else { - retval += RetVal::format(retError, 0, tr("Breakpoint in file '%s', line %i could not be edited since it has no valid Python breakpoint number (maybe a comment or blank line in script)").toLatin1().data(), - filename.toLatin1().data(), lineno + 1); + retval += RetVal::format( + retError, + 0, + tr("Breakpoint in file '%s', line %i could not be edited since it has no valid Python breakpoint number (maybe a comment or blank line in script)").toLatin1().data(), + newBreakpoint.filename.toLatin1().data(), + lineno); } return retval; @@ -3228,7 +3336,7 @@ void PythonEngine::enqueueDbgCmd(ito::tPythonDbgCmd dbgCmd) } } -//---------------------------------------------------------------------------------------------------------------------------------- +//------------------------------------------------------------------------------------- ito::tPythonDbgCmd PythonEngine::dequeueDbgCmd() { tPythonDbgCmd cmd = pyDbgNone; @@ -3244,7 +3352,7 @@ ito::tPythonDbgCmd PythonEngine::dequeueDbgCmd() return cmd; } -//---------------------------------------------------------------------------------------------------------------------------------- +//------------------------------------------------------------------------------------- bool PythonEngine::DbgCommandsAvailable() { bool ret; @@ -3254,7 +3362,7 @@ bool PythonEngine::DbgCommandsAvailable() return ret; } -//---------------------------------------------------------------------------------------------------------------------------------- +//------------------------------------------------------------------------------------- void PythonEngine::clearDbgCmdLoop() { dbgCmdMutex.lock(); @@ -3262,17 +3370,44 @@ void PythonEngine::clearDbgCmdLoop() dbgCmdMutex.unlock(); } -//---------------------------------------------------------------------------------------------------------------------------------- +//------------------------------------------------------------------------------------- void PythonEngine::breakPointAdded(BreakPointItem bp, int row) { int pyBpNumber; PyGILState_STATE gstate = PyGILState_Ensure(); - pythonAddBreakpoint(bp.filename, bp.lineIdx, bp.enabled, bp.temporary, bp.condition, bp.ignoreCount, pyBpNumber); + RetVal ret = pythonAddBreakpoint(bp, pyBpNumber); PyGILState_Release(gstate); - bpModel->setPyBpNumber(bp, pyBpNumber); + + if (ret.containsError()) + { + if (ret.hasErrorMessage()) + { + std::cerr << ret.errorMessage() << "\n"; + } + else + { + std::cerr << "unknown error while adding breakpoint\n"; + } + + if (ret.errorCode() == DbgErrorInvalidBp) + { + std::cerr << "The breakpoint will be deleted.\n" << std::endl; + + QModelIndex idx = bpModel->getFirstBreakPointIndex(bp.filename, bp.lineIdx); + bpModel->deleteBreakPoint(idx); + } + else + { + std::cerr << std::endl; + } + } + else + { + bpModel->setPyBpNumber(bp, pyBpNumber); + } } -//---------------------------------------------------------------------------------------------------------------------------------- +//------------------------------------------------------------------------------------- void PythonEngine::breakPointDeleted(QString /*filename*/, int /*lineNo*/, int pyBpNumber) { PyGILState_STATE gstate = PyGILState_Ensure(); @@ -3286,21 +3421,40 @@ void PythonEngine::breakPointDeleted(QString /*filename*/, int /*lineNo*/, int p PyGILState_Release(gstate); } -//---------------------------------------------------------------------------------------------------------------------------------- +//------------------------------------------------------------------------------------- void PythonEngine::breakPointChanged(BreakPointItem /*oldBp*/, ito::BreakPointItem newBp) { PyGILState_STATE gstate = PyGILState_Ensure(); - ito::RetVal ret = pythonEditBreakpoint(newBp.pythonDbgBpNumber, newBp.filename, newBp.lineIdx, newBp.enabled, newBp.temporary, newBp.condition, newBp.ignoreCount); + ito::RetVal ret = pythonEditBreakpoint(newBp.pythonDbgBpNumber, newBp); if (ret.containsError()) { - std::cerr << (ret.hasErrorMessage() ? ret.errorMessage() : "unknown error while editing breakpoint") << "\n" << std::endl; + if (ret.hasErrorMessage()) + { + std::cerr << ret.errorMessage() << "\n"; + } + else + { + std::cerr << "unknown error while editing breakpoint\n"; + } + + if (ret.errorCode() == DbgErrorInvalidBp) + { + std::cerr << "The breakpoint will be deleted.\n" << std::endl; + + QModelIndex idx = bpModel->getFirstBreakPointIndex(newBp.filename, newBp.lineIdx); + bpModel->deleteBreakPoint(idx); + } + else + { + std::cerr << std::endl; + } } PyGILState_Release(gstate); } -//---------------------------------------------------------------------------------------------------------------------------------- +//------------------------------------------------------------------------------------- ito::RetVal PythonEngine::setupBreakPointDebugConnections() { connect(bpModel, SIGNAL(breakPointAdded(BreakPointItem,int)), this, SLOT(breakPointAdded(BreakPointItem,int))); @@ -3309,7 +3463,7 @@ ito::RetVal PythonEngine::setupBreakPointDebugConnections() return RetVal(retOk); } -//---------------------------------------------------------------------------------------------------------------------------------- +//------------------------------------------------------------------------------------- ito::RetVal PythonEngine::shutdownBreakPointDebugConnections() { disconnect(bpModel, SIGNAL(breakPointAdded(BreakPointItem,int)), this, SLOT(breakPointAdded(BreakPointItem,int))); diff --git a/Qitom/python/pythonEngine.h b/Qitom/python/pythonEngine.h index 8c0d9df12..b8b5007d2 100644 --- a/Qitom/python/pythonEngine.h +++ b/Qitom/python/pythonEngine.h @@ -175,6 +175,13 @@ class PythonEngine : public QObject void connectNotify(const QMetaMethod &signal); + enum DebuggerErrorCode + { + DbgErrorNo = 0, + DbgErrorInvalidBp = 1, // the breakpoint candidate could not be set and will be deleted + DbgErrorOther = 2 // any other error + }; + private: enum DictUpdateFlag { @@ -215,10 +222,10 @@ class PythonEngine : public QObject ito::RetVal pythonStateTransition(tPythonTransitions transition, bool immediate = true); //methods for breakpoint - ito::RetVal pythonAddBreakpoint(const QString &filename, const int lineno, const bool enabled, const bool temporary, const QString &condition, const int ignoreCount, int &pyBpNumber); - ito::RetVal pythonEditBreakpoint(const int pyBpNumber, const QString &filename, const int lineno, const bool enabled, const bool temporary, const QString &condition, const int ignoreCount); + ito::RetVal pythonAddBreakpoint(const BreakPointItem &breakpoint, int &pyBpNumber); + ito::RetVal pythonEditBreakpoint(const int pyBpNumber, const BreakPointItem &newBreakpoint); ito::RetVal pythonDeleteBreakpoint(const int pyBpNumber); - ito::RetVal submitAllBreakpointsToDebugger(); + void submitAllBreakpointsToDebugger(); ito::RetVal autoReloaderCheck(); diff --git a/Qitom/ui/dialogEditBreakpoint.ui b/Qitom/ui/dialogEditBreakpoint.ui index aaca112ec..9b32ba9b2 100644 --- a/Qitom/ui/dialogEditBreakpoint.ui +++ b/Qitom/ui/dialogEditBreakpoint.ui @@ -10,7 +10,7 @@ 0 0 396 - 286 + 222 @@ -214,6 +214,12 @@ + + txtCondition + spinBoxIgnoreCount + checkTemporaryBP + checkEnabled + diff --git a/itoDebugger.py b/itoDebugger.py index 29ffdfe9e..3c4ec00da 100644 --- a/itoDebugger.py +++ b/itoDebugger.py @@ -267,7 +267,9 @@ def defaultFile(self): def addNewBreakPoint(self, filename, lineno, enabled, temporary, condition, ignoreCount): """Adds breakpoint to list of breakpoints. - Return breakpoint ID (int) or error string (str) + Return breakpoint ID (int) or error string (str). + + This method should not raise an exception. """ if not filename: filename = self.defaultFile() @@ -280,7 +282,8 @@ def addNewBreakPoint(self, filename, lineno, enabled, temporary, condition, except IndexError as err: return "Cannot add breakpoint: " + str(err) except UnicodeDecodeError as err: - return "Cannot add breakpoint: " + str(err) + # The _ in the error string indicates to not delete the break point. + return "_Cannot add breakpoint: " + str(err) if line > 0: # now set the break point @@ -310,7 +313,8 @@ def editBreakPoint(self, bpNumber, filename, lineno, enabled, temporary, c except IndexError as err: return "Cannot edit breakpoint: " + str(err) except UnicodeDecodeError as err: - return "Cannot edit breakpoint: " + str(err) + # The _ in the error string indicates to not delete the break point. + return "_Cannot edit breakpoint: " + str(err) if line > 0: #input values are ok, break point can be edited