Cython fixes for compatibility with current compilers #595
Conversation
The new approach follows this example: <https://cython.readthedocs.io/en/latest/src/tutorial/numpy.html#adding-types> The C version is a macro with control flow that Cython does not know anything about. It introduces a C type error. This fixes a build failure with some compilers: src/_geoslib.c: In function ‘__pyx_pymod_exec__geoslib’: src/_geoslib.c:8755:3: error: returning ‘void *’ from a function with return type ‘int’ makes integer from pointer without a cast 8755 | import_array(); | ^~~~~~~~~~~~
The C types GEOSGeom and GEOSCoordSeq are themselves pointer types.
The underlying struct typedefs are called GEOSGeometry and
GEOSCoordSequence.
This fixes C compilation errors with future compilers:
src/_geoslib.c:6918:28: error: passing argument 1 of ‘GEOSCoordSeq_setY’ from incompatible pointer type
6918 | (void)(GEOSCoordSeq_setY(__pyx_v_cs, 0, __pyx_v_dy));
| ^~~~~~~~~~
| |
| GEOSCoordSequence ** {aka struct GEOSCoordSeq_t **}
/usr/include/geos_c.h:2194:58: note: expected ‘GEOSCoordSequence *’ {aka ‘struct GEOSCoordSeq_t *’} but argument is of type ‘GEOSCoordSequence **’ {aka ‘struct GEOSCoordSeq_t **’}
2194 | extern int GEOS_DLL GEOSCoordSeq_setY(GEOSCoordSequence* s,
| ~~~~~~~~~~~~~~~~~~~^
This avoids lots of C compiler warnings.
The function pointers passed to initGEOS() did not have the correct
type. A proper fix needs <stdarg.h> support in Cython, which is
currently missing.
This fixes a build failure with Clang 16 and GCC 14.
src/_geoslib.c: In function ‘__pyx_pymod_exec__geoslib’:
src/_geoslib.c:8803:12: error: passing argument 1 of ‘initGEOS’ from incompatible pointer type
8803 | initGEOS(__pyx_f_8_geoslib_notice_h, __pyx_f_8_geoslib_error_h);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
| |
| void (*)(char *, char *)
In file included from src/_geoslib.c:1219:
/usr/include/geos_c.h:2074:24: note: expected ‘GEOSMessageHandler’ {aka ‘void (*)(const char *, ...)’} but argument is of type ‘void (*)(char *, char *)’
2074 | GEOSMessageHandler notice_function,
| ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
src/_geoslib.c:8803:40: error: passing argument 2 of ‘initGEOS’ from incompatible pointer type
8803 | initGEOS(__pyx_f_8_geoslib_notice_h, __pyx_f_8_geoslib_error_h);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
| |
| void (*)(char *, char *)
/usr/include/geos_c.h:2075:24: note: expected ‘GEOSMessageHandler’ {aka ‘void (*)(const char *, ...)’} but argument is of type ‘void (*)(char *, char *)’
2075 | GEOSMessageHandler error_function);
| ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
|
Thanks for this important contribution, @fweimer-rh! I think I have already been hit by these problems when building the The current GitHub workflows are passing, so everything looks smooth after your changes. Please let me have one last check in the evening (after my work), just to understand all the changes better. |
|
I only have a question about the change of the struct names: They used to be For example, I see that all the method-like functions still start with |
|
I had some time to review the original GEOS C headers and of course you were right with the name changes. Thanks again! |
Both Clang 16 and (future) GCC 14 reject some invalid C due to Cython-level type errors. These changes should improve compiler compatibility.
Related to: