There's currently noway to identify a Cython Coroutine Function.#3427
Conversation
Python's asyncio.coroutines uses an object to tag objects as coroutine functions. On this diff, we read this object and use it to tag Cython objects. It also includes tests to make sure asyncio.iscoroutinefunction() works as expected. This doesn't fix inspect.iscouroutinefunction() (which uses a flag that can trigger undesirable behavior for cython functions).
scoder
left a comment
There was a problem hiding this comment.
Thanks. I just think that a transform is not the right way to implement this.
| (which is copied from CPython). | ||
| """ | ||
|
|
||
| import asyncio |
There was a problem hiding this comment.
There is a dedicated test file for the integration with asyncio, py35_asyncio_async_def.srctree. Please don't add the dependency here.
|
|
||
| class IsCoroutineTransform(CythonTransform): | ||
| def visit_FuncDefNode(self, node): | ||
| if isinstance(node, (Nodes.AsyncDefNode, Nodes.IterableAsyncDefNode, Nodes.AsyncGenNode)): |
There was a problem hiding this comment.
Note that you can intercept on these directly by naming convention, rather than giving the method a more general name and checking the type here.
But, rather than implementing this as a transform, I think having it in C would be much better. The particular object can just be an additional parameter for the __Pyx_CyFunction_New() C function, and the import can happen in a utility function. Or, the import could even happen on first access to the attribute (property) if it's still NULL, and setting it to None initially would make it a normal non-async function. Or something like that. In any case, the place where the function object is being created in C seems a good place to make the distinction and pass it into the CyFunction_New(), as a flag, object, or whatever.
|
Yeah, I wasn't sure about the transform thing either. The idea was just to self-contain due to the comments mentioning moving to a protocol once cpython establishes one. First time working with cython, but I'll try to change it to C and address all the comments. Thanks for the input. :) |
|
COVID started and this flew completely under my radar, sorry. Took sometime to recover context now. I reverted the tree transform, and tried to use the C-API now. Doing a preliminary submission just to check if that's what you wanted @scoder. I decided to create my own flags, instead of using the "_is_coroutine" object, because that one will eventually go away when old-style coroutines are fully removed (the object is theoretically only used by the @coroutine based ones). With my own flag, I can patch the inspect module directly, and cover more usecases. I still don't fully grasp when the patching for inspect and asyncio trigger, but I'll try to figure it out. I can get it to trigger on local tests, but not for the ones I included. |
|
Okay... I finally understood how patching works, and why it wouldn't work here (it would only work for imports inside cythonized files, but not on regular python files just importing cython modules). Now I finally realized why the suggestion was to just add the On the latest commit I revert the previous attempts and just add the As suggested, I'm using a getter to only import the object when needed and requested. |
scoder
left a comment
There was a problem hiding this comment.
Looks quite good. Some comments below.
Not sure if the import isn't too heavy for this feature. Maybe we can cache the value somewhere instead of importing the module for it on each request (Which is at least a lookup in sys.modules and then a lookup of the attribute, but can be more than that – worth testing if it's noticeable.)
| PyObject* result = Py_None; | ||
| if (op->flags & __Pyx_CYFUNCTION_COROUTINE) { | ||
| PyObject *module = PyImport_ImportModule("asyncio.coroutines"); | ||
| result = PyObject_GetAttrString(module, "_is_coroutine"); |
There was a problem hiding this comment.
This case leaks a reference with the Py_INCREF() below, because it already is an owned reference. (Also, if the lookup fails, then the result is NULL and you get a crash on the incref.)
Also, it's more efficient to use Python strings directly instead of C strings for the lookup:
| result = PyObject_GetAttrString(module, "_is_coroutine"); | |
| result = __Pyx_PyObject_GetAttrStr(module, PYIDENT("_is_coroutine")); |
This requires an additional declaration at the beginning of this utility code section (~line 95) to also include this specific helper function:
//@requires: PyObjectGetAttrStr
You can also use PYIDENT("asyncio.coroutines") and then call PyImport_ImportModuleLevelObject(), at least in Py3. Py2 doesn't have that module anyway. And older Py3s don't have ._is_coroutine. Check PY_VERSION_HEX to make it a Py3.5(?)-only feature.
There was a problem hiding this comment.
Oh! Thanks for letting me know. :) Will amend, and add the PY_VERSION_HEX checks.
There was a problem hiding this comment.
FYI... Double checked and on v3.4.2rc1 _is_coroutine check was introduced, but it was just a True/False value. On v3.5.3rc1 the marker object was introduced. I'll add the macros to run for both versions.
|
|
||
| # mutually exclusive as far as "inspect" is concerned | ||
| if getattr(def_node, "is_asyncgen", None): | ||
| flags.append('__Pyx_CYFUNCTION_ASYNCGEN') |
There was a problem hiding this comment.
Not really, it's leftover from previous attempts. Although it's mutually exclusive with the other one... (see inspect.isasyncgenfunction py3.6) I can see people forgetting to add it, if the protocol is finally defined and implemented. I can remove it, if you think it's unnecessary.
There was a problem hiding this comment.
These are Cython-internal flags, though, not known to CPython.
| flags.append('__Pyx_CYFUNCTION_CCLASS') | ||
|
|
||
| # mutually exclusive as far as "inspect" is concerned | ||
| if getattr(def_node, "is_asyncgen", None): |
There was a problem hiding this comment.
Why would a simple attribute lookup not work here?
There was a problem hiding this comment.
Not all def_node have those attributes, they're added only on AsyncDefNode, and AsyncGenNode.
There was a problem hiding this comment.
Then we should add these attributes to DefNode. Because, why not?
|
As for caching, I thought about it too, but couldn't figure out where to store the reference. Would putting this on globals work? |
We could store it as a new struct field in CyFunction and initialise it at first use. That's a tiny bit wasteful (one reference per function instance), but seems good enough to me for now. It needs to be added to the traverse, clear and dealloc functions as well then, since it's a Python object field. |
|
Sounds good to me. Storing on CyFunction means it's a potential import flow for each coroutine function (which is better than one for each access of _is_coroutine). Is there someplace I could store the first import and just read the reference for other functions? That would be less wasteful. I don't know cython enough if such a place exists. :) |
|
If we use a global, then there's no use in storing the reference in the function. :) Let's keep it in the function for now, and see if it's good enough. (There are globals in Cython, but I'd like to avoid adding to them. Eventually, we'll need to make all global state local to an interpreter, and the function instance already is.) |
| op->func_is_coroutine = __Pyx_PyObject_GetAttrStr(module, marker); | ||
| Py_DECREF(module); | ||
| if (unlikely(!op->func_is_coroutine)) goto ignore; | ||
| return op->func_is_coroutine; |
There was a problem hiding this comment.
@scoder if I understood, GetAttrStr already owns the reference, so we need to skip Py_INCREF on the initial loading of the marker. Is this correct?
There was a problem hiding this comment.
We are dealing with two references here: one that we cache, and one that we return.
| return op->func_is_coroutine; | |
| return __Pyx_NewRef(op->func_is_coroutine); |
| is_coroutine = False | ||
| is_asyncgen = False |
There was a problem hiding this comment.
Now these can be removed from GeneratorDefNode (further down in the file), because they are inherited. In fact, looking through the class hierarchy now, I think these two should be set on FuncDefNode already, next to is_generator, is_async_def, etc.
| {(char *) "__defaults__", (getter)__Pyx_CyFunction_get_defaults, (setter)__Pyx_CyFunction_set_defaults, 0, 0}, | ||
| {(char *) "__kwdefaults__", (getter)__Pyx_CyFunction_get_kwdefaults, (setter)__Pyx_CyFunction_set_kwdefaults, 0, 0}, | ||
| {(char *) "__annotations__", (getter)__Pyx_CyFunction_get_annotations, (setter)__Pyx_CyFunction_set_annotations, 0, 0}, | ||
| #if PY_VERSION_HEX >= 0x030402C1 |
There was a problem hiding this comment.
We should use 0x03040000 here. The problem is that when someone builds the module on Python 3.4.1 and then imports it on 3.4.2+, the flag will not be available because it has been compiled out. It doesn't hurt to have it in 3.4.0 already.
Edit: actually, why not just always have it, also in Py2? It really doesn't hurt there.
| #if PY_VERSION_HEX >= 0x030503C1 | ||
| // on v3.5.3rc1 the marker object was introduced |
There was a problem hiding this comment.
Same version issue here – make it 3.5.0 and suppress the AttributeError.
| ignore: | ||
| op->func_is_coroutine = Py_None; |
There was a problem hiding this comment.
| ignore: | |
| op->func_is_coroutine = Py_None; | |
| ignore: | |
| PyErr_Clear(); | |
| op->func_is_coroutine = Py_None; |
| module = PyImport_ImportModuleLevelObject(PYIDENT("asyncio.coroutines"), NULL, NULL, fromlist, 0); | ||
| Py_DECREF(fromlist); | ||
| if (unlikely(!module)) goto ignore; | ||
| op->func_is_coroutine = __Pyx_PyObject_GetAttrStr(module, marker); |
There was a problem hiding this comment.
Since we're not interested in the AttributeError here, we can also use …_GetAttrStrNoError(), which avoids setting the exception (unless there is another error). Look for other usages to see how to apply it (it also has a different @requires name).
| fromlist = PyList_New(1); | ||
| if (unlikely(!fromlist)) goto ignore; |
There was a problem hiding this comment.
This looks like an error case that we shouldn't just ignore.
| fromlist = PyList_New(1); | |
| if (unlikely(!fromlist)) goto ignore; | |
| fromlist = PyList_New(1); | |
| if (unlikely(!fromlist)) return NULL; |
| op->func_is_coroutine = __Pyx_PyObject_GetAttrStr(module, marker); | ||
| Py_DECREF(module); | ||
| if (unlikely(!op->func_is_coroutine)) goto ignore; | ||
| return op->func_is_coroutine; |
There was a problem hiding this comment.
We are dealing with two references here: one that we cache, and one that we return.
| return op->func_is_coroutine; | |
| return __Pyx_NewRef(op->func_is_coroutine); |
| Py_INCREF(result); | ||
| return result; |
There was a problem hiding this comment.
| Py_INCREF(result); | |
| return result; | |
| return __Pyx_NewRef(result); |
|
@scoder, addressed the comments... BTW, thanks for the review and comments. Learning a lot. |
| return __Pyx_NewRef(op->func_is_coroutine); | ||
| #else | ||
| PyObject *result = (op->flags & __Pyx_CYFUNCTION_COROUTINE) ? Py_True : Py_False; | ||
| return __Py_NewRef(result); |
|
Personally I'd rather put the cached It isn't a strong preference though (especially since the current version looks like it's working) |
|
@da-woods, where can I access the CyFunction type-object dictionary? I can do a quick change. |
It'll be something close to (For reference this is an example of somewhere else Cython sets the There may be something else that I'm missing that makes it a bad idea though. |
That would also work. Not sure which is really better here. We already pay a couple of indirections to get to this code at all, so a couple more probably won't make a visible difference. I don't care about users changing that value, because they can also change It's yet to be proven that caching the value is necessary at all, though. Looking up |
|
Yeah, if this will add more code, and cause problems for other implementations, I suppose it's worth paying the price per function. |
| op->func_is_coroutine = Py_None; | ||
| return __Pyx_NewRef(op->func_is_coroutine); |
There was a problem hiding this comment.
I think we even need one more reference here. One new reference for the object field and one new reference to return.
There was a problem hiding this comment.
Nice catch, fixing it.
| PyObject *result = (op->flags & __Pyx_CYFUNCTION_COROUTINE) ? Py_True : Py_False; | ||
| Py_INCREF(result); | ||
| return result; | ||
| return __Py_NewRef(result); |
There was a problem hiding this comment.
There's even a function for this already: __Pyx_PyBool_FromLong().
There was a problem hiding this comment.
Good to know... Using it.
| PyList_SET_ITEM(fromlist, 0, marker); | ||
| module = PyImport_ImportModuleLevelObject(PYIDENT("asyncio.coroutines"), NULL, NULL, fromlist, 0); | ||
| Py_DECREF(fromlist); | ||
| if (unlikely(!module)) goto ignore; |
There was a problem hiding this comment.
This leaves the exception around. Need to call PyErr_Clear() somewhere on the way. Same for the other goto ignore, in case there was an exception (but we can just always clear it, even if there is none).
There was a problem hiding this comment.
Ah, I thought your previous comment was just for the GetAttr. I'll revert the NoError usage then, if we're going to clear it anyway.
There was a problem hiding this comment.
Well, why should we request the creation of an exception first just to clear it afterwards? If we can avoid it right away by calling …NoError(), the better.
| ignore: | ||
| PyErr_Clear(); | ||
| op->func_is_coroutine = __Pyx_NewRef(Py_None); | ||
| return __Pyx_NewRef(op->func_is_coroutine); | ||
| #else | ||
| return __Pyx_PyBool_FromLong(op->flags & __Pyx_CYFUNCTION_COROUTINE); | ||
| #endif |
There was a problem hiding this comment.
Wouldn't these fallbacks be more helpful?
| ignore: | |
| PyErr_Clear(); | |
| op->func_is_coroutine = __Pyx_NewRef(Py_None); | |
| return __Pyx_NewRef(op->func_is_coroutine); | |
| #else | |
| return __Pyx_PyBool_FromLong(op->flags & __Pyx_CYFUNCTION_COROUTINE); | |
| #endif | |
| ignore: | |
| PyErr_Clear(); | |
| #endif | |
| return __Pyx_PyBool_FromLong(op->flags & __Pyx_CYFUNCTION_COROUTINE); |
I'm not sure, I'm really wondering whether returning None is better than True / False (which both are not the _is_coroutine marker either).
There was a problem hiding this comment.
Checking both the pre 3.5.3rc1 version and the current one, it should work fine with False instead of None. The only disadvantage I see is that we're not caching the None for non-coroutine functions, and running the flag comparisson twice. Let me see what I can do here.
There was a problem hiding this comment.
we're not caching the None
Ah, but then just cache the return value, whatever it is.
|
Hey folks... Is anything else left for me to do here? |
|
Looks good to me. Thanks! |
* Fix test in Py2. * Check for exceptions also when @returns() is used, not only for "->" return type annotations. (cythonGH-3664) When you use Python type annotations, it would be weird if you lost Python exception propagation semantics along the way, just by compiling the code. So the default behaviour is "except? -1" here for C integer types. Arguably, this would also be a better default for the decorator case. See cython#3625 (comment) * Add tests that exception propagation works with the "@returns()" decorator. * Update changelog. * Allow selecting specific doctests in test modules with "-k pattern", instead of always running all tests. * Correct the positions reported for errors in f-strings. Closes cython#3674 * Fix f-string error positions in nogil test. * Clarify backwards incompatible special method change in changelog. * Fix many indentation and whitespace issues throughout the code base (cythonGH-3673) … and enforce them with pycodestyle. * Allow selecting specific doctests in test modules with "-k pattern", instead of always running all tests. * Prepare release of 0.29.20. * Update changelog. * Avoid calling PyUnicode_FromUnicode() in Py3. Closes cython#3677 * Update changelog. * Minor code simplification. * Always bind Cython functions * Update changelog. * Avoid an "unused variable" warning for code that gets compiled out in Py3. * Minor code simplification. * Revert "Always bind Cython functions" This reverts commit 6677326. * Update changelog. * Revert "Limited API updates and cleanup for cython#2056. cythonGH-3635)" This reverts commit 02bb311. * Revert "Invoke binop super method via direct slot access." This reverts commit bcb9387. * Revert "Add support for pow operator." This reverts commit d849fb2. * Revert "Python-style binary operation methods." This reverts commit e6a8124. * Re-add "c_api_binop_methods" directive for backwards compatibility after reverting cython#3633 and force it to "False". Closes cython#3688. * Change the default of the "c_api_binop_methods" directive to False. (cythonGH-3644) This is a backwards incompatible change that enables Python semantics for special methods by default, while the directive allows users to opt out of it and go back to the previous C-ish Cython semantics. See cython#2056 * Always bind Cython functions in Py3 (cythonGH-3683) Rebased 1bb26b9 for cython 0.29.x, and made conditional to Python 3. This does not solve the original staticmethod problem in Python 2 but it does resolve classmethod problems in Python 3.9. Therefore, it unbreaks other packages on Python 3.9, improves overall state for Python 3 and does not introduce regressions in Python 2. * Limit the scope of a local variable in a helper function. * Avoid a C compiler warning about a constant condition. * Remove dead code. * Update changelog. * Add support for C++ scoped enums with "enum class" and "enum struct" (cythonGH-3640) Closes cython#1603. * Add missing "PyUnicode_GET_LENGTH" to unicode.pxd (cythonGH-3692) * Add missing "PyUnicode_GET_LENGTH" to unicode.pxd (cythonGH-3692) * Fix usage of deprecated Py_UNICODE API. * Add safety fix to avoid reading a character from the empty string. * Fix prefix of internal function name. * Fix definition of "__Pyx_CyFunction_USED", which should only be #defined and not have a value. * Avoid using "tp_name" when CYTHON_COMPILING_IN_LIMITED_API (cythonGH-3693) * exec() did not allow recent Python syntax features in Py3.8+ due to https://bugs.python.org/issue35975 Closes cython#3695 * Avoid using the "tp_iternext" slot when CYTHON_USE_TYPE_SLOTS is disabled. * Give the "__Pyx_PyObject_GetIterNext" helper macro a more explanatory name. * Avoid unused variable in PyPy etc. * Avoid a call to PyTuple_GET_ITEM() to get the item array pointer if CYTHON_ASSUME_SAFE_MACROS is disabled. See cython#3701 * Enable travis for all branches. * Fix indentation counter for module init function. * Fix FunctionState handling for module cleanup function. * Validate that all temps were correctly released at the end of a function. * Keep reference to module dict around also in limited mode (cythonGH-3707) `PyModule_GetDict` is part of the limited API so we can keep a reference to the module dict around regardless of limited mode being enabled or not. * Validate and fix temp releasing (cythonGH-3708) * Fix a temp leak in the type init code. * Fix temp leaks in fused types initialisation code. * Correctly release the buffer index temps allocated for index calculations. * Make tests fails hard if a temp variable is not released at the end of a generated function. * Fix temp leak in switch statement code. * Make end-to-end tests fail on refnanny output. * Fix result temp leak in PyTypeTestNode. * Fix result temp leak in external type/function import code and enable the refnanny check for them. * Fix temp leak when try-return-finally is used in generators. * Make it explicit when an allocated temp is not meant to be reused. * Fix temp leak when assigning to the real/imag attributes of complex numbers. * Fix temp leak when assigning to a memoryview slice. * Clean up "num_threads" result temp in parallel section, not only in prange loop. * Fix temp leak in Pythran buffer setitem code. * Simplify NumPyMethodCallNode since it does not need the Python function anymore. Previously, it generated code that needlessly looked up the Python function without actually using it. * Fix temp leak when deleting C++ objects. * Add a test that actually reusing temps when deleting C++ objects works correctly. * Test runner: disable keep-alive output in --debug mode to keep a potential pdb console clean. * Fix argument name usage in finally blocks (cythonGH-3713) Fixes cython#3712 (hopefully) by reverting a small part of bbef4d7 * Fix argument name usage in finally blocks (cythonGH-3713) Fixes cython#3712 (hopefully) by reverting a small part of bbef4d7 * Document version-tagged pxd files (cythonGH-3587) * Implement generic optimized loop iterator with indexing and type inference for memoryviews (cythonGH-3617) * Adds bytearray iteration since that was not previously optimised (because it allows changing length during iteration). * Always set `entry.init` for memoryviewslice. * Update changelog. * Make end-to-end tests fail on refnanny output. * Fix FunctionState handling for module cleanup function. * Update change log. * Really only use PyUnicode_FromUnicode() when needed (cythonGH-3697) * Really only use PyUnicode_FromUnicode() for strings that contain lone surrogate, not for normal non-BMP strings and not for surrogate pairs on 16bit Unicode platforms. See cython#3678 * Extend buildenv test to debug a MacOS problem. * Add a test for surrogate pairs in Unicode strings. * Limit PyUnicode_FromUnicode() usage to strings containing lone surrogates. * Accept ambiguity of surrogate pairs in Unicode string literals when generated on 16bit Py2 systems. * Disable testing against NumPy 1.19+ in the 0.29.x branch, which removed C-API features. * Disable testing against NumPy 1.19.0 in the 0.29.x branch, which breaks a C-API call. * Validate and fix temp releasing (cythonGH-3708) (cythonGH-3717) * Validate and fix temp releasing (cythonGH-3708) Backports 92147ba. * Fix a temp leak in the type init code. * Fix temp leaks in fused types initialisation code. * Correctly release the buffer index temps allocated for index calculations. * Make tests fails hard if a temp variable is not released at the end of a generated function. * Fix temp leak in switch statement code. * Make end-to-end tests fail on refnanny output. * Fix result temp leak in PyTypeTestNode. * Fix result temp leak in external type/function import code and enable the refnanny check for them. * Fix temp leak when try-return-finally is used in generators. * Make it explicit when an allocated temp is not meant to be reused. * Fix temp leak when assigning to the real/imag attributes of complex numbers. * Fix temp leak when assigning to a memoryview slice. * Clean up "num_threads" result temp in parallel section, not only in prange loop. * Fix temp leak in Pythran buffer setitem code. * Simplify NumPyMethodCallNode since it does not need the Python function anymore. Previously, it generated code that needlessly looked up the Python function without actually using it. * Fix temp leak when deleting C++ objects. * Add a test that actually reusing temps when deleting C++ objects works correctly. * Improve syntax feature support of Cython CodeWriter (cythonGH-3514) * Disable Py_UNICODE fallback for __Pyx_UnicodeContainsUCS4() in Py3.9 since Py_UNICODE is deprecated and PEP-393 unicode is practically required. * Avoid calling PyUnicode_FromUnicode() in Py3 (cythonGH-3721) See cython#3677 * Add missing name substitution in __Pyx_PyUnicode_Substring() utility code. * Update changelog. * Prepare release of 0.29.21. * Really only use PyUnicode_FromUnicode() when needed (cythonGH-3697) * Really only use PyUnicode_FromUnicode() for strings that contain lone surrogate, not for normal non-BMP strings and not for surrogate pairs on 16bit Unicode platforms. See cython#3678 * Extend buildenv test to debug a MacOS problem. * Add a test for surrogate pairs in Unicode strings. * Limit PyUnicode_FromUnicode() usage to strings containing lone surrogates. * Accept ambiguity of surrogate pairs in Unicode string literals when generated on 16bit Py2 systems. * Disable Py_UNICODE fallback for __Pyx_UnicodeContainsUCS4() in Py3.9 since Py_UNICODE is deprecated and PEP-393 unicode is practically required. * Fix test in 16-bit Unicode deployments. * Update changelog. * Always consider 0-sized arrays as C- and F-contiguous (cythonGH-3728) Fixes cython#2093 * Always consider 0-sized arrays as C- and F-contiguous (cythonGH-3728) Fixes cython#2093 * Update changelog. * Add missing unordered_map template defaults (cythonGH-3686) * Add missing unordered_map template defaults (cythonGH-3686) * Update changelog. * Improve test output in case of failures. * Using Py_UNICODE to store lone surrogates makes Py3 join surrogate pairs on 16-bit Unicode platforms (Windows) when reading them back in, although we correctly processed them before. Instead, we now use the "unicode_escape" codec to store byte strings, because it can return surrogate characters (which the other codecs cannot). * Disable test in Py2.6. * Update changelog. * Use the more appropriate CYTHON_USE_TYPE_SLOTS guard for accessing the binop number slot. * Fix typo in error message. * Update the documentation on the arithmetic special methods and issue a "backwards compatibility" warning when the reversed method is not implemented. See cython#2056 * Add the new "c_api_binop_methods" directive to the documentation. * PyPy does not support PyType_GetSlot(). Use type slots instead. * Reformat doc paragraph. * Document C inline properties. * Beautify example output. * Use inline properties on the "PyComplex" builtin type declared in "cpython.complex" to provide C level access to the "real" and "imag" attributes (which Cython provides anyway for the 'undeclared' builtin type). * Prevent compile error when the result of repr() is assigned to a "unicode" LHS with language_level=3. Closes cython#3736 * Handle `for x in cpp_function_call()` (cythonGH-3667) Fixes cython#3663 This ensures that rvalues here are saved as temps, while keeping the existing behaviour for `for x in deref(vec)`, where the pointer for vec is copied, meaning it doesn't crash if vec is reassigned. The bit of this change liable to have the biggest effect is that I've changed the result type of dereference(x) and x[0] (where x is a c++ type) to a reference rather than value type. I think this is OK because it matches what C++ does. If that isn't a sensible change then I can probably inspect the loop sequence more closely to try to detect this. * Clarify changelog entry on ways to deal with the incompatible binop method change. * Remove Google Analytics script from documentation to avoid tracking our users. * Add donation banner to documentation. * Create bug template * Update issue templates * Delete unused custom ticket template * Add handshake emoji to donations banner to make it more visible. * Don't create CReference in C (only c++) (cythonGH-3746) * Fixed reference types being passed to getitemint (cythonGH-3755) * Reorder test module to restore the "invalid - valid" order. * Update changelog. * Prepare release of 3.0a6. * Add test comments on how "memslice.pyx" and "memoryview.pyx" relate. * Print test dependency versions to help with test failure debugging. * Add CI builds for different CPU architectures on travis. * Update issue templates * Add type "Py_hash_t" in pure Python mode. * Update changelog. * Clarify changelog entry. * Remove dead code and dead comments from "numpy/__init__.pxd". * Change "Py_intptr_t" declaration in numpy.pxd to what CPython uses as fallback. "int" seems overly lazy if it tends to be larger on many systems. * Readability improvements in "numpy.pxd". * Add a comment that numpy.pxd is maintained by the NumPy project. * Try to fix NumPy test failures by not setting the "NPY_NO_DEPRECATED_API" #define for NumPy 1.19.[01]. * Use NumPy 1.18.x for testing on travis as long as 1.19.[01+] ships a numpy.pxd that is incompatible with Cython 3.0. * Remove useless "extern" modifiers from cdef classes declared in "extern" sections. * Do not depend on the default type inference in "cpython/array.pxd". * Avoid unused C variable warning by moving declaration and usage under the same condition. Closes cython#3763 * Use a generic shape accessor for pythranized array (cythonGH-3762) This is a follow up to cython#3307 * Silence a C compiler warning about a constant condition. * Remove an unused function that had been added for the now-deleted "__getbuffer__" implementation. * Support PEP-560 ("__class_getitem__") for extension classes (cythonGH-3765) * Add "make checks" target to run code checks. * In bug template, ask for Python version in addition to Cython version * Support simple, non-strided views of "cython.array". Closes cython#3775 * Remove unused cimports. * Update changelog. * Set PYTHONHOME in embedding test to fix compilation issues in Py3.8/macOS. * Fix unrelated test after changing MemoryView.pyx. * Revert "Set PYTHONHOME in embedding test to fix compilation issues in Py3.8/macOS." This reverts commit a333d6a. The change did not resolve the test issue in travis. * Fix Python 3.4 + MSVC issue with elaborated type specifier for enum (cythonGH-3782) * Add more cimport_from_pyx tests (cythonGH-3786) There's now a fairly wide range of valid syntax for declaring things in pyx files and it should all be supported when cimporting from them. * Split known types into separate lines to let them have their own VCS history. * Add a note on PayPal fees for small payments. * Avoid merged-in code picking up directives from main module (cythonGH-3785) Fixes cython#1071 * Fix cygdb (cythonGH-3542) * Cython debugger documentation: Added link to an installation script. * Got a new libpython.py from the cpython source distribution. * Default language level in tests is now 3 instead of 2 * Migrated codefile from python 2 to python 3. * Added testcase for the cy list command in cygdb. * Temporarily removing test case that freezes gdb. * Fixed a bug that broke several Cygdb tests. The cython_debug/cython_debug_info_* files map the names of the C-functions generated by the Cython compiler to the names of the functions in the *.pyx source. If the function was defined using "def" (and not "cpdef" or "cdef") in the *.pyx source file, the C-function named in cython_debug/cython_debug_info_* used to be __pyx_pw_*, which is the name of the wrapper function and now it is __pyx_f_*, which is the name of the actual function. This makes some Cygdb tests pass that did not pass before. * Better error messages: If a cygdb command raises, a traceback will be printed. * Fixed a bug in cygdb. The following now works: 1. Start cygdb 2. Type "cy exec" and hit enter 3. Type some other lines 4. Type "end" and hit enter. -> These "other lines" will get executed * Fixed a bug in cygdb: cy list now works outside of functions. * Added print_hr_allmarkers function for easier debugging. * Fixed a bug that broke cygdb: cy break did not work if you put the breakpoint outside of a function if there was e.g. the following somewhere in your *.pyx file: cdef class SomeClass(): pass * Added a Cygdb test for printing global variables. * Fixing cygdb: Replaced cy print with a simple, working solution. * If an exception in Cygdb occurs, a stacktrace will be printed. * Fixed a bug that broke cy break -p * Bugfix: The compiler now writes out correctly which cython linenumber and path corresponds to which c linenumber. * Set language_level=2 in runtests.py * Update changelog. * Add a "gdb" test tag that depends on being able to run gdb. * Update CPython "test_fstring" copy to Py3.9. * Improved documentation for annotation typing (cythonGH-3799) Mainly by moving it to a separate section to make it easier to find, however also added a small amount of extra information about some of the obvious limitations. * Fix `return None` in CodeWriter. (cythonGH-3795) * Rename "GCC_DIAGNOSTIC" macro to make it Cython specific and make it available to other utility code sections. * Add a test for unpacking large PyLong values. * Silence gcc diagnostics whenever -1 is cast to something user provided. (cythonGH-3803) Fixes cythonGH-2749. * Define extern `PyBUF_MAX_NDIM` (cythonGH-3811) Ensure that Cython exposes `PyBUF_MAX_NDIM` from Python as part of `cpython.buffer` to allow access to developers. * Change test to use only integer calculations to prevent platform specific rounding issues, while keeping a reasonable distribution of integers across the PyLong digit ranges. * Created a glossary and added one new entry (cythonGH-3810) * Call destructors for structs in C++ (cythonGH-3228) In C++ mode, structs can contain C++ classes. Therefore structs should have their destructors called to ensure any class contained is also destructed. Also, a bit more thorough about ensuring constructor is generated where necessary. Closes cythonGH-3226. * Rename "GCC_DIAGNOSTIC" macro to make it Cython specific and make it available to other utility code sections. * Silence gcc diagnostics whenever -1 is cast to something user provided. (cythonGH-3803) Fixes cythonGH-2749. * Allow cast to ctuple (cythonGH-3812) * Make asyncio.iscoroutinefunction() recognise Cython compiled coroutines. (cythonGH-3427) Python's asyncio.coroutines uses an object to tag objects as coroutine functions. We now read this object and use it to tag Cython compiled coroutines as well. It also includes tests to make sure `asyncio.iscoroutinefunction()` works as expected. This doesn't fix `inspect.iscouroutinefunction()` (which uses a flag that can trigger undesirable behaviour for cython functions). Closes cython#2273 * Do not cover lines that were excluded in the coveragerc config file (cythonGH-3682) Closes cython#3680. * Add doc support for cpdef enum (cythonGH-3813) * Update changelog. * Add "check_size ignore" to all NumPy.pxd class declarations to silence the useless size warnings about them. See numpy/numpy#432 (comment) Also remove the useless "extern" modifiers from cdef classes declared in "extern" sections. * Update changelog. * Update changelog. * Support utility code in headers (cythonGH-3779) Specifically this allows public packed structs but may also allow other public declarations that require small amounts of utility code. * Prevent overflowing hash values for "Machines.Node" due to arbitrarily large unsigned pointer values. Closes cython#3840 * Extract "error_type" handling from the type creation functions. * Remember in the AnnotationsWriter when a serialised expression contained unknown/unsupported nodes. * Do not include "u" string prefix in annotations since tools that process them probably expect Py3 string semantics anyway. * Keep AnnotationNode instead of the bare annotation expression in "entry.annotation" to get access to the string representation. * Fix test after removing the 'u' prefix from unicode string annotation values. * Add support for PEP 526 `__annotations__` in class body. (cythonGH-3829) Closes cython#2552 * Some more glossary entries (cythonGH-3836) * Rename test file to avoid ambiguity with the C "const" modifier. * Join '*' and '**' parsing in declarators to avoid differences for 'const' parsing etc. * Set language level in Cython's speed-up .pxd files since it no longer depends on the .py files that they correspond to. * Declare "scanner.sy" as "unicode" string to optimise its usage. * Use more recent C-API functions on tear-down of the embedding code. * Improve some wordings in README.rst (cythonGH-3852) * Restore Py2 compatibility in test. * Replace useless comment. * Fixed assorted crashes with fused types (cythonGH-3853) Show a more detailed error for unspecializable fused return types. * Add a more common and more versatile example to the Verbatim C-Code documentation. * Fix RST typo. * Make C code C89 again. * Add support for forwarding references (cythonGH-3821) See, for example, https://isocpp.org/blog/2012/11/universal-references-in-c11-scott-meyers Closes cython#3814 * Avoid C compiler warnings about unused variables in test. * Avoid C compiler warning about unused variable in test. * Test Py3.7 and Py3.8 in C and C++ mode under appveyor. Run both in the same job since the machines are quite fast with parallel processes, but setting one up is slow. * Fix test compile failure in MSVC. * Allow creation of wrappers for cdef functions with memoryviews (cythonGH-3856) Fixes cython#3843 * Clean up and test type identifier escaping. - hash() hashing lead to unpredictable random prefixes for long names across multiple runs - use a single regex run instead of repeated calls to replace() * Move a memoryview test out of the "run" test directory since memoryview tests tend to be slow and have their own test directory. * Fix test compile failure in MSVC. * Clarify the section on exception return values. * Be a bit more paranoid about macro usage in the refnanny code. * Looks like the usual macro on Windows is "_WIN32" and not "WIN32". Let's support both, to be on the safe side. * Try to fix platform specific test once more. "synchapi.h" is not supposed to be included directly, and "windows.h" can break ... other stuff. Let's see what we can do. * Fix ReST typo. * docs: fix typos, minor clarification. * Minor docs clarification on error return values. (cythonGH-3859) * Replace deprecated Element.getiterator() with Element.iter(). (cythonGH-3864) `xml.etree.ElementTree.Element.getiterator()` was deprecated in Python 2.7 & 3.2 and removed in the freshly released Python 3.9. * Prefer Element.iter() over the deprecated Element.getiterator(). (cythonGH-3865) `xml.etree.ElementTree.Element.getiterator()` was deprecated in Python 2.7 & 3.2 and removed in Python 3.9. * Split a combined assert in two to avoid an unused C temp variable when assertions are disabled. (cythonGH-3870) * Split a combined assert in two to avoid an unused C temp variable when assertions are disabled. (cythonGH-3870) * Disable a test that fails in Py2 in 0.29.x since 'str' is 'unicode' in cython3, also in Python 2. * Rename test file to avoid ambiguity with the C "const" modifier. * Use more recent C-API functions on tear-down of the embedding code. * Make type identifier escaping deterministic: hash() hashing lead to unpredictable random prefixes for long names across multiple Python runs. * Clarify the section on exception return values. * Fix ReST typo. * docs: fix typos, minor clarification. * Minor docs clarification on error return values. (cythonGH-3859) * Add Python 3.9 to appveyor build. * Resolve merge conflict. * Removed `same_as` methods from Plex.Actions.Action (cythonGH-3847) It seems to be unused and it looked like the existing implementation was faulty. * Fix test after changing the hash method used for escaping long type descriptions. * Add warning for common user mistake and fix crash relating to annotated attributes. (cythonGH-3850) Closes cython#3830 * Improve error reporting when users mistakenly write "&&" or "||" instead of Python's "and" and "or" operators. (cythonGH-3858) * Disable the embedding test on MacOS-X to get the builds green again. Too many PRs depend on the travis tests to give a green light. * Simplify the output stream capturing for the C compiler runs by using a temp file instead of threads. * Help reporters see the first paragraph of the bug report template. * Detect _MSC_VER for __PYX_STD_MOVE_IF_SUPPORTED to support MSVC support even when "__cplusplus" is not set appropriately (cythonGH-3792) * Fix memoryview casts involving fused types (cythonGH-3882) I think this approach is more satisfactory than the old way it used to "work", where "fused_to_specific" was permanently added to the module scope containing the fused type (in this case the Cython scope), was applied in "Scope.lookup_type", but continued to have an effect on the scope forever. Closes cython#3881 * Add O_DIRECT to posix/fcntl.pxd (cythonGH-3894) Closes cython#3242 Co-authored-by: Stefan Behnel <[email protected]> Co-authored-by: Jeroen Demeyer <[email protected]> Co-authored-by: Robert Bradshaw <[email protected]> Co-authored-by: Michał Górny <[email protected]> Co-authored-by: Ashwin Srinath <[email protected]> Co-authored-by: Thomas A Caswell <[email protected]> Co-authored-by: Matthias Braun <[email protected]> Co-authored-by: da-woods <[email protected]> Co-authored-by: Tao He <[email protected]> Co-authored-by: Victor Stinner <[email protected]> Co-authored-by: Clemens <[email protected]> Co-authored-by: will <[email protected]> Co-authored-by: serge-sans-paille <[email protected]> Co-authored-by: Yuan <[email protected]> Co-authored-by: Volker-Weissmann <[email protected]> Co-authored-by: Tao He <[email protected]> Co-authored-by: cf-natali <[email protected]> Co-authored-by: jakirkham <[email protected]> Co-authored-by: Tasha "Ren" Chin <[email protected]> Co-authored-by: Pedro Marques da Luz <[email protected]> Co-authored-by: Mathias Laurin <[email protected]> Co-authored-by: matham <[email protected]> Co-authored-by: Sean <[email protected]> Co-authored-by: Rajat Dash <[email protected]> Co-authored-by: ptype <[email protected]> Co-authored-by: Nick Pope <[email protected]> Co-authored-by: Jeppe Dakin <[email protected]> Co-authored-by: Zackery Spytz <[email protected]>
|
Would it be possible to backport this to 0.29. Happy to make a PR if that would be helpful |
Hmm. Doesn't really feel like an 0.29 feature to me. And there need to be reasons for users to switch to 3.0 as well. :) |
|
Do we know when Cython 3.0 is planned to be released? |
|
Hi, I was testing out this patch for some application. I am using the Master branch code. While testing I was faced with "Key Error" after a fixed number of asyncio.iscoroutinefunction() calls on distinct coroutine functions. The following minimal code example can be used to reproduce the above issue: import asyncio
async def cb1(msg):
print("cb1")
async def cb2(msg):
print("cb2")
async def cb3(msg):
print("cb3")
async def cb4(msg):
print("cb4")
async def cb5(msg):
print("cb5")
async def cb6(msg):
print("cb6")
async def cb7(msg):
print("cb7")
async def cb8(msg):
print("cb8")
async def cb9(msg):
print("cb9")
async def cb10(msg):
print("cb10")
async def cb11(msg):
print("cb11")
async def cb12(msg):
print("cb12")
async def cb13(msg):
print("cb13")
async def cb14(msg):
print("cb14")
def main():
#asyncio.iscoroutinefunction(cb1)
#asyncio.iscoroutinefunction(cb1)
#asyncio.iscoroutinefunction(cb1)
#asyncio.iscoroutinefunction(cb1)
asyncio.iscoroutinefunction(cb1)
asyncio.iscoroutinefunction(cb2)
asyncio.iscoroutinefunction(cb3)
asyncio.iscoroutinefunction(cb4)
asyncio.iscoroutinefunction(cb5)
asyncio.iscoroutinefunction(cb6)
asyncio.iscoroutinefunction(cb7)
asyncio.iscoroutinefunction(cb8)
asyncio.iscoroutinefunction(cb9)
asyncio.iscoroutinefunction(cb10)
asyncio.iscoroutinefunction(cb11)
asyncio.iscoroutinefunction(cb12)
asyncio.iscoroutinefunction(cb13)
if __name__ == '__main__':
main()The compilation was done as follows on Debian Buster with Python 3.7 and gcc (Debian 8.3.0-6) 8.3.0: cython -3 cython_bug.py --embed && gcc -ggdb -Os -I /usr/include/python3.7m/ -o cython_bug.o cython_bug.c -lpython3.7m -lpthread -lm -lutil -ldl && ./cython_bug.oThe number of checks performed on distinct coroutine functions is 13 which causes the crash. If the last line Further, if the last line is commented out and the first few commented checks are uncommented, I'm faced with Seg faults sometimes. Please review the same and suggest what could have gone wrong. |
|
To simplify tracking, could you please file this as an issue, @HVP1621? |
Done. Tracking here: #4182 |
|
Great thank you 😄 |
Python's asyncio.coroutines uses an object (_is_coroutine) to tag objects as coroutine functions.
On this diff, we read _is_coroutine and use it to tag Cython functions and methods where appropriate.
I tried to keep all changes inside a transform, that way it becomes simple to remove/replace with the protocol once CPython folks decide on one. I'm not sure about the pipeline order, so will gladly move if needed.
It also includes tests to make sure asyncio.iscoroutinefunction() works as expected. Please let me know if I need to move those to a specific place.
This doesn't fix inspect.iscouroutinefunction() (which uses a flag that can trigger
undesirable behavior for cython functions).
Closes #2273