|
There are still a few PyErr_BadInternalCall() and PyErr_BadArgument() in:
|
|
Oh, this change breaks the sqlite module which expect that PyUnicode_AsUTF8AndSize() raises an excpetion if its argument is not a Unicode object: |
|
I wrote PR #14386 to change the sqlite3 module. |
When Python is built in release mode (when Py_DEBUG is not defined), remove debug/sanity runtime checks in unicodeobject.c (checks using PyErr_BadInternalCall() or PyErr_BadArgument()).
|
Wouldn't it be simpler to replace those checks by assertions then? For example assert(PyUnicode_Check(from));
assert(PyUnicode_Check(to));instead of #ifdef Py_DEBUG
if (!PyUnicode_Check(from) || !PyUnicode_Check(to)) {
PyErr_BadInternalCall();
return -1;
}
#endif |
That's a big behavior difference: currrently, it's possible to catch and handle the exception. If we start using assert(), the process is killed immediately by SIGABRT using abort(). I prefer to not change too many things at once :-) |
Given that the default is to build without debug info, this PR is a big behavior difference: explicit checks are removed.
I actually consider that a good thing. It's easier to debug a But I'm not going to argue this further, it was just a suggestion. |
Almost everybody use release builds of Python. So yes, this PR changes the default behavior. It's a deliberate choice: https://bugs.python.org/issue37406 I was talking about no changing the behavior of the debug build. |
|
There were some problems with the Azure CI. Can you close and re-open such that it gets tested again? |
Yes, it's possible but it's completely meaningless to do that. It indicates a serious bug in the code, not something that should be handled in a |
|
There are discussions against this change in https://bugs.python.org/issue37406 My plan is to finish an article to explain the problem I am trying to solve here. |
There was a problem hiding this comment.
I don't think that the code paths using PyErr_BadArgument instead of PyErr_BadInternalCall are meant for debug builds only. It's raising a TypeError and catching that is a reasonable thing to do. So this could cause applications that work fine in debug to crash in non-debug mode. That would be pretty bad.
So there are two ways to fix this inconsistency:
PyErr_BadArgument by PyErr_BadInternalCall inside those #ifdef Py_DEBUG blocks.#ifdef Py_DEBUG for code already using PyErr_BadInternalCallI would vote for option 1.
|
And this absolutely deserves a |
|
@jdemeyer: Please discuss https://bugs.python.org/issue37406 since more people are involved there, and I prefer to not have the discussion at two different places. |
When Python is built in release mode (when Py_DEBUG is not defined),
remove debug/sanity runtime checks in unicodeobject.c (checks using
PyErr_BadInternalCall() or PyErr_BadArgument()).
https://bugs.python.org/issue37406