Compiler error while building python swig

[cgsbob]

While doing a make in swig/python, I get this error:

make -C interfaces
make[1]: Entering directory `/home/bobm/src/gis/grass6_devel/swig/python/interfaces'
echo "/* auto-generate swig typedef file (with some GRASS functions removed) */" > gisdefs.i
cat ../../../include/gisdefs.h | grep -v '#\|asprintf\|G_snprintf\|G_debug\|G_fatal_error\|G_warning\|G_verbose\|lzw\|G_done_msg\|G_message\|G_important_message' >> gisdefs.i
echo "/* auto-generate swig typedef file */" > dig_structs.i
cat ../../../include/vect/dig_structs.h >> dig_structs.i
echo "/* auto-generate swig typedef file */" > vect.i
cat ../../../include/Vect.h | grep -v 'V1_\|V2_' >> vect.i
make[1]: Leaving directory `/home/bobm/src/gis/grass6_devel/swig/python/interfaces'
/usr/bin/swig -python -shadow python_grass6.i
renames.i:2: Warning(301): class keyword used, but not in C++ mode.
interfaces/gisdefs.i:116: Error: Syntax error in input(1).
make: *** [python_grass6_wrap.c] Error 1

The syntax error in interfaces/gisdefs.i points to this line:

attribute ((format(printf, 2, 3)));

[hamish]

builds fine for me. What version of python/swig/grass?

The syntax error in interfaces/gisdefs.i points to this line: __attribute__ ((format(printf, 2, 3)));

for me line 116 of interfaces/gisdefs.i is like:

 /* auto_mask.c */
 int G__check_for_auto_masking(void);

'make clean' in the swig/python dir and try again?

?, Hamish

[glynn]

Replying to cgsbob:

While doing a make in swig/python, I get this error:

interfaces/gisdefs.i:116: Error: Syntax error in input(1).

The syntax error in interfaces/gisdefs.i points to this line: attribute ((format(printf, 2, 3)));

Right. The Makefile generates gisdefs.i from gisdefs.h, removing the declarations of any variadic functions using grep. One of the declarations which it removes is G_asprintf(), which has the above attribute declaration on the following line, so it gets left behind, separated from the actual function declaration.

I'm guessing that this happened when the file was "indent"ed.

A more robust filter is:

cat ../../../include/gisdefs.h | sed -e '/\.\.\..*;/d' -e '/\.\.\./,/;/d' >> gisdefs.i

This removes any declaration containing "..."; if the terminating semicolon isn't on the same line, it will remove subsequent lines until it finds the semicolon.

[cgsbob]

I changed the Makefile with the filter you specified below. Now there are problems with vect.i.

Replying to glynn:

Replying to cgsbob:

While doing a make in swig/python, I get this error:

interfaces/gisdefs.i:116: Error: Syntax error in input(1).

The syntax error in interfaces/gisdefs.i points to this line: attribute ((format(printf, 2, 3)));

Right. The Makefile generates gisdefs.i from gisdefs.h, removing the declarations of any variadic functions using grep. One of the declarations which it removes is G_asprintf(), which has the above attribute declaration on the following line, so it gets left behind, separated from the actual function declaration. I'm guessing that this happened when the file was "indent"ed. A more robust filter is: {{{ cat ../../../include/gisdefs.h | sed -e '/\.\.\..*;/d' -e '/\.\.\./,/;/d' >> gisdefs.i }}} This removes any declaration containing "..."; if the terminating semicolon isn't on the same line, it will remove subsequent lines until it finds the semicolon.

[hamish]

thanks Glynn. a few fine tuning issues:

The new sed filter does not remove lzw and G_verbose*() functions. Also #ifdef and #define statements are no longer removed.

I am not sure, but perhaps skipping all G_verbose*() was a bug and the intention was only to skip G_verbose_message(,...) ?? G_vasprintf(...,va_list) now becomes available FWIW.

No idea about why lzw was left out; I suspect that #defines should be filtered out (-e '/#/d').

Hamish

[hamish]

Replying to cgsbob:

Now there are problems with vect.i.

perhaps something like:

cat ../../../include/Vect.h | sed -e '/V[12]_.*/,/;/d'

is better, but "/* Read/write lines */" goes away which is a bad sign. And question re. to remove #ifndef's apply here too.

?

Hamish

[glynn]

Replying to hamish:

I am not sure, but perhaps skipping all G_verbose*() was a bug and the intention was only to skip G_verbose_message(,...) ?? G_vasprintf(...,va_list) now becomes available FWIW.

Based solely upon the regex which was used, I strongly suspect that it was meant to filter out variadic functions.

If that's the case, the other G_verbose* functions (from verbose.c) should remain, as should G_vasprintf.

No idea about why lzw was left out

Probably because lib/gis/lzw.c doesn't exist; I don't know exactly when it was removed, but it's absent in 5.3. The solution there is to just remove those declarations from gisdefs.h.

I suspect that #defines should be filtered out (-e '/#/d').

Agreed.

[hamish]

Hamish:

I suspect that #defines should be filtered out (-e '/#/d').

Glynn:

Agreed.

It happens that filtering out /^{#/ also filters out any '#include <foo.h>' etc. include/vect/dig_structs.h is the most diverse example of that, with #defined constants, structs, and #ifdef HAVE_OGR etc.}

also include/Vect.h #includes the dig_*.h files in include/vect/, should those be followed? stdio.h? grass/gis.h?

Hamish

[hamish]

fyi, "SWIG-1.3.12 and newer releases support variadic preprocessor macros."

 http://www.swig.org/Doc1.3/Preprocessor.html#Preprocessor_nn7

Hamish

[glynn]

Replying to hamish:

It happens that filtering out /^{#/ also filters out any '#include <foo.h>' etc. include/vect/dig_structs.h is the most diverse example of that, with #defined constants, structs, and #ifdef HAVE_OGR etc. also include/Vect.h #includes the dig_*.h files in include/vect/, should those be followed? stdio.h? grass/gis.h?}

The existing Makefile doesn't filter out preprocessor directives from Vect.h, only from gisdefs.h.

Replying to hamish:

fyi, "SWIG-1.3.12 and newer releases support variadic preprocessor macros."

We don't use variadic preprocessor macros, as they aren't allowed by ANSI C. The issue is whether the interface file should include variadic function declarations.

It appears that the problem is with the attribute declarations. If I insert

#define __attribute__(x)

at the top of gisdefs.i, gisdefs.h can be inserted in its entirety and nothing needs to be filtered out.

But then there's the more fundamental problem that the other interfaces/*.i files are duplicating information from the headers, and are somewhat out of date. And it's safe to say that anything which attempts to duplicate information from the headers will be continually out of date. The SWIG stuff is going to have to use the actual headers directly.

[hamish]

we can echo #define __attribute__(x) >> *.i at the top of the swig interface file if access to those functions are desired.

Perhaps leaving out G_fatal_error() is good due to the exit(1). A scripter can access the other G_message()s with a call to the g.message module if they like, maybe that method could be wrapped.

My guess is that access to the core GIS engine processing functions is the thing real value to SWIGers. Python probably has its own (possibly nicer) versions of any G_string_manipulate() function that libgis provides.

We only have to provide interfaces to functions which would be useful. But I don't want to limit others needlessly based on my lack of imagination wrt what is "useful".

AFAIU #includes are ignored/not followed, so I'll filter those out to avoid confusion, while leaving other preprocessor #ifdefs and #defines in place.

GC:

But then there's the more fundamental problem that the other interfaces/*.i files are duplicating information from the headers, and are somewhat out of date. And it's safe to say that anything which attempts to duplicate information from the headers will be continually out of date. The SWIG stuff is going to have to use the actual headers directly.

Right, AFAIK that's a known issue and not done currently simply because it is still waiting on the TODO list and none of the devels are actively working this part of the code.

At minimum I'll make a note of it in the Makefile.

Hamish

[hamish]

I checked in some fixes to devbr6 SVN but it still doesn't build.

python_grass6_wrap.c: In function '_wrap_Cluster_nbands_set':
python_grass6_wrap.c:48044: error: dereferencing pointer to incomplete type
python_grass6_wrap.c: In function '_wrap_Cluster_nbands_get':
python_grass6_wrap.c:48067: error: dereferencing pointer to incomplete type
python_grass6_wrap.c: In function '_wrap_Cluster_npoints_set':
python_grass6_wrap.c:48097: error: dereferencing pointer to incomplete type
python_grass6_wrap.c: In function '_wrap_Cluster_npoints_get':
...
python_grass6_wrap.c: In function '_wrap_new_Cluster':
python_grass6_wrap.c:49087: error: invalid application of 'sizeof' to incomplete type 'struct Cluster' 
...
python_grass6_wrap.c: At top level:
python_grass6_wrap.c:82965: warning: function declaration isn't a prototype
make: *** [python_grass6_wrap.o] Error 1

---

Auto-generation of gis.i seems to be an important thing, so wrt to tackling that next:

• gis.i seems to be based on gis.h circa 12/17/04 (r16393)

After removing comments and trailing whitespace, then diff'ing versus gis.h r16393 the changes between gis.h and gis.i are as follows:

-extern CELL CELL_NODATA;
+/*extern CELL CELL_NODATA; Sajith */


@@ -159,10 +101,17 @@
        unsigned char red;
        unsigned char grn;
        unsigned char blu;
-    } low, high;
+    }  high;
+    struct
+    {
+       DCELL value;
+       unsigned char red;
+       unsigned char grn;
+       unsigned char blu;
+    }  low;
 
-    struct _Color_Rule_ *next;
-    struct _Color_Rule_ *prev;
+/*    struct _Color_Rule_ *next;
+    struct _Color_Rule_ *prev;  Commented By sajith I am confued here....*/
 };
 
 struct _Color_Info_

The duplication of the high,low structs was done by Sajith AFAICT.

I've no idea why of course, but that's what it is.

Hamish

[glynn]

Replying to hamish:

Perhaps leaving out G_fatal_error() is good due to the exit(1).

SWIG needn't just be for wxGUI. It should be possible to write modules in Python (full raster processing in Python is likely to be slow, but code which only deals with support files isn't a problem), and those modules may need to call G_fatal_error().

My guess is that access to the core GIS engine processing functions is the thing real value to SWIGers. Python probably has its own (possibly nicer) versions of any G_string_manipulate() function that libgis provides.

A Python function can only be "equivalent" to a libgis function if the libgis function has clearly defined behaviour which won't change in future versions. If there's any chance that the behaviour might change, Python code should normally call the libgis function to ensure consistent behaviour.

[Here, I'm talking about Python code which already needs to call GRASS functions for other reasons. An otherwise "pure" Python script shouldn't import GRASS' SWIG bindings for something so minor as G_message().]

AFAIU #includes are ignored/not followed, so I'll filter those out to avoid confusion, while leaving other preprocessor #ifdefs and #defines in place.

If they're ignored, there's no specific need to filter them out.

But then there's the more fundamental problem that the other interfaces/*.i files are duplicating information from the headers, and are somewhat out of date. And it's safe to say that anything which attempts to duplicate information from the headers will be continually out of date. The SWIG stuff is going to have to use the actual headers directly.

Right, AFAIK that's a known issue and not done currently simply because it is still waiting on the TODO list and none of the devels are actively working this part of the code.

What's to do? Just remove the .i files and %include the .h files directly.

[glynn]

Replying to hamish:

I checked in some fixes to devbr6 SVN but it still doesn't build.

> python_grass6_wrap.c: In function '_wrap_Cluster_nbands_set':
> python_grass6_wrap.c:48044: error: dereferencing pointer to incomplete type

The cluster stuff was moved into a separate library (lib/cluster and cluster.h). The imagery.i file still contains a definition for "struct Cluster", but the C++ code is assuming that imagery.h will provide a definition. The quick fix is to #include cluster.h.

Auto-generation of gis.i seems to be an important thing, so wrt to tackling that next: - gis.i seems to be based on gis.h circa 12/17/04 (r16393) After removing comments and trailing whitespace, then diff'ing versus gis.h r16393 the changes between gis.h and gis.i are as follows:

> -extern CELL CELL_NODATA;
> +/*extern CELL CELL_NODATA; Sajith */

This appears to be unused.

> -    struct _Color_Rule_ *next;
> -    struct _Color_Rule_ *prev;
> +/*    struct _Color_Rule_ *next;
> +    struct _Color_Rule_ *prev;  Commented By sajith I am confued here....*/

Can SWIG not handle recursive type declarations?

The duplication of the high,low structs was done by Sajith AFAICT.

Can SWIG not handle multiple fields with the same anonymous structure type? If not, just add a named declaration for the structure before the "struct _Color_Rule_" declaration.

I've no idea why of course, but that's what it is.

First, I suggest replacing the contents of gis.i with "%include <grass/gis.h>" and seeing if that actually causes any errors. Ditto for all of the other interfaces/*.i files.

Essentially, SWIG aims to support the use of C/C++ header files directly. Although there is still some "not yet supported" syntax, the amount of it has decreased significantly since the early versions.

[hamish]

cluster lib stuff removed from imagery.i in SVN, SWIG/Python now builds again.

TODO/redo: further automated grass/*.h -> *.i

Glynn:

First, I suggest replacing the contents of gis.i with "%include <grass/gis.h>" and seeing if that actually causes any errors. Ditto for all of the other interfaces/*.i files.

It doesn't know where to find $(MODULE_TOPDIR)/include/

interfaces/gis.i:1: Error: Unable to find 'grass/gis.h'

If I get around that by hardcoding the path it shows that _Color_Rule_ is still a problem:

struct _Color_Rule_
{
    struct
    {
        DCELL value;
        unsigned char red;
        unsigned char grn;
        unsigned char blu;
    } low, high;

    struct _Color_Rule_ *next;
    struct _Color_Rule_ *prev;
};

$ make
...
/usr/bin/swig -python -shadow python_grass6.i
renames.i:2: Warning(301): class keyword used, but not in C++ mode.
/usr/src/grass/svn/grass64/include/gis.h:289: Error: Syntax error in input(3).
/usr/src/grass/svn/grass64/include/gis.h:39: Warning(451): Setting a const char * variable may leak memory.
/usr/src/grass/svn/grass64/include/gis.h:574: Warning(451): Setting a const char * variable may leak memory.
/usr/src/grass/svn/grass64/include/gis.h:578: Warning(451): Setting a const char * variable may leak memory.
...

Recursive use of a struct is not the problem, it's the "low, high" multiple defn which is creating the syntax error. If I clone that into two separate struct defns it builds ok.

Hamish

[glynn]

Replying to hamish:

First, I suggest replacing the contents of gis.i with "%include <grass/gis.h>" and seeing if that actually causes any errors. Ditto for all of the other interfaces/*.i files.

It doesn't know where to find $(MODULE_TOPDIR)/include/

Does this work:

     -I<dir>         - Look for SWIG files in directory <dir>

Also:

     -importall      - Follow all #include statements as imports
     -includeall     - Follow all #include statements

If I get around that by hardcoding the path it shows that _Color_Rule_ is still a problem:

Recursive use of a struct is not the problem, it's the "low, high" multiple defn which is creating the syntax error. If I clone that into two separate struct defns it builds ok.

How about:

struct _Color_Value_
{
    DCELL value;
    unsigned char red;
    unsigned char grn;
    unsigned char blu;
};

struct _Color_Rule_
{
    struct _Color_Value_ low, high;

    struct _Color_Rule_ *next;
    struct _Color_Rule_ *prev;
};

[hamish]

Adding a _Color_Value_ struct helps it get past the preprocessor stage, but then the compliation fails:

python_grass6_wrap.c:2945: warning: function declaration isn't a prototype
python_grass6_wrap.c: In function '_wrap__Color_Value__value_set':
python_grass6_wrap.c:5033: error: dereferencing pointer to incomplete type
python_grass6_wrap.c: In function '_wrap__Color_Value__value_get':
python_grass6_wrap.c:5056: error: dereferencing pointer to incomplete type
python_grass6_wrap.c: In function '_wrap__Color_Value__red_set':
python_grass6_wrap.c:5086: error: dereferencing pointer to incomplete type
python_grass6_wrap.c: In function '_wrap__Color_Value__red_get':
python_grass6_wrap.c:5109: error: dereferencing pointer to incomplete type
python_grass6_wrap.c: In function '_wrap__Color_Value__grn_set':
python_grass6_wrap.c:5139: error: dereferencing pointer to incomplete type
python_grass6_wrap.c: In function '_wrap__Color_Value__grn_get':
python_grass6_wrap.c:5162: error: dereferencing pointer to incomplete type
python_grass6_wrap.c: In function '_wrap__Color_Value__blu_set':
python_grass6_wrap.c:5192: error: dereferencing pointer to incomplete type
python_grass6_wrap.c: In function '_wrap__Color_Value__blu_get':
python_grass6_wrap.c:5215: error: dereferencing pointer to incomplete type
python_grass6_wrap.c: In function '_wrap_new__Color_Value_':
python_grass6_wrap.c:5228: error: invalid application of 'sizeof' to incomplete type 'struct _Color_Value_' 
python_grass6_wrap.c: In function '_wrap__Color_Rule__low_set':
python_grass6_wrap.c:5287: error: dereferencing pointer to incomplete type
python_grass6_wrap.c: In function '_wrap__Color_Rule__high_set':
python_grass6_wrap.c:5340: error: dereferencing pointer to incomplete type
python_grass6_wrap.c: In function '_wrap__Color_Info__fp_lookup_vals_set':

Hamish

[hamish]

Glynn wrote:

At this point, it may help if you post (or attach to the ticket) your modified version.

That was with the _Color_Value_ struct as given in comment:15 to the ticket.

I have now made an attempt at automatically expanding the struct in gis.h to avoid the problem. Now py/swig builds without error and we are finally using an up to date copy of gis.h.

SVN r33454 thru r33456. Improvements to the expansion method are most welcome.

Hamish

[hamish]

can extern CELL CELL_NODATA; be removed from gis.h? AFAICT CELL_NODATA is not used or defined anywhere in the code.

After removing CELL_NODATA python_grass6.pyc compiles ok upon running "python test.py" but then test.py gets stuck due to the new definition of G_gisinit():

Traceback (most recent call last):
  File "test.py", line 17, in ?
    g6lib.G_gisinit('')
AttributeError: 'module' object has no attribute 'G_gisinit'

Within a python shell "help(g6lib)" only shows Ggisinit() as a valid function name. Running g6lib.Ggisinit(,) in python exits with the mismatched version error.

How to recreate these from gis.h in a way that SWIG will like?

#define GIS_H_VERSION "$Revision: 33187 $"
#define G_gisinit(pgm) G__gisinit(GIS_H_VERSION, (pgm))
#define G_no_gisinit() G__no_gisinit(GIS_H_VERSION)

if in test.py I replace G_gisinit() with Ggisinit():

-g6lib.G_gisinit('')
+g6lib.G__gisinit(g6lib.GIS_H_VERSION, '')

Then it all works, but that's more to type.

Hamish

[glynn]

Replying to glynn:

First, I suggest replacing the contents of gis.i with "%include <grass/gis.h>" and seeing if that actually causes any errors. Ditto for all of the other interfaces/*.i files.

Okay; I've removed the swig/python/interfaces directory. python_grass7.i now includes the headers directly (I've also un-nested the structure definitions from the headers).

[hamish]

Replying to glynn:

First, I suggest replacing the contents of gis.i with "%include <grass/gis.h>" and seeing if that actually causes any errors. Ditto for all of the other interfaces/*.i files.

Replying to glynn: Okay; I've removed the swig/python/interfaces directory. python_grass7.i now includes the headers directly (I've also un-nested the structure definitions from the headers).

thanks for that, but the problem AFAICT was not nested headers, it's with

struct { "..." } a, b;

needing to be like:

struct { "..." } a;
struct { "..." } b;

? Hamish

[glynn]

Replying to hamish:

test.py gets stuck due to the new definition of G_gisinit():

if in test.py I replace G_gisinit() with Ggisinit(): {{{ -g6lib.G_gisinit() +g6lib.Ggisinit(g6lib.GIS_H_VERSION, ) }}} Then it all works, but that's more to type.

Just re-implement the macro in Python:

%pythoncode %{
def G_gisinit(pgm):
    G__gisinit(GIS_H_VERSION, pgm)
%}

[glynn]

Replying to hamish:

Okay; I've removed the swig/python/interfaces directory. python_grass7.i now includes the headers directly (I've also un-nested the structure definitions from the headers).

thanks for that, but the problem AFAICT was not nested headers, it's with

struct { "..." } a, b;

needing to be like:

struct { "..." } a;
struct { "..." } b;

The problem occurs when a nested definition is used for multiple fields. It is solved by extracting the definition then using the tag when defining the field, i.e.:

struct foo { ... };

struct bar {
   ...
   struct foo a, b;
   ...
};

There's no need to repeat the definition for each field.

[hamish]

Glynn:

The problem occurs when a nested definition is used for multiple fields. It is solved by extracting the definition then using the tag when defining the field,

Ok. I don't know why it didn't work the first time I tried untangling those structs but with the latest edits to SVN it all works now. Great. Backported to devbr6.

One last thing I'd like to figure out before closing this bug:

"python test.py" should be run by the Makefile. Besides testing that it all works this compiles the .pyc version while we know we have write access to the dir. If we wait until after install and let it happen at runtime, it is likely that the user won't have write permissions to the dir and it will have to be recompiled in memory every time the user uses the lib.

For python_grass[67].py the test must be run in a fake grass session (same as the man pages) but I'm not sure how to do that in the Makefile.

For NumPtr?/build/lib.linux-i686-2.4/NumPtr.py I am not sure how to generalize the $ARCH. Otherwise it is easy to add "python build/lib.linux-i686-2.4/test.py" to the Makefile there to generate the .pyc.

thanks, Hamish

[glynn]

Replying to hamish:

One last thing I'd like to figure out before closing this bug: "python test.py" should be run by the Makefile. Besides testing that it all works this compiles the .pyc version while we know we have write access to the dir. If we wait until after install and let it happen at runtime, it is likely that the user won't have write permissions to the dir and it will have to be recompiled in memory every time the user uses the lib. For python_grass[67].py the test must be run in a fake grass session (same as the man pages) but I'm not sure how to do that in the Makefile.

If you just want to compile the python_grass[67].py file, why not run a minimal script which simply imports that file, i.e.:

#!/usr/bin/env python
import python_grass7

This won't require a dummy session.

For NumPtr/build/lib.linux-i686-2.4/NumPtr.py I am not sure how to generalize the $ARCH.

You can specify the directories with:

python setup.py build --build-lib build/lib.foo --build-temp build/temp.foo

Or you can calculate the directories which it will use as:

plat_specifier = ".%s-%s" % (distutils.util.get_platform(), sys.version[0:3])
lib_dir = os.path.join('build', 'lib' + plat_specifier)
temp_dir = os.path.join('build', 'temp' + plat_specifier)

[hamish]

SWIG/python seems to be in much better shape now. Thanks for the tips.

For kicks I tried python -O to create basic optimized .pyo file instead of the .pyc. The resulting .pyo was binary equal to the .pyc file. Shrug.

NumPtr?

You can specify the directories with: python setup.py build --build-lib build/lib.foo ...

done.

Hamish