poppopjmp / shedskin

Automatically exported from code.google.com/p/shedskin
0 stars 0 forks source link

Subclasses fail type conversion #92

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
The test case is rather a lot of code at the moment.  If you actually need one, 
let me know and I'll get you one, but reducing it is a bit of a pain, and I'm 
hoping this will be enough info to see the problem without an actual repro.

When you've got multiple subclasses, say Tri and Sphere, deriving from a parent 
type, Shape, and a function that takes a list of T (for example, [sphere0, 
sphere1, tri0, tri1]), you'll get "error in conversion to Shed Skin (Shape 
expected)".  It looks like the conversion function __to_ss() just doesn't 
acknowledge inheritance at all.  Here's the generated function:

template<> __Shape__::Shape *__to_ss(PyObject *p) {
    if(p == Py_None) return NULL;
    if(p->ob_type != &__Shape__::ShapeObjectType)
        throw new TypeError(new str("error in conversion to Shed Skin (Shape expected)"));
    return ((__Shape__::ShapeObject *)p)->__ss_object;
}

By changing the error string to p->ob_type->tp_name, I've verified that I'm 
actually passing in the subclass Sphere when the error happens.

Perhaps you can maintain the type tree and walk up it when doing this check?

Original issue reported on code.google.com by uran...@gmail.com on 19 Aug 2010 at 5:52

GoogleCodeExporter commented 8 years ago
yes, I think so, and there's probably some useful C API function for this. I 
will have to set base class pointers on the CPython side though, because those 
are also still missing.. :)

is there any chance I could see your whole program later on, perhaps even 
adding it as an example program?

Original comment by mark.duf...@gmail.com on 19 Aug 2010 at 8:04

GoogleCodeExporter commented 8 years ago
I'd be happy to give it to you privately.  For now it's still very unfinished, 
and I'd be embarrassed to publish it in its current state.

I haven't ported it all to shedskin yet.  I've done about 5 of 8 files, and was 
going to wait for this issue to be fixed before going further--this is the only 
blocker of the ones I just posted.  If you'd like, I can send you what I have 
now with some notes on what's done.  Those 5 files together constitute an 
ungainly but effective test case for this issue.

Oddly, the trivial changes needed to get those files to compile have slowed the 
interpreted execution down by a factor of 5!  I'll have to dig into it to find 
out why; I didn't think I'd done anything significant.

Drop me a line at uranium+shedskin at gmail if you want the current source.

Original comment by uran...@gmail.com on 19 Aug 2010 at 8:11

GoogleCodeExporter commented 8 years ago
I was just wondering if I could see it at some point. it's always nicer to work 
with code that is or will be open source.

I think I know how to solve this, so I will try to fix it and ask you to verify 
if it works for you.

Original comment by mark.duf...@gmail.com on 19 Aug 2010 at 8:35

GoogleCodeExporter commented 8 years ago
Yeah, sure.  I'll be happy to share it with you later.

Thanks!

Original comment by uran...@gmail.com on 19 Aug 2010 at 8:43

GoogleCodeExporter commented 8 years ago
Side note: that 5x slowdown was due to a simple error on my part.

Original comment by uran...@gmail.com on 20 Aug 2010 at 4:37

GoogleCodeExporter commented 8 years ago
I'm afraid I will be away on all of my three free days this week, but I managed 
to setup the base class pointers, and use PyObject_IsInstance to do the type 
check, before leaving today.. please let me know how this works for you. it 
works fine for a small example with two classes, but I can't believe it would 
be this easy.

Original comment by mark.duf...@gmail.com on 21 Aug 2010 at 8:20

GoogleCodeExporter commented 8 years ago
(note that I'm probably starting to break some examples here - no time to test 
and fix everything. I will fix those later, and finally start a test set for 
extmod examples).

Original comment by mark.duf...@gmail.com on 21 Aug 2010 at 8:24

GoogleCodeExporter commented 8 years ago
No joy:

uranium@Slick:/tmp/backup29$ /usr/bin/shedskin -e -m Shape.make Shape.py
*** SHED SKIN Python-to-C++ Compiler 0.6 ***
Copyright 2005-2010 Mark Dufour; License GNU GPL version 3 (See LICENSE)

[iterative type analysis..]
***
iterations: 3 templates: 696
[generating c++ code..]
uranium@Slick:/tmp/backup29$ make -f Shape.make 
g++  -O0 -pipe -Wno-deprecated  -I. 
-I/usr/lib/python2.5/site-packages/shedskin/lib -g -fPIC -D__SS_BIND 
-I/usr/include/python2.5 -I/usr/include/python2.5 
/usr/lib/python2.5/site-packages/shedskin/lib/string.cpp Color.cpp Utils.cpp 
Vector4.cpp Shape.cpp /usr/lib/python2.5/site-packages/shedskin/lib/builtin.cpp 
/usr/lib/python2.5/site-packages/shedskin/lib/copy.cpp 
/usr/lib/python2.5/site-packages/shedskin/lib/math.cpp Ray.cpp 
/usr/lib/python2.5/site-packages/shedskin/lib/re.cpp -lgc -lpcre  -shared 
-Xlinker -export-dynamic -lpthread -ldl  -lutil -lm -lpython2.5 
-L/usr/lib/python2.5/config -o Shape.so
Shape.cpp:694: error: ‘objectObjectType’ was not declared in this scope
Shape.cpp:1035: error: ‘objectObjectType’ was not declared in this scope
Shape.cpp:1358: error: ‘ShapeObjectType’ was not declared in this scope
Shape.cpp:1622: error: ‘ShapeObjectType’ was not declared in this scope
Shape.cpp:1838: error: ‘objectObjectType’ was not declared in this scope
Shape.cpp:2273: error: ‘objectObjectType’ was not declared in this scope
Shape.cpp:2427: error: ‘objectObjectType’ was not declared in this scope

The errors are for objectObjectType being used in various PyTypeObjects' 
tp_base members, and for ShapeObjectType being used in the same field, as the 
parent class of various classes derived from Shape.  I'll see if I can generate 
a simple set of test inputs for you.

Original comment by uran...@gmail.com on 24 Aug 2010 at 3:29

GoogleCodeExporter commented 8 years ago
Here's a reasonably-short test case.

Original comment by uran...@gmail.com on 24 Aug 2010 at 4:11

Attachments:

GoogleCodeExporter commented 8 years ago
thanks for testing! as I thought, I forgot some details.. I will probably be 
able to fix them later today.

Original comment by mark.duf...@gmail.com on 24 Aug 2010 at 6:25

GoogleCodeExporter commented 8 years ago
alright, the example works now. thanks again for testing. please let me know if 
you run into anything else.

Original comment by mark.duf...@gmail.com on 24 Aug 2010 at 3:07

GoogleCodeExporter commented 8 years ago
It complies the real file now too; I'll have to port a few more files to see 
how it goes, but I've got no more blockers.  I'll be offline for a bit over a 
week, though, so don't be surprised by a bit of silence.

Thanks!

Original comment by uran...@gmail.com on 25 Aug 2010 at 3:27

GoogleCodeExporter commented 8 years ago
I've fixed up some other files, and now trying to run my app I'm seeing a 
problem with the new code.

Here's what you generate:

template<> __Vector4__::Vector4 *__to_ss(PyObject *p) {
    if(p == Py_None) return NULL;
    if(PyObject_IsInstance(p, (PyObject *)&__Vector4__::Vector4ObjectType)!=1)
        throw new TypeError(new str("error in conversion to Shed Skin (Vector4 expected)"));
    return ((__Vector4__::Vector4Object *)p)->__ss_object;
}

I'm getting that TypeError thrown when I'm pretty sure I'm passing in the right 
type [although it will be None some of the time, it shouldn't ever be a 
non-None object of a different type].  When I alter the code to this:

template<> __Vector4__::Vector4 *__to_ss(PyObject *p) {
    if(p == Py_None) return NULL;
    if(p->ob_type == &__Vector4__::Vector4ObjectType)
      return ((__Vector4__::Vector4Object *)p)->__ss_object;

    if(PyObject_IsInstance(p, (PyObject *)&__Vector4__::Vector4ObjectType)!=1)
      throw new TypeError(new str(p->ob_type->tp_name));
    return ((__Vector4__::Vector4Object *)p)->__ss_object;
}

...in order to figure out the type of whatever I've passed in, I see this error:

TypeError: Vector4.Vector4

So the object passed in /is/ a Vector4.Vector4, but it's throwing anyway.  So I 
think something's not set up right for the call to PyObject_IsInstance.

Let me know if you need the test case; it's a whole bunch of files, 
unfortunately, but I can mail them to you.  I won't have time to shrink it down 
this week, and I'm gone all next week, I'm afraid.

Original comment by uran...@gmail.com on 25 Aug 2010 at 4:40

GoogleCodeExporter commented 8 years ago
I will try to figure it out this weekend, but please do send me the files, in 
case I won't be able to reproduce the problem. thanks!

Original comment by mark.duf...@gmail.com on 28 Aug 2010 at 8:57

GoogleCodeExporter commented 8 years ago
I haven't been able to reproduce the problem, unfortunately. please do send me 
the files. it's fine if there's a lot of code..

Original comment by mark.duf...@gmail.com on 7 Sep 2010 at 8:38

GoogleCodeExporter commented 8 years ago
okay, so we solved the second problem over email, and started a shared-library 
approach in a new 'extmods' branch. the approach appears to work well, which is 
nice to know, but there may be some challenges merging this back into master 
and in distributing something that's easily usable. closing this issue then.

Original comment by mark.duf...@gmail.com on 28 Sep 2010 at 10:08