classification
Title: Argument Clinic: broken support for 'O!'
Type: behavior Stage: resolved
Components: Build, Documentation Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: larry Nosy List: larry, python-dev, serhiy.storchaka
Priority: normal Keywords:

Created on 2014-01-06 13:12 by serhiy.storchaka, last changed 2014-01-07 20:18 by larry. This issue is now closed.

Files
File name Uploaded Description Edit
spammodule.c serhiy.storchaka, 2014-01-06 13:55 An example
larry.argument.clinic.o-bang.rethink.diff.1.txt larry, 2014-01-07 13:20 review
Messages (9)
msg207430 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-01-06 13:12
First, the documentation says that syntax for 'O!' conversion is
object(type='name_of_Python_type'), but actually the type argument should be the name of C structure which represents Python type. I.e. object(type='list') is rejected, and object(type='PyList_Type') is accepted.

Second, from this declaration

/*[clinic]
module spam
spam.ham

    items: object(type='PyList_Type')
    /

[clinic]*/

Argument Clinic generates following parsing code:

    if (!PyArg_ParseTuple(args,
        "O!:ham",
        PyList_Type, &items))
        goto exit;

It is wrong, should be &PyList_Type.
msg207431 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-06 13:47
So a documentation error and having to manually specify "&" at the front of your string means it's "broken"?

Nevertheless, I'll take a look at it.
msg207432 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-01-06 13:50
You can't manually specify "&" at the front of the name.
msg207455 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-06 16:57
Actually, the documentation (the "howto") states:

Note that object() must explicitly support each Python type you specify for the type argument. Currently it only supports str. It should be easy to add more, just edit Tools/clinic/clinic.py, search for O! in the text, and add more entries to the dict mapping types to strings just above it.

Maybe this is a bad API.  But it's not broken.
msg207532 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-07 13:20
Attached is a new, simpler approach for supporting O!.  The object() converter now takes two arguments:
  * type, which is the type you want the parameter declared as
    (e.g. "PyUnicodeObject *")
  * subclass_of, which is the PyTypeObject you want to enforce the
    value is an instance of (e.g. "&PyUnicode_Type")
The old approach was kind of a lovely idea, but was too complicated, and it would have meant registering any new type (like third-party types).  This is less convenient, but simple.  And it would lend itself well to making a custom converter if you used it a lot.

I'll write some documentation for it now, but I wanted to post the code so I could get a review.

p.s. Why was is marked "crash" and "release blocker"?  That's very inaccurate.  And assigned to docs?  WTH?
msg207538 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-01-07 13:57
It is marked "crash" because Argument Clinic generates code which crashes. It is assigned to docs because this feature documentation is misleading.
msg207546 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-07 15:06
There are lots of ways you can crash Python by giving erroneous input to Argument Clinic.  Clinic has no visibility into the C type system, so it has no way of verifying whether or not the type objects you pass in are correct.  That's unfixable and not really interesting.
msg207587 - (view) Author: Roundup Robot (python-dev) Date: 2014-01-07 20:13
New changeset ddb5cd3e0860 by Larry Hastings in branch 'default':
Issue #20141: Improved Argument Clinic's support for the PyArg_Parse "O!"
http://hg.python.org/cpython/rev/ddb5cd3e0860
msg207588 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-07 20:18
Argument Clinic's support for "O!" is now simpler and more flexible.  It's not as convenient as the previous API, but that API wasn't really sustainable.  I'm assuming this fixes your problem; if not please open a new issue.
History
Date User Action Args
2014-01-07 20:18:45larrysetstatus: open -> closed
resolution: fixed
messages: + msg207588

stage: patch review -> resolved
2014-01-07 20:13:45python-devsetnosy: + python-dev
messages: + msg207587
2014-01-07 15:06:32larrysetmessages: + msg207546
2014-01-07 13:57:09serhiy.storchakasetmessages: + msg207538
2014-01-07 13:20:48larrysetfiles: + larry.argument.clinic.o-bang.rethink.diff.1.txt
priority: release blocker -> normal
type: crash -> behavior

assignee: docs@python -> larry

nosy: - docs@python
messages: + msg207532
stage: patch review
2014-01-07 09:29:52serhiy.storchakalinkissue20159 dependencies
2014-01-06 17:08:14serhiy.storchakalinkissue20148 dependencies
2014-01-06 16:57:11larrysetmessages: + msg207455
2014-01-06 13:55:57serhiy.storchakasetfiles: + spammodule.c
2014-01-06 13:50:27serhiy.storchakasetmessages: + msg207432
2014-01-06 13:47:22larrysetmessages: + msg207431
2014-01-06 13:12:14serhiy.storchakacreate