Hello all, First, I tried to post this earlier but am pretty sure it didn't go through. If this is a double, I apologize. This comes out of a discussion on the cython-users list. I am getting some warnings when using cython to compile a python module and then calling into the module from c. I was able to condense the problem down to a very succinct example. There are three files of interest: test.pyx: ---------------------------------- cdef class foo: cdef int a cdef int b def __init__(foo self, int a, int b): self.a = a self.b = b cdef public void bar(foo x): print "My_foo", x.a, x.b cdef public foo make_foo(int a, int b): return foo(a, b) ------------------------------------------ setup.py: ------------------------------------------ from distutils.core import setup from distutils.extension import Extension from Cython.Distutils import build_ext ext_modules = [Extension("test", ["test.pyx"])] setup( name = 'Hello world app', cmdclass = {'build_ext': build_ext}, ext_modules = ext_modules ) ------------------------------------------ and dummy.c: ----------------------------------------- #include "Python.h" #include "test.h" int main(int argc, char** argv){ Py_Initialize(); inittest(); bar(make_foo(1,2)); return 0; } --------------------------------------- I use the following commands to compile and link: python setup.py build_ext --inplace gcc -I/usr/include/python2.6 -lpython2.6 -L/usr/include/python2.6/config -c dummy.c The second command generates the following error messages: In file included from dummy.c:2: test.h:11: warning: 'struct __pyx_obj_4test_foo' declared inside parameter list test.h:11: warning: its scope is only this definition or declaration, which is probably not what you want dummy.c: In function 'main': dummy.c:7: warning: passing argument 1 of 'bar' from incompatible pointer type test.h:11: note: expected 'struct __pyx_obj_4test_foo *' but argument is of type 'struct __pyx_obj_4test_foo *' After a bit of digging, I found the problem: The definition for the struct in question was created in test.c, not test.h. GCC is therefore treating the parameter in test.h as a completely separate (and empty) struct declaration. Luckily, in this case struct pointers are being used, so enough mem gets allocated, etc. and everything actually ends up working. That being said, the compiler errors are quite bothersome, and it may be that this could cause real errors in more complex programs with more interdependencies of types. I would like to propose as a fix that the struct declaration be generated inside the header file instead of the .c file (and on a side note, I would personally prefer that the #includes be placed in the header file as well). Thanks for all the work that has been put into this project. Cython is really quite a remarkable tool! -Seth
On Sat, Jun 4, 2011 at 10:36 AM, Seth Shannin <sshannin@stwing.upenn.edu> wrote:
Hello all,
First, I tried to post this earlier but am pretty sure it didn't go through. If this is a double, I apologize.
This comes out of a discussion on the cython-users list. I am getting some warnings when using cython to compile a python module and then calling into the module from c.
I was able to condense the problem down to a very succinct example.
There are three files of interest:
test.pyx: ---------------------------------- cdef class foo: cdef int a cdef int b def __init__(foo self, int a, int b): self.a = a self.b = b
cdef public void bar(foo x): print "My_foo", x.a, x.b
cdef public foo make_foo(int a, int b): return foo(a, b) ------------------------------------------
setup.py: ------------------------------------------ from distutils.core import setup from distutils.extension import Extension from Cython.Distutils import build_ext
ext_modules = [Extension("test", ["test.pyx"])]
setup( name = 'Hello world app', cmdclass = {'build_ext': build_ext}, ext_modules = ext_modules ) ------------------------------------------ and dummy.c: ----------------------------------------- #include "Python.h" #include "test.h"
int main(int argc, char** argv){ Py_Initialize(); inittest(); bar(make_foo(1,2)); return 0; } ---------------------------------------
I use the following commands to compile and link: python setup.py build_ext --inplace gcc -I/usr/include/python2.6 -lpython2.6 -L/usr/include/python2.6/config -c dummy.c
The second command generates the following error messages: In file included from dummy.c:2: test.h:11: warning: 'struct __pyx_obj_4test_foo' declared inside parameter list test.h:11: warning: its scope is only this definition or declaration, which is probably not what you want dummy.c: In function 'main': dummy.c:7: warning: passing argument 1 of 'bar' from incompatible pointer type test.h:11: note: expected 'struct __pyx_obj_4test_foo *' but argument is of type 'struct __pyx_obj_4test_foo *'
After a bit of digging, I found the problem: The definition for the struct in question was created in test.c, not test.h. GCC is therefore treating the parameter in test.h as a completely separate (and empty) struct declaration. Luckily, in this case struct pointers are being used, so enough mem gets allocated, etc. and everything actually ends up working.
That being said, the compiler errors are quite bothersome, and it may be that this could cause real errors in more complex programs with more interdependencies of types.
True. Cython doesn't usually get used this way, so thanks for the report. BTW, for embedding the --embed option is usually easier to use.
I would like to propose as a fix that the struct declaration be generated inside the header file instead of the .c file (and on a side note, I would personally prefer that the #includes be placed in the header file as well).
Thanks for all the work that has been put into this project. Cython is really quite a remarkable tool!
-Seth
_______________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
On Sat, Jun 4, 2011 at 10:36 AM, Seth Shannin <sshannin@stwing.upenn.edu> wrote:
test.h:11: warning: 'struct __pyx_obj_4test_foo' declared inside parameter list test.h:11: warning: its scope is only this definition or declaration, which is probably not what you want
Not sure about Cython, but the way to to this in Pyrex is to declare the class 'public' as well, and then the struct declaration for it will be put in the .h file. You will need to supply C names for the struct and type object as well, e.g. cdef public class foo[type FooType, object FooObject]: ... The generated test.h file then contains struct FooObject { PyObject_HEAD int a; int b; }; __PYX_EXTERN_C DL_IMPORT(void) bar(struct FooObject *); __PYX_EXTERN_C DL_IMPORT(struct FooObject) *make_foo(int,int); -- Greg
Ah, that's what it means. I saw something similar to this somewhere in some documentation, but the object keyword threw me off. To me, 'object' indicates something more on the python side than on the c side. Also, even though you can do this to ensure the struct declaration ends up in the header file, I would still make the case that currently something is a bit off in that the header file tries to use something which it has no information about (and in the process actually ends up creating its own empty struct type). Thanks for all the help. -Seth On 06/07/2011 06:24 PM, Greg Ewing wrote:
On Sat, Jun 4, 2011 at 10:36 AM, Seth Shannin <sshannin@stwing.upenn.edu> wrote:
test.h:11: warning: 'struct __pyx_obj_4test_foo' declared inside parameter list test.h:11: warning: its scope is only this definition or declaration, which is probably not what you want
Not sure about Cython, but the way to to this in Pyrex is to declare the class 'public' as well, and then the struct declaration for it will be put in the .h file.
You will need to supply C names for the struct and type object as well, e.g.
cdef public class foo[type FooType, object FooObject]: ...
The generated test.h file then contains
struct FooObject { PyObject_HEAD int a; int b; };
__PYX_EXTERN_C DL_IMPORT(void) bar(struct FooObject *); __PYX_EXTERN_C DL_IMPORT(struct FooObject) *make_foo(int,int);
Seth Shannin wrote:
I would still make the case that currently something is a bit off in that the header file tries to use something which it has no information about
Yes, I agree that it could do with improvement. I'm not sure that automatically putting the struct declaration in the header is the best idea, though, since with mangled names it's hard to do anything with it from external C code (that's why you're required to manually supply type and object struct names). Maybe it should just be an error to make a declaration public if it uses something that's non-public? -- Greg
Yeah, I think just erroring would be perfectly reasonable. -Seth Quoting "Greg Ewing" <greg.ewing@canterbury.ac.nz>:
Seth Shannin wrote:
I would still make the case that currently something is a bit off in that the header file tries to use something which it has no information about
Yes, I agree that it could do with improvement. I'm not sure that automatically putting the struct declaration in the header is the best idea, though, since with mangled names it's hard to do anything with it from external C code (that's why you're required to manually supply type and object struct names).
Maybe it should just be an error to make a declaration public if it uses something that's non-public?
-- Greg _______________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
I agree. On Wed, Jun 8, 2011 at 9:47 AM, <sshannin@stwing.upenn.edu> wrote:
Yeah, I think just erroring would be perfectly reasonable.
-Seth
Quoting "Greg Ewing" <greg.ewing@canterbury.ac.nz>:
Seth Shannin wrote:
I would still make the case that currently something is a bit off in that the header file tries to use something which it has no information about
Yes, I agree that it could do with improvement. I'm not sure that automatically putting the struct declaration in the header is the best idea, though, since with mangled names it's hard to do anything with it from external C code (that's why you're required to manually supply type and object struct names).
Maybe it should just be an error to make a declaration public if it uses something that's non-public?
-- Greg _______________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
_______________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
One note: I personally found the documentation on how to make a class declaration public fairly hard to find. It could be nice to make this clearer. -Seth On 06/09/2011 01:57 AM, Robert Bradshaw wrote:
I agree.
On Wed, Jun 8, 2011 at 9:47 AM, <sshannin@stwing.upenn.edu> wrote:
Yeah, I think just erroring would be perfectly reasonable.
-Seth
Quoting "Greg Ewing" <greg.ewing@canterbury.ac.nz>:
Seth Shannin wrote:
I would still make the case that currently something is a bit off in that the header file tries to use something which it has no information about
Yes, I agree that it could do with improvement. I'm not sure that automatically putting the struct declaration in the header is the best idea, though, since with mangled names it's hard to do anything with it from external C code (that's why you're required to manually supply type and object struct names).
Maybe it should just be an error to make a declaration public if it uses something that's non-public?
-- Greg _______________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
_______________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
_______________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
I agree. We welcome improvement to our documentation--fork and make a push request: https://github.com/cython/cython/tree/master/docs On Thu, Jun 9, 2011 at 4:49 AM, Seth Shannin <sshannin@stwing.upenn.edu> wrote:
One note: I personally found the documentation on how to make a class declaration public fairly hard to find. It could be nice to make this clearer.
-Seth
On 06/09/2011 01:57 AM, Robert Bradshaw wrote:
I agree.
On Wed, Jun 8, 2011 at 9:47 AM, <sshannin@stwing.upenn.edu> wrote:
Yeah, I think just erroring would be perfectly reasonable.
-Seth
Quoting "Greg Ewing" <greg.ewing@canterbury.ac.nz>:
Seth Shannin wrote:
I would still make the case that currently something is a bit off in that the header file tries to use something which it has no information about
Yes, I agree that it could do with improvement. I'm not sure that automatically putting the struct declaration in the header is the best idea, though, since with mangled names it's hard to do anything with it from external C code (that's why you're required to manually supply type and object struct names).
Maybe it should just be an error to make a declaration public if it uses something that's non-public?
-- Greg _______________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
_______________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
_______________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
_______________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
participants (4)
-
Greg Ewing -
Robert Bradshaw -
Seth Shannin -
sshannin@stwing.upenn.edu