Missing virtual qualifier - Different behaviour between CPython and Shedskin

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Execute main.py. Notice that it works and the asserts are passed
2. Generate module for blah.py
3. Execute

What is the expected output? What do you see instead?
The expected output (like CPython):
<blah.A object at 0xb74b178c> <blah.B object at 0xb74b17cc>
I'm A
I'm B

Actual output:
<blah.A object at 0xb74b1120> <blah.B object at 0xb74b1140>
I'm Base
I'm Base
Traceback (most recent call last):
  File "main.py", line 14, in <module>
    assert both.say_a() == "I'm A"
AssertionError

What version of the product are you using? On what operating system?
Shedskin 0.9.3
Linux Netbook 3.5.0-25-generic #39-Ubuntu SMP Mon Feb 25 19:02:34 UTC 2013 i686 
i686 i686 GNU/Linux

Please provide any additional information below.

I'm not sure here, but I think this should work. I know that the virtual 
qualifier works on other scenarios so I guess this is a valid issue.

If I change the "say" method in Base to be virtual, then it works as CPython.

Regards

Original issue reported on code.google.com by ernestof...@gmail.com on 5 Mar 2013 at 11:50

[GoogleCodeExporter]

did you forget to attach main.py and blah.py..?

Original comment by mark.duf...@gmail.com on 6 Mar 2013 at 10:02

[GoogleCodeExporter]

Oops. I did. I even remember adding them.
I'll upload them in two hours when I'm home.

Regards

Original comment by ernestof...@gmail.com on 6 Mar 2013 at 10:03

[GoogleCodeExporter]

I know why the files were missing. I captcha validation failed and the page was 
reloaded with all the text in the comment, but the information about the files 
did not (which is expected).
Anyway, here are the files.

Original comment by ernestof...@gmail.com on 6 Mar 2013 at 10:56

Attachments:

• blah.py
• main.py

[GoogleCodeExporter]

I see the difference between cpython and shedskin, but did you mean to use this 
in blah.py:

    both = Both(base, base)

instead of:

    both = Both(a, b)

I will think about why shedskin doesn't automatically add a virtual keyword 
though.. virtuals always hurt my brain :P

Original comment by mark.duf...@gmail.com on 7 Mar 2013 at 8:02

[GoogleCodeExporter]

this is a bit tricky, but I think it's correct. in the bottom of blah.py, Both 
is initialized with only Base objects. so there is no virtual behaviour 
(self.a.say() always works on a Base object, so the version in A and B are 
never called at this point, so we don't need a virtual keyword). 

perhaps we should just make all methods virtual that have similarly named 
methods in any subclass.... 

Original comment by mark.duf...@gmail.com on 7 Mar 2013 at 10:59

[GoogleCodeExporter]

Exactly. I actually think that it wouldn't hurt to make all methods virtual.
The reason I wrote Both(base, base) instead of Both(a, b) in the module is 
because I might call it with Both(A(), A()) or Both(A(), B()) or Both(B(), A()) 
Both(B(), B()).

To me what is tricky here is that even though the module is a subset of Python, 
if that subset is valid for shedskin and is able to generate C++ from it, I 
expect that: 1) It runs exactly like CPython or 2) It fails due to an invalid 
type.

For example, if in the module I write Both(A(), B()) instead of Both(base, 
base), this example will work. It won't behave like CPython in the sense that I 
can't call Both(b, b) but that doesn't matter because it will raise an 
exception.

I guess what I don't like about this one is that is tricky to see that 
something's wrong.

Question, what would be the disadvantage of making every method virtual? I 
think it will work and shedskin wouldn't have to check or detect for anything.

Original comment by ernestof...@gmail.com on 7 Mar 2013 at 2:02

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I'm still thinking about this whole "virtual" thing. I've been reading about 
this and I've found this line from 
http://docs.python.org/2/tutorial/classes.html.

"For C++ programmers: all methods in Python are effectively virtual"

Original comment by ernestof...@gmail.com on 7 Mar 2013 at 7:16

[GoogleCodeExporter]

well, virtuals are slower than non-virtuals, which can cause a big slowdown in 
tight loops. 

another disadvantage would be that a 
non-virtual-method-with-the-same-name-in-a-subclass will now have to have the 
same signature as the similarly named method in the superclass. 

another option might be to add more virtual keywords when generating an 
extension module.. 

I will think a bit about this. the next release of shedskin is still a few 
months away, probably.

Original comment by mark.duf...@gmail.com on 7 Mar 2013 at 7:51

[GoogleCodeExporter]

Well, it might be worth checking if that's the case. Maybe the compiler is 
smart enough to notice that it doesn't have to have to use virtual tables.

And I think (I have to check) that a subclass with a method with the same name 
as the parent, but different signature, will still work as it will be use the 
method overloading feature of C++.

Original comment by ernestof...@gmail.com on 7 Mar 2013 at 8:20