Repository navigation
Argument Clinic: broken support for 'O!' #64340
Description
| BPO | 20141 |
|---|---|
| Nosy | @larryhastings, @serhiy-storchaka |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/larryhastings'
closed_at = <Date 2014-01-07.20:18:45.031>
created_at = <Date 2014-01-06.13:12:14.194>
labels = ['type-bug', 'build', 'docs']
title = "Argument Clinic: broken support for 'O!'"
updated_at = <Date 2014-01-07.20:18:45.030>
user = 'https://github.com/serhiy-storchaka'bugs.python.org fields:
activity = <Date 2014-01-07.20:18:45.030>
actor = 'larry'
assignee = 'larry'
closed = True
closed_date = <Date 2014-01-07.20:18:45.031>
closer = 'larry'
components = ['Build', 'Documentation']
creation = <Date 2014-01-06.13:12:14.194>
creator = 'serhiy.storchaka'
dependencies = []
files = ['33325', '33341']
hgrepos = []
issue_num = 20141
keywords = []
message_count = 9.0
messages = ['207430', '207431', '207432', '207455', '207532', '207538', '207546', '207587', '207588']
nosy_count = 3.0
nosy_names = ['larry', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue20141'
versions = ['Python 3.4']
Activity
serhiy-storchaka commentedon Jan 6, 2014
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
[clinic]*/
Argument Clinic generates following parsing code:
It is wrong, should be &PyList_Type.
larryhastings commentedon Jan 6, 2014
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.
serhiy-storchaka commentedon Jan 6, 2014
You can't manually specify "&" at the front of the name.
larryhastings commentedon Jan 6, 2014
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.
larryhastings commentedon Jan 7, 2014
Attached is a new, simpler approach for supporting O!. The object() converter now takes two arguments:
(e.g. "PyUnicodeObject *")
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?
serhiy-storchaka commentedon Jan 7, 2014
It is marked "crash" because Argument Clinic generates code which crashes. It is assigned to docs because this feature documentation is misleading.
larryhastings commentedon Jan 7, 2014
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.
python-dev commentedon Jan 7, 2014
New changeset ddb5cd3e0860 by Larry Hastings in branch 'default':
Issue bpo-20141: Improved Argument Clinic's support for the PyArg_Parse "O!"
http://hg.python.org/cpython/rev/ddb5cd3e0860
larryhastings commentedon Jan 7, 2014
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.
Issue #20141: Improved Argument Clinic's support for the PyArg_Parse …
Issue #20141: Improved Argument Clinic's support for the PyArg_Parse …