Repository navigation
Derby #13: Convert 50 sites to Argument Clinic across 5 files #64381
Description
Activity
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 sitesTalk 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.htmlHere'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.
loewis commented
on Jul 27, 2014 loewismannequinMannequinThe 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.
22 remaining items
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.
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. ...
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__, ^~~~~~~~~~~~
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:
bugs.python.org fields: