Repository: cython/cython
Pull request: Fix type of self argument in cdef staticmethod declared in a pxd file
Fixes https://github.com/cython/cython/issues/3174

I've removed all assignment of `is_self_arg` out of the parser, since it can be overridden in too many ways so I don't think it's usefully done there. It's possible that the tests will show I've missed something important where its used (in which case I can undo the change)

I took the tests from @navytux's https://github.com/cython/cython/pull/3175/ (thanks!), so this PR will supercede that one

Problem statement:
@staticmethod with implicit object argument type is handled wrongly
Hello up there. Please consider the following example:

---- 8< ---- `moda.pxd`
```pyx
cdef class MyClass:
    @staticmethod
    cdef static_func(x)
```

---- 8< ---- `moda.pyx`
```pyx
cdef class MyClass:
    @staticmethod
    cdef static_func(x):
        return x+1
```

---- 8< ---- `modb.pyx`
```pyx
from moda cimport MyClass

def test():
    x = MyClass.static_func(2)
    print(x)
```

When compiled and run it fails with:

```
$ python -c 'import modb; modb.test()'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "modb.pyx", line 4, in modb.test
    x = MyClass.static_func(2)
TypeError: Cannot convert int to moda.MyClass
```

If I add explicit `object` to `x` argument in `moda.pxd`:

```diff
--- a/moda.pxd
+++ b/moda.pxd
@@ -1,3 +1,3 @@
 cdef class MyClass:
     @staticmethod
-    cdef static_func(x)
+    cdef static_func(object x)

```

It works:

```
$ python -c 'import modb; modb.test()'
3
```

Thanks beforehand,
Kirill

/cc @robertwb

Hints from issue discussion:
I forgot to mention that it fails for both current 0.29.x and master.
I added corresponding test in https://github.com/cython/cython/pull/3175.
My guess is that it fails because `@staticmethod` is not evaluated (correctly) in `.pxd` files. Or maybe it incorrectly overwrites the prior handling of the decorator in the implementation file somehow.

PR for that is welcome, but I can't say what needs to be done for that. Needs some analysis to find out where this should be fixed.
@scoder, thanks for feedback. My impression is that somehow the "pass" that automatically annotates first argument of a method of a `cdef class` with corresponding type overwrites `@staticmethod` effect somehow. But I did not looked into the code at all. For now I filed PR with test (#3175, it can be already merged), this issue, so that the bug is not forgotten, and in my codebase worked around the problem with explicitly using `object` type for the first argument.
Same issue reproduced. Any updates on this?

Commit message: Fix merge

Somehow deleted a colon

Patch to review:
diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py
index ad740a379af..a11a496ef46 100644
--- a/Cython/Compiler/Nodes.py
+++ b/Cython/Compiler/Nodes.py
@@ -901,8 +901,7 @@ def hdr_cname(self):
 
 
     def analyse(self, env, nonempty=0, is_self_arg=False):
-        if is_self_arg:
-            self.base_type.is_self_arg = self.is_self_arg = True
+        self.base_type.is_self_arg = self.is_self_arg = is_self_arg
         if self.type is not None:
             return self.name_declarator, self.type
 
@@ -1024,6 +1023,7 @@ class CSimpleBaseTypeNode(CBaseTypeNode):
     module_path = []
     is_basic_c_type = False
     complex = False
+    is_self_arg = False
 
     def analyse(self, env, could_be_name=False):
         # Return type descriptor.
diff --git a/Cython/Compiler/Parsing.py b/Cython/Compiler/Parsing.py
index a1a3a739219..1dfef929073 100644
--- a/Cython/Compiler/Parsing.py
+++ b/Cython/Compiler/Parsing.py
@@ -2477,13 +2477,11 @@ def p_positional_and_keyword_args(s, end_sy_set, templates = None):
         s.next()
     return positional_args, keyword_args
 
-def p_c_base_type(s, self_flag = 0, nonempty = 0, templates = None):
-    # If self_flag is true, this is the base type for the
-    # self argument of a C method of an extension type.
+def p_c_base_type(s, nonempty = 0, templates = None):
     if s.sy == '(':
         return p_c_complex_base_type(s, templates = templates)
     else:
-        return p_c_simple_base_type(s, self_flag, nonempty = nonempty, templates = templates)
+        return p_c_simple_base_type(s, nonempty = nonempty, templates = templates)
 
 def p_calling_convention(s):
     if s.sy == 'IDENT' and s.systring in calling_convention_words:
@@ -2527,8 +2525,7 @@ def p_c_complex_base_type(s, templates = None):
     return type_node
 
 
-def p_c_simple_base_type(s, self_flag, nonempty, templates = None):
-    #print "p_c_simple_base_type: self_flag =", self_flag, nonempty
+def p_c_simple_base_type(s, nonempty, templates = None):
     is_basic = 0
     signed = 1
     longness = 0
@@ -2549,7 +2546,7 @@ def p_c_simple_base_type(s, self_flag, nonempty, templates = None):
             break
         s.next()
     if is_const or is_volatile:
-        base_type = p_c_base_type(s, self_flag=self_flag, nonempty=nonempty, templates=templates)
+        base_type = p_c_base_type(s, nonempty=nonempty, templates=templates)
         if isinstance(base_type, Nodes.MemoryViewSliceTypeNode):
             # reverse order to avoid having to write "(const int)[:]"
             base_type.base_type_node = Nodes.CConstOrVolatileTypeNode(pos,
@@ -2607,7 +2604,7 @@ def p_c_simple_base_type(s, self_flag, nonempty, templates = None):
         name = name, module_path = module_path,
         is_basic_c_type = is_basic, signed = signed,
         complex = complex, longness = longness,
-        is_self_arg = self_flag, templates = templates)
+        templates = templates)
 
     #    declarations here.
     if s.sy == '[':
@@ -3052,7 +3049,7 @@ def p_c_arg_decl(s, ctx, in_pyfunc, cmethod_flag = 0, nonempty = 0,
             complex = 0, longness = 0,
             is_self_arg = cmethod_flag, templates = None)
     else:
-        base_type = p_c_base_type(s, cmethod_flag, nonempty = nonempty)
+        base_type = p_c_base_type(s, nonempty = nonempty)
     declarator = p_c_declarator(s, ctx, nonempty = nonempty)
     if s.sy in ('not', 'or') and not s.in_python_file:
         kind = s.sy
diff --git a/tests/run/static_methods.pxd b/tests/run/static_methods.pxd
index 4dc19995586..383808149ef 100644
--- a/tests/run/static_methods.pxd
+++ b/tests/run/static_methods.pxd
@@ -1,3 +1,6 @@
 cdef class FromPxd:
     @staticmethod
     cdef static_cdef(int* x)
+
+    @staticmethod
+    cdef static_cdef_with_implicit_object(obj)
diff --git a/tests/run/static_methods.pyx b/tests/run/static_methods.pyx
index ba7df379db7..9a8c9ea3fdd 100644
--- a/tests/run/static_methods.pyx
+++ b/tests/run/static_methods.pyx
@@ -87,6 +87,10 @@ cdef class FromPxd:
     cdef static_cdef(int* x):
         return 'pxd_cdef', x[0]
 
+    @staticmethod
+    cdef static_cdef_with_implicit_object(obj):
+        return obj+1
+
 def call_static_pxd_cdef(int x):
     """
     >>> call_static_pxd_cdef(2)
@@ -94,3 +98,11 @@ def call_static_pxd_cdef(int x):
     """
     cdef int *x_ptr = &x
     return FromPxd.static_cdef(x_ptr)
+
+def call_static_pxd_cdef_with_implicit_object(int x):
+    """
+    # currently fails: https://github.com/cython/cython/issues/3174
+    >>> call_static_pxd_cdef_with_implicit_object(2)
+    3
+    """
+    return FromPxd.static_cdef_with_implicit_object(x)


Code review comments:
diff_hunk: @@ -2477,13 +2477,11 @@ def p_positional_and_keyword_args(s, end_sy_set, templates = None):
         s.next()
     return positional_args, keyword_args
 
-def p_c_base_type(s, self_flag = 0, nonempty = 0, templates = None):
-    # If self_flag is true, this is the base type for the
-    # self argument of a C method of an extension type.
+def p_c_base_type(s, nonempty = 0, templates = None):
path: Cython/Compiler/Parsing.py
text: @user1:
Whenever we touch such code, it's a good time to clean it up along the way.
```suggestion
def p_c_base_type(s, nonempty=False, templates=None):
```
Note that you need to change `Parsing.pxd` as well (we should have a test job that exercises the `--cython-compile-more` build option – would also be interesting to see if it makes a performance dfference).

@user1:
I'm working on adding CI jobs. Seems we've piled up some more differences in the past already.

diff_hunk: @@ -2527,8 +2525,7 @@ def p_c_complex_base_type(s, templates = None):
     return type_node
 
 
-def p_c_simple_base_type(s, self_flag, nonempty, templates = None):
-    #print "p_c_simple_base_type: self_flag =", self_flag, nonempty
+def p_c_simple_base_type(s, nonempty, templates = None):
path: Cython/Compiler/Parsing.py
text: @user1:
```suggestion
def p_c_simple_base_type(s, nonempty, templates=None):
```

diff_hunk: @@ -2477,13 +2477,11 @@ def p_positional_and_keyword_args(s, end_sy_set, templates = None):
         s.next()
     return positional_args, keyword_args
 
-def p_c_base_type(s, self_flag = 0, nonempty = 0, templates = None):
-    # If self_flag is true, this is the base type for the
-    # self argument of a C method of an extension type.
+def p_c_base_type(s, nonempty = 0, templates = None):
     if s.sy == '(':
         return p_c_complex_base_type(s, templates = templates)
     else:
-        return p_c_simple_base_type(s, self_flag, nonempty = nonempty, templates = templates)
+        return p_c_simple_base_type(s, nonempty = nonempty, templates = templates)
path: Cython/Compiler/Parsing.py
text: @user1:
```suggestion
        return p_c_simple_base_type(s, nonempty=nonempty, templates=templates)
```

diff_hunk: @@ -901,8 +901,7 @@ def hdr_cname(self):
 
 
     def analyse(self, env, nonempty=0, is_self_arg=False):
-        if is_self_arg:
-            self.base_type.is_self_arg = self.is_self_arg = True
+        self.base_type.is_self_arg = self.is_self_arg = is_self_arg
path: Cython/Compiler/Nodes.py
text: @user1:
Can we keep the `if` test? Given the default class attributes, it doesn't seem necessary to always set it (and that avoids an unnecessary attribute in the instance dicts).

diff_hunk: @@ -3052,7 +3049,7 @@ def p_c_arg_decl(s, ctx, in_pyfunc, cmethod_flag = 0, nonempty = 0,
             complex = 0, longness = 0,
             is_self_arg = cmethod_flag, templates = None)
     else:
-        base_type = p_c_base_type(s, cmethod_flag, nonempty = nonempty)
+        base_type = p_c_base_type(s, nonempty = nonempty)
path: Cython/Compiler/Parsing.py
text: @user1:
```suggestion
        base_type = p_c_base_type(s, nonempty=nonempty)
```