Subclassing cdef inline methods

[Stefan]

Dear all,

In moving to Cython 0.17 (Sage 5.1 to 5.2) some of our code broke due to changed handling of "cdef inline" methods. I made a small example here:
http://www.math.princeton.edu/~svanzwam/cytest.tar.gz

The relevant code is the following:

# foo/B.pyx
cdef class B:
    cdef inline int makedo(self):
        return 2
       
    cpdef work(self, n):
        return n + self.makedo()

# foo/A.pyx
cdef class A(B):
    cdef inline int makedo(self):
        return 5

Running this from sage 5.1 we get:

sage: import foo.A
sage: import foo.B
sage: a = foo.A.A()
sage: b = foo.B.B()
sage: b.work(1)
3
sage: a.work(1)
6

Running the same code from sage 5.2.rc0 we get:

sage: import foo.A
sage: import foo.B
sage: a = foo.A.A()
sage: b = foo.B.B()
sage: b.work(1)
3
sage: a.work(1)
3

So in Cython 0.17, the class A calls B's makedo(), rather than its own makedo(). I think this is a bug.

Regards,

Stefan.

[Robert Bradshaw]

The code shouldn't compile; inline methods are implicitly final.
Previously, the inline keyword was simply ignored on methods, which is
why it used to work. This should be a compile error (and indeed it is
if A and B are in the same file).

- Robert

[Robert Bradshaw]

On Fri, Jul 20, 2012 at 10:43 AM, Robert Bradshaw <robe...@gmail.com> wrote:
> The code shouldn't compile; inline methods are implicitly final.
> Previously, the inline keyword was simply ignored on methods, which is
> why it used to work. This should be a compile error (and indeed it is
> if A and B are in the same file).

I pushed a fix:
https://github.com/robertwb/cython/commit/cffa68ccaf6fc058f1e79811cfd5f761abf38be6

This means you need to fix your code. The safe thing to do is removal
all inline methods that you override, as the two are mutually
exclusive by definition. (When Cython 0.17 is actually released, I'll
update the spkg.)

[Stefan]

This means you need to fix your code. The safe thing to do is removal
all inline methods that you override, as the two are mutually
exclusive by definition. (When Cython 0.17 is actually released, I'll
update the spkg.)

Thanks a lot for your quick reply! If the "inline" keyword was ignored in the past as you claim, I guess the speed loss my co-coder worried about is nonexistent.

And while I'm writing: some time ago I posted a compiler crash on this list. That still occurs in the latest version. My report is here:

https://groups.google.com/d/topic/cython-users/tsh1u6N0wfs/discussion

I haven't had a comment on it.

Cheers,

Stefan.

[Robert Bradshaw]

I don't have an idea on that one, other than yes, it looks like a bug.
However, forward declarations shouldn't be needed in recent versions
of Cython.

- Robert