Generates invalid C when omitting self in lambda

[instance01]

I'm using Cython version 0.29.7.

Part of the error message (full message attached: output.txt):

AA/bug.c:830:57: error: expected identifier or ‘(’ before ‘.’ token
 struct __pyx_obj_2AA_3bug___pyx_scope_struct____pyx_base.func;

Here's a quick example to reproduce, including a workaround.

# cython: language_level=3
cdef class FBase():
    def __init__(self):
        pass
        
    def __cinit__(self):
        pass
        
    cdef func(self, whatever):
        pass
        

cdef class Bug(FBase):
    def __init__(self):
        pass
        
    cdef check(self, x):
        return x
        
    cdef func(self, whatever):
        a = [1, 2, 3]
        min(a, key=lambda x: self.check(x))

        # This generates correct C, but doesn't work in Python:
        # min(a, key=lambda self, x: self.check(x))

        # Workaround:
        # import functools
        # min(a, key=functools.partial(lambda self, x: self.check(x), self))

[scoder]

Yeah, that looks wrong. Would you like investigate what Cython thinks it's doing there?

[rjtobin]

It seems that this issue occurs generally whenever there is a closure inside an inherited extension class method. In ParseTreeTransforms.py, CreateClosureClasses.create_from_class_scope generates the name for the C struct for the closure. The name of this struct is set to be a string that contains node.entry.cname. The problem here is that the cname for an inherited extension class method contains a period (eg. __pyx_base.func in the example here). So the generated C code tries to put a '.' into the name of a struct, and we get the error above.

For reference, the code that sets the cname for these inherited methods is Symtab.py, CClassScope.declare_inherited_c_attributes. The period is introduced in the local function adapt().

In the closure struct generation code, if one replaces periods with an allowable string then the example compiles. See rjtobin@8b1a50c for a fix like this.

I didn't make a PR for this because I had some reservations:

1. Can replacing '.' with '__' lead to name collisions?
2. Are there other non-period characters that might occur in cnames that also need to be replaced?
3. Maybe the right change is to modify the cname itself (which would require a few other changes), or some other option I don't see.
4. Also the inline string replace is pretty ugly.

[scoder]

Thanks for your investigations. Actually, the cname isn't strictly needed here, because the name is already made unique by next_id(), which inserts a global counter, so no, no naming collisions here. Yes, the string replace is ugly, so maybe a comment would be justified, but it does fix the problem at hand, it seems, and in a simple way, so I'd be fine with that for now.

Could you make sure that your test actually works by turning it into a mode: run test (and moving it to tests/run/)?