[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Python-Dev] New Python Initialization API

Le dim. 31 mars 2019 ? 01:49, Steve Dower <steve.dower at 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

> > * ``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