Apparently the %import directive is meant to be used with .i files rather than .h files. impEM isn't doing the right thing (nor was mine). I haven't yet checked if that solves my problems linking multiple swig libs as I have only had my mac which doesn't exhibit the problems.
In addition, we are not properly sharing the swig runtime info between modules. Apparently we need to add " -DSWIG_TYPE_TABLE=IMP" to each compilation of a _wrap.cc so that the information is properly shared. Unfortunately, this includes EMbed (although EMbed doesn't have to actually see any of the IMP code as the IMP there is just used as a string).
I would also propose changing IMP_exceptions.i as follows in order to allow other libraries to reuse the exception handling function by %including IMP_exceptions.i rather than redefine a new one: IMP_exceptions.i should contain: %{ void IMP_swig_handle_exception(void); %}
%exception { try { $action } catch (...) { IMP_swig_handle_exception(); /* This should be unnecessary, since handle_imp_exception cannot return; here just to quell lots of warnings about the 'result' variable not being initialized. */ SWIG_fail; } }
and putting the following in IMP.i to define the function uniquely.
%{ /* Code to convert C++ exceptions into scripting language errors. Saves having lots of catch statements in every single wrapper. */ void IMP_swig_handle_exception(void) { try { throw; } catch (std::out_of_range &e) { SWIG_exception(SWIG_IndexError, e.what()); } catch (IMP::IndexException &e) { SWIG_exception(SWIG_IndexError, e.what()); } catch (IMP::InvalidStateException &e) { SWIG_exception(SWIG_ValueError, e.what()); } catch (IMP::ErrorException &e) { SWIG_exception(SWIG_RuntimeError, e.what()); } /* SWIG_exception contains "goto fail" so make sure the label is defined */ fail: return; } %}
Here is a patch for everything in IMP (not for EMbed).
Daniel Russel wrote: > Apparently the %import directive is meant to be used with .i files > rather than .h files.
No, %import is meant to be used with SWIG interface files, just like %include is. In many cases, regular header files are valid SWIG interface files, and that's the case with IMP. By %import'ing all of IMP, you're telling SWIG about a whole bunch of classes it doesn't need to care about for impEM. So this change is unnecessary - unless it does actually fix a problem you're having (in which case, add a testcase which crashes with the current setup and works with your patch).
> In addition, we are not properly sharing the swig runtime info between > modules. Apparently we need to add " -DSWIG_TYPE_TABLE=IMP" to each > compilation of a _wrap.cc so that the information is properly shared.
On the contrary, you only need to define that if you _don't_ want to share the information except with other modules that have the same type table. So we don't need this change either - by default everything is shared (and since we use namespaces properly we shouldn't conflict with other SWIG modules). See http://www.swig.org/Doc1.3/Modules.html#Modules_nn2 Admittedly that documentation is a little misleading, so again, if you need it to fix a problem, add a testcase.
> I would also propose changing IMP_exceptions.i as follows in order to > allow other libraries to reuse the exception handling function by > %including IMP_exceptions.i rather than redefine a new one:
Sounds good to me. There's been no need to do that thus far because the impEM guys seem to be allergic to exceptions. ;)
Ben
Anyway, am looking in to these things since IMP currently does not work reliably with other swig modules which use IMP. For example, doing for p in IMP.Particles(): print p crashes on my linux box if IMP and another swig library are loaded. Try adding that line to anything in the IMPEM tests to see.
On Jan 3, 2008, at 10:47 AM, Ben Webb wrote:
> Daniel Russel wrote: >> Apparently the %import directive is meant to be used with .i files >> rather than .h files. > > No, %import is meant to be used with SWIG interface files, just like > %include is. In many cases, regular header files are valid SWIG > interface files, and that's the case with IMP. By %import'ing all of > IMP, you're telling SWIG about a whole bunch of classes it doesn't > need > to care about for impEM. So this change is unnecessary - unless it > does > actually fix a problem you're having (in which case, add a testcase > which crashes with the current setup and works with your patch). > To quote the manual: To create the wrapper properly, module derived needs to know the base class and that it's interface is covered in another module. The line %import "base.i" lets SWIG know exactly that. The common mistake here is to %import the .h file instead of the .i, which sadly won't do the trick. Another issue to take care of is that multiple dependent wrappers should not be linked/loaded in parallel from multiple threads as SWIG provides no locking - for more on that issue, read on.
>> In addition, we are not properly sharing the swig runtime info >> between >> modules. Apparently we need to add " -DSWIG_TYPE_TABLE=IMP" to each >> compilation of a _wrap.cc so that the information is properly shared. > > On the contrary, you only need to define that if you _don't_ want to > share the information except with other modules that have the same > type > table. So we don't need this change either - by default everything is > shared (and since we use namespaces properly we shouldn't conflict > with > other SWIG modules). See http://www.swig.org/Doc1.3/Modules.html#Modules_nn2 > Admittedly that documentation is a little misleading, so again, if you > need it to fix a problem, add a testcase. Well, when I don't have it, swig doesn't handle casts correctly across modules, and the manual does explicitly say otherwise: The runtime functions are private to each SWIG-generated module. That is, the runtime functions are declared with "static" linkage and are visible only to the wrapper functions defined in that module. The only problem with this approach is that when more than one SWIG module is used in the same application, those modules often need to share type information. This is especially true for C++ programs where SWIG must collect and share information about inheritance relationships that cross module boundaries.
Daniel Russel wrote: > Anyway, am looking in to these things since IMP currently does not work > reliably with other swig modules which use IMP. For example, doing > for p in IMP.Particles(): > print p > crashes on my linux box if IMP and another swig library are loaded. Try > adding that line to anything in the IMPEM tests to see.
Indeed. But your changes don't seem to fix that. I just posted a stripped-down example of this problem to the SWIG mailing list, which may help us or others figure out what's going on. It doesn't use %import, so that can't be the problem, and I tried your SWIG_TYPE_TABLE fix, which had no effect. I think you were right before that it's a symbol clash problem.
Ben
Indeed my changes don't fix it. I still think the documentation says that my way is more correct (although in practice equally broken :-) Anyway, I have to continue stuffing everything into _IMP.so for now :-(
On Jan 3, 2008, at 12:59 PM, Ben Webb wrote:
> Daniel Russel wrote: >> Anyway, am looking in to these things since IMP currently does not >> work >> reliably with other swig modules which use IMP. For example, doing >> for p in IMP.Particles(): >> print p >> crashes on my linux box if IMP and another swig library are loaded. >> Try >> adding that line to anything in the IMPEM tests to see. > > Indeed. But your changes don't seem to fix that. I just posted a > stripped-down example of this problem to the SWIG mailing list, which > may help us or others figure out what's going on. It doesn't use > %import, so that can't be the problem, and I tried your > SWIG_TYPE_TABLE > fix, which had no effect. I think you were right before that it's a > symbol clash problem. > > Ben > -- > ben@salilab.org http://salilab.org/~ben/ > "It is a capital mistake to theorize before one has data." > - Sir Arthur Conan Doyle > _______________________________________________ > IMP-dev mailing list > IMP-dev@salilab.org > https://salilab.org/mailman/listinfo/imp-dev
participants (2)
-
Ben Webb -
Daniel Russel