Derby #13: Convert 50 sites to Argument Clinic across 5 files

@loewis

BPO	20182
Nosy	@loewis, @birkenfeld, @vstinner, @taleinat, @larryhastings, @serhiy-storchaka
PRs	bpo-31938: Convert selectmodule.c to Argument Clinic #4265 bpo-20182: AC convert remaining functions/methods in _hashopenssl.c #9213 bpo-20182: AC convert Python/sysmodule.c #11328 bpo-20182: AC convert Python/sysmodule.c #11328 bpo-20182: remove doc-string declaration no longer used after AC conversion #11444
Files	modules_issue20182.patch modules_issue20182_v2.patch modules_issue20182_v3.patch: updated version of Georg's patch, now applies cleanly to default branch with tests passing issue20182.signalmodule.patch: updated patch for signalmodule after Serhiy's review issue20182.selectmodule.patch: updated patch for selectmodule after Serhiy's review issue20182.selectmodule.v2.patch: updated patch; set epoll method function names back to what they were issue20182.sysmodule.patch: updated patch for sysmodule after Serhiy's review issue20182._hashopenssl.patch: AC conversion of Modules/_hashopenssl.c

^{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 2019-01-01.08:59:22.836>
created_at = <Date 2014-01-08.00:17:37.422>
labels = ['extension-modules', 'type-feature', 'expert-argument-clinic']
title = 'Derby #13: Convert 50 sites to Argument Clinic across 5 files'
updated_at = <Date 2019-01-06.07:04:54.833>
user = 'https://github.com/larryhastings'

bugs.python.org fields:

activity = <Date 2019-01-06.07:04:54.833>
actor = 'taleinat'
assignee = 'none'
closed = True
closed_date = <Date 2019-01-01.08:59:22.836>
closer = 'taleinat'
components = ['Extension Modules', 'Argument Clinic']
creation = <Date 2014-01-08.00:17:37.422>
creator = 'larry'
dependencies = []
files = ['33441', '33491', '39188', '39194', '39201', '39203', '39387', '39389']
hgrepos = []
issue_num = 20182
keywords = ['patch']
message_count = 47.0
messages = ['207639', '207941', '207945', '207951', '208010', '208258', '224138', '224763', '241864', '241868', '241872', '241900', '241916', '241917', '241919', '241922', '241936', '241989', '242003', '242476', '242488', '242489', '242492', '243302', '243305', '243308', '243309', '243310', '243312', '304907', '304932', '305168', '305410', '305429', '305483', '305484', '305519', '325127', '332587', '332589', '332594', '332749', '332820', '332822', '332823', '333062', '333091']
nosy_count = 8.0
nosy_names = ['loewis', 'georg.brandl', 'vstinner', 'taleinat', 'larry', 'nadeem.vawda', 'python-dev', 'serhiy.storchaka']
pr_nums = ['4265', '9213', '11328', '11328', '11444']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue20182'
versions = ['Python 3.5']

This issue is part of the Great Argument Clinic Conversion Derby,
where we're trying to convert as much of Python 3.4 to use
Argument Clinic as we can before Release Candidate 1 on January 19.

This issue asks you to change the following bundle of files:
Modules/signalmodule.c: 12 sites
Modules/selectmodule.c: 12 sites
Modules/zlibmodule.c: 11 sites
Python/sysmodule.c: 10 sites
Modules/_hashopenssl.c: 5 sites

Talk to me (larry) if you only want to attack part of a bundle.

For instructions on how to convert a function to work with Argument
Clinic, read the "howto":
http://docs.python.org/dev/howto/clinic.html

Here's the signalmodule.

Now without crap.

Here's sys.

For it to generate the correct code you need to add "args" to c_keywords in clinic.py.

New patch.

In selectmodule.c I haven't touched devpoll and kqueue, which I can't compile here.

_hashopenssl does weird things with macros, I'm waiting with that.

New patch addressing comments.

The patch does not apply anymore. Can you please update it, and rerun AC on it?

All the Derby patches should only go into trunk at this point.

Note: Despite not appearing in any of these patches, the zlib module seems to have already been converted.

I'm working on a new version of Georg's patch which applies to the current default branch.

I'm having trouble with AC output going into a separate file, since this requires a lot of things to be declared before the #include of the .c.h file. Should I move all of those definitions to the top of the file, even if they are currently spread all over it? Should I just add forward declarations? And if the latter, how do I forward-declare a struct typedef?

You only need include generated file before defining PyMethodDef arrays and PyTypeObject instances. There shouldn't be problems if only one class is defined in a file, otherwise you can move these static initializations together to the end of the file.

Here's a new version of Georg's patch which applies to the current default branch. On my OSX 10.8, CPython compiles fine and all tests pass, though test_devpoll and test_epoll were skipped.

I haven't made additional changes, so there is still AC-related work to be done here. For example, there are legacy converters used throughout the code, which should be replaced.

Changes I had to make to get this working:

Updated the class declarations in selectmodule.c with the additional parameters.

The doc-string for signal.sigwaitinfo was changed since Georg made his patch, and its first line was now too long. I shortened the first line from:

Wait synchronously for a signal until one of the signals in *sigset* is delivered.

to:

Wait synchronously until one of the signals in *sigset* is delivered.

Converted the new sys.is_finalizing function.

Reverted the signal.set_wakeup_fd function, since it has since been changed, and now depending on #ifdef MS_WINDOWS it interprets the argument as either 'O' or 'i'. I'm not sure what to do with this one, so I reverted it back to not use AC for now.

I haven't converted the new signal.default_int_handler function, since it seems to accept any number of arguments, and ignores them. Is there a way to do this with AC?

Moved the definitions of object structs and PyTypeObject types to the top of selectmodule.c, so that they are defined when Modules/clinic/selectmodule.c.h is imported.

I really hate the title of these issues "Derby #13: Convert 50 sites to Argument Clinic across 5 files". I would prefer to have one issue per file. Would it possible to close this one and open a new issue once someone has a PR for one file?

As the author of all of the updated patches, I wouldn't mind opening new issues separately for each of the remaining modules. Actually I would prefer it :)

See issue bpo-31938 regarding Modules/selectmodule.c.

See new updated PR9213 for completing the _hashopenssl.c conversion.

New changeset c6c7237 by Tal Einat in branch 'master':
bpo-20182: AC convert remaining functions/methods in _hashopenssl.c (GH-9213)
c6c7237

The conversion of Modules/_hashopenssl.c leaves just Python/sysmodule.c to be converted from this batch.

There's an old patch here from 3.5 years ago that will likely need to be heavily updated.

See new PR #55537 with updated AC conversion of Python/sysmodule.c.

PR 9213 added a compiler warning:

In file included from /home/serhiy/py/cpython/Modules/_hashopenssl.c:69:0:
/home/serhiy/py/cpython/Modules/clinic/_hashopenssl.c.h:90:1: warning: ‘EVP_tp_init’ defined but not used [-Wunused-function]
EVP_tp_init(PyObject *self, PyObject *args, PyObject *kwargs)
^~~~~~~~~~~

help(_hashlib.HASH) shows now the signature of the constructor of the object. But a hash object can not be created using it.

class HASH(builtins.object)
 |  HASH(name, string=b'')
 |  
 |  A hash is an object used to calculate a checksum of a string of information.
...

New changeset ede0b6f by Tal Einat in branch 'master':
bpo-20182: AC convert Python/sysmodule.c (GH-11328)
ede0b6f

Note that the problematic part of the Modules/_hashopenssl.c AC conversion was reverted, as part of #55588.

AFAICT, this issue can finally be closed!

Seems PR 11328 introduced a compiler warning:

In file included from ./Include/Python.h:64:0,
                 from ./Python/sysmodule.c:17:
./Python/sysmodule.c:1597:14: warning: ‘sys_clear_type_cache__doc__’ defined but not used [-Wunused-variable]
 PyDoc_STRVAR(sys_clear_type_cache__doc__,
              ^
./Include/pymacro.h:70:37: note: in definition of macro ‘PyDoc_VAR’
 #define PyDoc_VAR(name) static char name[]
                                     ^~~~
./Python/sysmodule.c:1597:1: note: in expansion of macro ‘PyDoc_STRVAR’
 PyDoc_STRVAR(sys_clear_type_cache__doc__,
 ^~~~~~~~~~~~

Thanks Serhiy! The compiler doesn't warn about that on Windows.

See PR #55653 with a fix.

Navigation Menu

Repository navigation

Derby #13: Convert 50 sites to Argument Clinic across 5 files #64381

22 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Derby #13: Convert 50 sites to Argument Clinic across 5 files #64381

Description

Activity

larryhastings commented on Jan 8, 2014

birkenfeld commented on Jan 12, 2014

birkenfeld commented on Jan 12, 2014

birkenfeld commented on Jan 12, 2014

birkenfeld commented on Jan 13, 2014

birkenfeld commented on Jan 16, 2014

loewis commented on Jul 27, 2014

larryhastings commented on Aug 4, 2014

taleinat commented on Apr 23, 2015

taleinat commented on Apr 23, 2015

serhiy-storchaka commented on Apr 23, 2015

taleinat commented on Apr 24, 2015

22 remaining items

vstinner commented on Nov 3, 2017

taleinat commented on Nov 3, 2017

taleinat commented on Nov 3, 2017

taleinat commented on Sep 12, 2018

taleinat commented on Dec 27, 2018

taleinat commented on Dec 27, 2018

taleinat commented on Dec 27, 2018

serhiy-storchaka commented on Dec 30, 2018

taleinat commented on Dec 31, 2018

taleinat commented on Dec 31, 2018

taleinat commented on Dec 31, 2018

serhiy-storchaka commented on Jan 5, 2019

taleinat commented on Jan 6, 2019

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions