Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fused functions don't necessarily pass __Pyx_CyFunction_Check #3384

Closed
da-woods opened this issue Feb 29, 2020 · 4 comments
Closed

Fused functions don't necessarily pass __Pyx_CyFunction_Check #3384

da-woods opened this issue Feb 29, 2020 · 4 comments

Comments

@da-woods
Copy link
Contributor

da-woods commented Feb 29, 2020

Reproducible test-case:

cdef extern from *:
    int __Pyx_CyFunction_Check(object)

ctypedef fused IntOrDouble:
    int
    double

def f(IntOrDouble x):
    # fused function
    pass

def run_cyfunction_check():
    """
    >>> run_cyfunction_check()
    <class 'fused_cython_function'>
    1
    """
    print(type(f))
    print(__Pyx_CyFunction_Check(f))  # should be True

Run this as part of the test-suite with a compiled version of Cython (fails) and a non-compiled version of Cython (passes).


I believe this issue is as follows

__pyx_FusedFunctionType_type uses as its base __pyx_CyFunctionType_type:
https://github.com/cython/cython/blob/master/Cython/Utility/CythonFunction.c#L1408

__Pyx_CyFunction_Check uses __pyx_CyFunctionType (note lack of _type suffix)

#define __Pyx_CyFunction_Check(obj) (__Pyx_TypeCheck(obj, __pyx_CyFunctionType))

__pyx_CyFunctionType is initialized from using __Pyx_FetchCommonType
__pyx_CyFunctionType = __Pyx_FetchCommonType(&__pyx_CyFunctionType_type);
This means that when using a compiled version of Cython, it can end up different to __pyx_CyFunctionType_type, meaning fused functions no longer register as cyfunctions.

Unfortunately I have no idea how this should be fixed....


For context (not really part of this bug report) this is causing me problems in __Pyx_Method_ClassMethod because it's deciding that fused functions are unknown types and throwing an exception. This is actually an easily-fixed bug in __Pyx_Method_ClassMethod, which should follow the behaviour of the Python classmethod and accept any object then fail at runtime if it isn't callable.

@scoder
Copy link
Contributor

scoder commented Feb 29, 2020

Excellent analysis, thanks. The check function must use the correct (possibly imported) type pointer.

@da-woods
Copy link
Contributor Author

I think the check function does use the correct type pointer. It's the creation of __pyx_FusedFunctionType_type that uses the wrong type pointer.

@scoder
Copy link
Contributor

scoder commented Feb 29, 2020

Ok, got it. We can change the base pointer after importing the type pointer and before initialising the fused function type.

da-woods added a commit to da-woods/cython that referenced this issue Mar 1, 2020
Issue cython#3384

This happened where Cython was using a shared API because the
CyFunction type imported from the shared api wasn't the same as
used to initialize the fused function tp_base
scoder pushed a commit that referenced this issue Mar 7, 2020
…3386)

* Fixed issue where fused functions didn't register as cyfunctions

Issue #3384

This happened where Cython was using a shared API because the
CyFunction type imported from the shared api wasn't the same as
used to initialize the fused function tp_base.
@scoder scoder added this to the 0.29.16 milestone Mar 7, 2020
@scoder
Copy link
Contributor

scoder commented Mar 7, 2020

Closed in #3386

@scoder scoder closed this as completed Mar 7, 2020
scoder pushed a commit that referenced this issue Mar 7, 2020
…3386)

* Fixed issue where fused functions didn't register as cyfunctions

Issue #3384

This happened where Cython was using a shared API because the
CyFunction type imported from the shared api wasn't the same as
used to initialize the fused function tp_base.
alexhenrie pushed a commit to alexhenrie/cython that referenced this issue Mar 25, 2020
…honGH-3386)

* Fixed issue where fused functions didn't register as cyfunctions

Issue cython#3384

This happened where Cython was using a shared API because the
CyFunction type imported from the shared api wasn't the same as
used to initialize the fused function tp_base.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants