Error in Python 3 docs for PyMethodDef

[sandrotosi]

BPO	15657
Nosy	@warsaw, @jcea, @abalkin, @asvetlov, @sandrotosi, @berkerpeksag, @vadmium, @serhiy-storchaka
Files	issue15657.diff issue15657_36.diff

^{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 = None
closed_at = <Date 2017-02-22.23:07:11.345>
created_at = <Date 2012-08-14.19:02:45.538>
labels = ['interpreter-core', 'type-bug', '3.7']
title = 'Error in Python 3 docs for PyMethodDef'
updated_at = <Date 2017-02-22.23:07:11.343>
user = 'https://github.com/sandrotosi'

bugs.python.org fields:

activity = <Date 2017-02-22.23:07:11.343>
actor = 'barry'
assignee = 'docs@python'
closed = True
closed_date = <Date 2017-02-22.23:07:11.345>
closer = 'barry'
components = ['Interpreter Core']
creation = <Date 2012-08-14.19:02:45.538>
creator = 'sandro.tosi'
dependencies = []
files = ['39164', '43364']
hgrepos = []
issue_num = 15657
keywords = ['patch']
message_count = 14.0
messages = ['168226', '168463', '186676', '186679', '211126', '241780', '247285', '268091', '268379', '268380', '268382', '285100', '285218', '288390']
nosy_count = 13.0
nosy_names = ['barry', 'jcea', 'belopolsky', 'md5i', 'asvetlov', 'sandro.tosi', 'docs@python', 'python-dev', 'berker.peksag', 'martin.panter', 'serhiy.storchaka', 'bkabrda', 'cimarron']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue15657'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

[sandrotosi]

Hello,
it has been reported at http://mail.python.org/pipermail/docs/2012-April/008215.html but given it raises some question whether it's a bug in the doc or in the code, i'd rather report the issue here and hear what other think:

>>
In the Python 3.2.2 documentation (and earlier Python 3 versions), in
the Python/C API Reference Manual, chapter Object Implementation
Support, the documentation for PyMethodDef says:

The ml_flags field is a bitfield which can include the following
flags. The individual flags indicate either a calling convention or a
binding convention. Of the calling convention flags, only METH_VARARGS
and METH_KEYWORDS can be combined (but note that METH_KEYWORDS alone
is equivalent to METH_VARARGS | METH_KEYWORDS).

The bit in the parenthetical is incorrect. If you take a look at
PyCFunction_Call in Objects/methodobject.c, you will find a switch for
METH_VARARGS | METH_KEYWORDS, but no switch for METH_KEYWORDS. Hence,
using METH_KEYWORDS will land you with a SystemError that complains
about METH_OLDARGS.

This is either a bug in the documentation, or a bug in Python. In this
case, since the code has persisted through three major revisions of
Python 3, I suggest that the bug _is_ in the documentation (whether it
should be or not), since changing the code for this at this late date
means a programmer has to use METH_VARARGS | METH_KEYWORDS anyway for
compatibility.

It may be work pointing out specifically in the documentation that just
using METH_KEYWORDS will not work.
<<<

[asvetlov]

I think we can change PyCFunction_Call to accept METH_KEYWORDS as alias for METH_VARARGS | METH_KEYWORDS.
It cannot make incompatibility with existing code base.

[warsaw]

I just ran into this too.

[warsaw]

We should fix the code for 3.2 through 3.4, but change the docs for 3.2 and 3.3 to remove the parenthetical note. For 3.4 we can leave the parenthetical note but say this is new in 3.4 (or maybe, that it doesn't actually work in some versions < 3.4). I agree that for code to work consistently across all Python 3.2 and 3.3 microversions, extension code is going to have to include both flags anyway.

[cimarron]

Appears to be a duplicate of bpo-11587 but better explanation here

[berkerpeksag]

Here is a patch for Python 3.5.

[serhiy-storchaka]

In 3.5 it would be better to make METH_KEYWORDS == METH_VARARGS | METH_KEYWORDS.

Current definition:

#define METH_VARARGS  0x0001
#define METH_KEYWORDS 0x0002

Should be:

#define METH_VARARGS  0x0001
#define METH_KEYWORDS 0x0003

But it can't be applied in maintained releases. In 3.4 and 2.7 we should add explicit test as in the patch or change the documentation.

If fix the code rather than documentation in 3.4 and 2.7, then the versionchanged directive in 3.5 shouldn't be added.

[berkerpeksag]

I'm going to delete

(but note that :const:`METH_KEYWORDS` alone is equivalent to ``METH_VARARGS | METH_KEYWORDS``)

from Doc/c-api/structures.rst in 3.5.

Then change the value of METH_KEYWORDS from 0x0002 to 0x0003 in 3.6.

Thanks for the review Serhiy.

[python-dev]

New changeset 367b3f41710a by Berker Peksag in branch '3.5':
Issue bpo-15657: Delete incorrect statement from PyMethodDef documentation
https://hg.python.org/cpython/rev/367b3f41710a

New changeset 7fa4986d8218 by Berker Peksag in branch 'default':
Issue bpo-15657: Null merge from 3.5
https://hg.python.org/cpython/rev/7fa4986d8218

[python-dev]

New changeset f520ae3b537b by Berker Peksag in branch '2.7':
Issue bpo-15657: Delete incorrect statement from PyMethodDef documentation
https://hg.python.org/cpython/rev/f520ae3b537b

[berkerpeksag]

Then change the value of METH_KEYWORDS from 0x0002 to 0x0003 in 3.6.

Here is a patch.

[vadmium]

.
The documentation did not get merged properly into 3.6+. And even in 3.5, under METH_KEYWORDS, I propose to change “The flag is typically combined with METH_VARARGS” to “The flag must be combined . . .”.

The remaining issue15657_36.diff patch looks out of date. There is a _PyCFunction_FastCallDict() function that appears to also need adjusting.

But instead of changing the value of METH_KEYWORDS, wouldn’t it be safer to just accept METH_KEYWORDS (= 2) on its own as a valid value? This is what Python 2 does. Essentially, revert the first hunk of

https://hg.python.org/cpython/diff/b7bfa780a882/Objects/methodobject.c

but without METH_OLDARGS, whose value was zero anyway.

BTW, the statement did not need to be removed in Python 2, but IMO it’s fine now without the statment. The statement was added in revision 1564c6839e6b.

[python-dev]

New changeset d7d2d24003f5 by Martin Panter in branch '3.5':
Issue bpo-15657: METH_KEYWORDS cannot be used alone in Python 3
https://hg.python.org/cpython/rev/d7d2d24003f5

New changeset c140e72492a4 by Martin Panter in branch '3.6':
Issue bpo-15657: Delete incorrect statement from PyMethodDef documentation
https://hg.python.org/cpython/rev/c140e72492a4

New changeset 021fd2ff7ca4 by Martin Panter in branch '3.6':
Issue bpo-15657: Merge other doc fix from 3.5
https://hg.python.org/cpython/rev/021fd2ff7ca4

New changeset 1058e151049a by Martin Panter in branch 'default':
Issue bpo-15657: Merge METH_KEYWORDS doc from 3.6
https://hg.python.org/cpython/rev/1058e151049a

[warsaw]

I think this bug has been fixed.