Le dim. 31 mars 2019 à 01:49, Steve Dower <steve.do...@python.org> a écrit : > Here is my first review of https://www.python.org/dev/peps/pep-0587/ and > in general I think it's very good.
Ah nice, that's a good start :-) Thanks for reviewing it. Your email is long, and answer makes it even longer, so I will reply in multiple emails. > > ``PyWideCharList`` is a list of ``wchar_t*`` strings. > > I always forget whether "const" is valid in C99, but if it is, can we > make this a list of const strings? Short answer: no :-( This structure mostly exists to simplify the implementation. Sadly, "const PyWideCharList" doesn't automatically make PyWideCharList.items an array of "const wchar_t*". I tried some hacks to have an array of const strings... but it would be very complicated and not natural at all in C. Sadly, it's way more simple to have "wchar_t*" in practice. > I also prefer a name like ``PyWideStringList``, since that's what it is > (the other places we use WideChar in the C API refer only to a single > string, as far as I'm aware). I'm fine with this name. > > ``PyInitError`` is a structure to store an error message or an exit code > > for the Python Initialization. > > I love this struct! Currently it's private, but I wonder whether it's > worth making it public as PyError (or PyErrorInfo)? The PEP 587 makes the structure public, but I'm not sure about calling it PyError because PyInitError also allows to exit Python with an exit status which is something specific to the initialization. If you want to use a structure to reporting errors, I would prefer to add a new simpler PyError structure to only report an error message, but never exit Python. PyInitError use case is really specific to Python initialization. Moreover, the API is inefficient since it is returned by copy, not by reference. That's fine for Python initialization which only happens once and is not part of "hot code". I'm not sure if PyError would need to store the C function name where the error is triggered. Usually, we try hard to hide Python internals to the user. > > * ``exitcode`` (``int``): if greater or equal to zero, argument passed to > > ``exit()`` > > Windows is likely to need/want negative exit codes, as many system > errors are represented as 0x80000000|(source of error)|(specific code). Hum, int was used in Python 3.6 code base. We change change PyInitError.exitcode type to DWORD on Windows, but use int on Unix. We can add a private field to check if the error is an error message or an exit code. Or maybe check if the error message is NULL or not. Py_INIT_ERR(MSG) must never be called with Py_INIT_ERR(NULL) and it should be called with a static string, not with a dynamically allocated string (since the API doesn't allow to release memory). > > * ``user_err`` (int): if non-zero, the error is caused by the user > > configuration, otherwise it's an internal Python error. > > Maybe we could just encode this as "positive exitcode is user error, > negative is internal error"? I'm pretty sure struct return values are > passed by reference in most C calling conventions, so the size of the > struct isn't a big deal, but without a proper bool type it may look like > this is a second error code (like errno/winerror in a few places). Honestly, I'm not sure that we really have to distinguish "user error" and "internal error". It's an old debate about calling abort()/DebugBreak() or not. It seems like most users are annoyed by getting a coredump on Unix when abort() is called. Maybe we should just remove Py_INIT_USER_ERR(), always use Py_INIT_ERR(), and never call abort()/DebugBreak() in Py_ExitInitError(). Or does anyone see a good reason to trigger a debugger on an initialization error? See https://bugs.python.org/issue19983 discussion: "When interrupted during startup, Python should not call abort() but exit()" Note: I'm not talking about Py_FatalError() here, this one will not change. Victor _______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
