Repository navigation
gh-108494: Argument Clinic: fix support of Limited C API - #108536
serhiy-storchaka merged 7 commits into
Conversation
Sorry, something went wrong.
|
Yet one issue: the TokenizerIter constructor is incompatible with PyArg_ParseTupleAndKeywords. I temporary worked it around by adding the default value for keyword-only extra_tokens parameter. Similar incompatibility was also a cause of failing two test_clinic tests. Resolving #74366 should fix this. |
Sorry, something went wrong.
|
|
||
| def test_varargs1min(self): | ||
| msg = r"get expected at least 1 argument, got 0" | ||
| msg = (r"get\(\) takes at least 1 argument \(0 given\)|" |
There was a problem hiding this comment.
PyArg_ParseTuple() produces slightly different error message.
Sorry, something went wrong.
|
I'm very excited by this change! I will try to review it soon. Would it be possible to write unit tests in _testclinic_limited and test_clinic, for unusual cases like deprecated parameters and (if possible) defining class? In my other PR, I wrote tests for basic positional or keywords parameters. |
Sorry, something went wrong.
|
As for tests, I think that we should run all (or almost all) clinic tests in two modes. Instead of adding new tests for the limited API or duplicating test code we should generate and build different modules from the same source with different options. But this is a different PR. |
Sorry, something went wrong.
|
|
||
| arg = PyLong_AsInt(arg_); | ||
| if (arg == -1 && PyErr_Occurred()) { | ||
| if (!PyArg_Parse(arg_, "i:my_int_func", &arg)) { |
There was a problem hiding this comment.
It looks like regression, but actually not all inlined parsing code is compatible with the limited C API. In following PR I will implement inlining correctly, only for converters compatible with the limited C API. It will be large but mostly boring change (adding new parameter and simple checks in tens of functions), so I did not include it here.
Sorry, something went wrong.
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
erlend-aasland
left a comment
There was a problem hiding this comment.
I may have missed something, but this looks good to me. Thanks, Serhiy!
Sorry, something went wrong.
AlexWaygood
left a comment
There was a problem hiding this comment.
No complaints from me (though I'm not an expert on the limited C API)
Sorry, something went wrong.
I tried to enable the use of the limited C API in Argument Clinic for all code. With this PR it generates working code, and all tests are passed, except the low-level vectorcall test in test_call and few tests in test_clinic (as expected). _struct.c and winreg.c use converters incompatible with the limited C API. I temporary blacklisted these files. I only tested on Linux, so there may be errors in Windows or macOS specific modules.