jaeandersson / swig Goto Github PK

Raw pointers to the internal SwigPtr struct are now stored in the MATLAB proxy classes. To make this a bit safer I propose to add one more field containing its own address. This field will be set to zero before freeing memory. This can be used as a consistency check giving less sever error messages when memory is corrupted (error instead of segfault).

for example this rename produces invalid matlab code:
%rename(assign) spin_system::operator=;

where matlab says:
Error using gammam.sys_dynamic
Error: File: sys_dynamic.m Line: 17 Column: 26
The input character is not valid in MATLAB statements or expressions.

and line 17 is:
function varargout = assign(self,varargin)

this stackoverflow answer claims an expression ([A-Za-z][A-Za-z0-9_]*) for valid function names:
http://stackoverflow.com/questions/687605/in-matlab-what-ascii-characters-are-allowed-to-be-in-a-function-name

Currently, swig-matlab uses the name of the generated cxx file as the mex function name. I assume that's because it's conventional in Matlab for the mexfile to match the name of its cxx source file (since that's what the mex command does by default). That's reasonable, but I'd suggest considering using the actual module name instead.

The build process provided by Cmake, contained in UseSWIG.cmake, results in a generated cxx file with a language-specific filename, something like:

mymodulePYTHON_wrap.cxx

but the final generated library is named to nicely match the SWIG module name:

_mymodule.so
mymodule.py

UseSWIG.cmake produces similar results for Matlab, with an intermediate file name mymoduleMATLAB_wrap.cxx and a compiled mex file named mymodule.mexmaci64 or similar. But because swig-matlab internally assumes that the mex file will be named mymoduleMATLAB_wrap to match the .cxx file, the resulting bindings can't be used without some renaming.

Of course, there are lots of other ways to build swig bindings without that particular cmake library, but since it's installed with cmake its likely to be used by others in the future. Fixing this is just a matter of setting mex_fcn = pkg_name; in matlab.cxx.

Was it intentional to make the mex function match the .cxx file instead of the module name? Or is this something you'd be interested in changing?

@jgillis added a setup script in c67924d. However, if you don't have a matlabsetup section in your interface file, this results in a run-time error at first use as the script is called but doesn't exist. This means we need to either provide a default empty one, or check if one exists, e.g. by using

mexEvalString("if (exist('stirsetup','function')) stirsetup;end");

(replacing the module name of course).

I'm also wondering what to do when the setup script fails. At present, isLoaded is set to true before the script runs, so it won't be run again. This has the advantage that if the setup failed, it won't be tried again. However, that seems a dangerous thing...

Sometimes you want to have variable number of inputs and/or outputs on the MATLAB side but a fixed number of inputs on the C++ side. Example: horzcat(x1, x2, x3, x3).

Can be fixed with a %feature directive.

Octave has support for most of the features used by the MATLAB SWIG module: mex, classdef etc. It should (in principle at least) be possible to make the generated SWIG-MATLAB modules compilable for both Octave and MATLAB. This would be an attractive alternative to using SWIG's Octave module in many cases.

Issues encountered:

• .cxx ending not recognised by mkoctfile
• stdio.h missing (needed by vsnprintf)
• CallMATLABWithTrap missing
• mxSetProperty, mxGetProperty missing
• ne not defined for handle objects
• import module.* is not supported by Octave

The class SwigRef.m contains little functionality other than holding some memory. It would be better to remove the class completely and move the functionality to the derived classes.

I wasn't aware of the "No Input Argument Constructor Requirement" in MATLAB, but it could be pretty useful in the context of SWIG, cf.

http://es.mathworks.com/help/matlab/matlab_oop/class-constructor-methods.html#btn2kiy

This would motivate getting rid of the current constructor which takes a pointer and simply rely on the default constructor.

Now, the MATLAB wrapper classes hold mangled raw pointers to the corresponding C/C++ object. This is potentially unsafe and results in unnecessary copying of data back and forth MATLAB. I think it makes sense to change this by implementing a memory allocator in the mex-function. This memory allocator could support both owning and weak-references ensuring a safe failure also when trying to use an already deleted object.

Cf. discussion in #24

The functions subsref and subsasgn can moved from the SwigRef base class to the classes inheriting directly from SwigRef. That way, they can be created only when needed, meaning better encapsulation.

It will also render SwigRef more minimalistic and hence more maintainable:

classdef SwigRef < handle
  properties 
    swigPtr
  end
  methods
    function disp(self)
      disp(sprintf('<Swig object, ptr=%d>',self.swigPtr))
    end
  end
end

A simpler SwigRef will avoid conflicts when a user has several projects, each project potentially build with a different version of Swig.

The proxy classes and functions belonging to a module mymodule are currently placed in a directory +mypackage. Often mymodule and mypackage are the same, but this doesn't need to be the case. You should be able to place wrappers for multiple modules in the same package. This would make sense if you have a large project and you want to decrease the size of the wrapper file, but without changing user syntax.

The ability to have a package name different from the module name does not appear to be supported at the moment.

After updating to the latest commit, I just noticed that accessing internal members by refence is now broken (I guess this was caused by the memory refactor in 04ab706 ) .

I plan to better understand the issue and try to solve it in July, but in this issue I can already describe what stopped working.

Basically, let's say that I have two classes A and B:

class B
{
private:  
     double value;
public:
     B()
     {
          value = 0.0;
     }
     void setValue(const double &_value)
     {
           value = _value;
     }
     double getValue() const
     {
          return value;
     }
}

class A 
{
private:
    B privateB; 
public:
    B& getB();
}

With the old behavior (and with other swig bindings, such as Lua and Python) I was able to do:

a = swig.A();
% the following command returns 0.0
a.getB().getValue()
% then we can set the value
a.getB().setValue(1.0)
% now the following line returns 1.0
a.getB().getValue()

The current behavior is instead:

a = swig.A();
% the following line returns 0.0
a.getB().getValue()
% then we can set the value
a.getB().setValue(1.0)
% the following line still returns 0.0 !
a.getB().getValue()

It is also possible that this is not actually a bug, but this is the indented behavior because in Matlab the "copy by value" is the default behavior.

On Windows, MATLAB usually sits in C:\Program Files. This creates problems in the test-suite. I've fixed this by inserting some spaces. It also works on Linux as far as my tests goes. The commit is here

KrisThielemans@f0b4c49

http://es.mathworks.com/help/matlab/ref/cast.html

"clear all" used to work (although sometimes it caused a crash). now it gives me an error if I want to use my module after the clear all

Error using stirMATLAB_wrap
Global type table deleted unexpectedly

I do image processing and want to use this SWIG to exchange multidimensional mex arrays (matrices) to carrays or something similar. I want to either process them in-place (without copying) or create a new mex array as output (dimensions would be known in advance).

On the C/C++ side I would like to have the mex arrays wrapped as

T * data, size_t * dimenions, int number_dimensions
or
T * data, std::vector<size_t> dimensions
T = int, double, ...

Important is that no additional copying takes place (my matrices are big) and that multidimensional arrays are supported.

I looked at matlab/carray.i but couldn't find anything useful. Is this already possible with the currents means?

If not, I could volunteer in writing such bindings, because I need them either way. Also I think that this would probably be a very often used task with Malab being centered so much on matrices...

MATLAB's multiple dispatch works differently from a member function in C++. If you wrap a class such as (i):

struct foo {
  double v_;
  foo (double v) : v_(v) { } // implicit type conversion!
  foo operator+( const foo& b) const { return v_ +  b.v_);}
};

It will generate something along the lines of (ii):

classdef foo
  methods
    function varargout = plus(self, varargin)
        ...
    end
end

which really corresponds to something like (iii):

struct foo {
  double v_;
  foo (double v) : v_(v) { } // implicit type conversion!
  friend foo operator+( const foo& a, const foo& b) const { return a.v_ +  b.v_);}
};

The MATLAB class (ii) and the second C++ snippet (iii) will happily accept input such as 3 + foo(2) and foo(2) + 3, whereas (i) will accept foo(2) + 3 but not 3 + foo(2). I think that it makes sense to have (i) always behave like (iii) but for that to work an input typemap from self to type foo is needed.

Finally, if you would try to wrap (iii) using SWIG, it would create a global function plus.m, which is most definitely not what you want. That function will override basic behavior such as 3 + 2, which you don't want. So it's important that the function plus remains in the class scope.

The test suite got (more) broken in one of the more recent commits.

matlab chokes when two super classes define the same (virtual) function, but the class itself hasn't defined it. demo code in #4

@jgillis added a setup script in c67924d. I think it would be more prudent to move this script inside the +package directory.

I just started experimenting with swig-matlab as a possible way to replace our huge amount of hand-written mexFunction code, and it looks very promising. However, I'm having trouble understanding how wrapped C++ classes are handled: it looks like the class destructor is never called, and the class may never be deallocated.

My test case is something like this:

#include <iostream>

class Foo {
public:
  Foo() {
    std::cout << "allocating a new Foo" << std::endl;
  }

  ~Foo() {
    std::cout << "destructing Foo at address " << this << std::endl;
  }
};

Using swig with python, I get the correct behavior:

>>> g = swigexample.Foo()
allocating a new Foo
>>> g = 1
destructing Foo at address: 0x2708b00

But with swig-matlab, the destructor seems to never be called, even when I clear mex or exit matlab.

>> x = swigexample.Foo()
allocating a new Foo
>> x = 1;
>> clear mex;
>> exit;

Is this an expected behavior? I see the same issue before and after f5532b8

The user should be warned if trying to define symbols with conflicting names due to the requirement of one file per global variable/function. This can be done using WARN_LANG_PORTABILITY_FILENAME.

MATLAB functions work differently from member functions in C++. In particular, there is no requirement that the first argument needs to be "self/this". The closest C++ correspondence to such a non-static method in MATLAB is a static function. It therefore makes sense to add a %feature marker to static C++ functions allowing them to be mapped to non-static methods in the MATLAB proxy.

The MATLAB int64 type is quite rarely used in practise. If you call numel(zeros(3)) or shape(zeros(3)) you'll get a double and a vector of doubles respectively. Therefore, it makes sense to have a int in C/C++ map to double in MATLAB by default.

Since the IEEE floating point standard guarantees that integers up to a certain number are represented exactly, I see no problem with this.

Unfortunately I don't have any benchmarks to share, but I have had several complaints from our users of the swig+MATLAB wrapper that the computational speed isn't very good,and much slower than our simpler C wrapper. I suspect it has to do with all the lookup of the class methods using strings. Is there any reason to use strings rather than something like auto-generated enumerations? That should be much faster.

Right now errors messages in the wrappers raise warnings instead of errors in MATLAB followed by a generic error message at the end. It should be possible to avoid these warning messages by instead constructing an MException array with all the error information and return this to the mexFunction gateway.

This means function signature of the structure of the wrapper functions from:

int _wrap_foo(int resc, mxArray *resv[], int argc, mxArray *argv[]) {
 ...
}

to

mxArray* _wrap_foo(int resc, mxArray *resv[], int argc, mxArray *argv[]) {
 ...
}

On success, it would return 0 (cf. mexCallMATLABWithTrap).

I realized that swig_this (cf. #6) can be made to work quite easily. The corresponding member function just needs to be added to the classes inheriting directly from SwigRef instead of being added to SwigRef itself.

I got some feedback from confused users of my SWIG-wrapped library that saved (with "save") and loaded back (with "load") their workspace containing SWIG Classes.

Clearly the "save" and "load" are not supported out-of-the-box by the SWIG Classes. The main source of confusion is that the class are saved and loaded without any warning, and then they fail with an error message when the user interacts with the loaded classes.

To have a clearer error message, I tried to implement a saveobj method in SwigRef that just print a warning as soon as the user try to save a SWIG class, but I don't know if it could make sense to have this in the SwigRef.m generated by SWIG.

It looks like SWIG-MATLAB tries to wrap a default constructor that doesn't exist:

checking matlab testcase array_typedef_memberin
Building with 'Xcode Clang++'.
/Users/jaeandersson/dev/swig/build/Examples/test-suite/matlab/array_typedef_memberin_wrap.cxx:1612:47: error: call to implicitly-deleted default constructor of 'ArrayExample::ExampleDetail'
  result = (ArrayExample::ExampleDetail *)new ArrayExample::ExampleDetail();
                                              ^
/Users/jaeandersson/dev/swig/build/Examples/test-suite/matlab/array_typedef_memberin_wrap.cxx:1175:21: note: default constructor of 'ExampleDetail' is implicitly deleted because field 'node_list2' of const-qualified type 'const Eight' (aka 'short const[8]') would not be initialized
        const Eight node_list2;
                    ^
1 error generated.

make[2]: *** [matlab_cpp] Error 255
make[1]: *** [array_typedef_memberin.cpptest] Error 2

It is important not to clear the mex memory before the corresponding wrapper classes have been freed. This can be achieved by calling mexLock in class constructors and mexUnlock in class destructors.

The following test works for MATLAB, but not Octave:

checking matlab testcase director_default
ERROR: director_defaultMEX: No matching function for overload function 'new_Foo'
.  Possible C/C++ prototypes are:
    Foo::Foo(mxArray *,int)
    Foo::Foo(mxArray *)\nERROR: director_defaultMEX: No matching function for ov
erload function 'new_Foo'.  Possible C/C++ prototypes are:
    Foo::Foo(mxArray *,int)
    Foo::Foo(mxArray *)\n
   director_default failed
checking matlab testcase director_detect

test case li_std_vector fails when trying to assign elements to a std::vector:

iv(3)=3
Error using subsref
No appropriate method, property, or field setparen for class
li_std_vector.IntVector.
Error in SwigRef/subsasgn (line 29)
builtin('subsref',self,substruct('.','setparen','()',{v, s.subs{:}}));

It's easily fixable by replacing setparen in SwigRef.m with paren_asgn. Presumably the same for setbrace?

At present, SWIG_Error calls mexWarnMsgIdAndTxt. SWIG_exception calls SWIG_Error and then SWIG_fail. The result of this is that when the wrapped code throws an exception, you get a MATLAB warning (with the exception text), and then an error (with the text 'Fatal error'). This is not good in my opinion. First instance, when catching the error in matlab, you still get the warning first.

I think SWIG_Error should call mexErrMsgIdAndTxt instead. This seems the desired behaviour for SWIG_exception etc, but maybe this is not appropriate for SWIG_Error as it is also used in the traits code (where there is an argument that prevents it from throwing). As I'm not sure about the traits stuff, I cannot suggest the best solution...

The implementation of subsref and subsasgn (which controls behaviour of data member access and assignment and function calls), is hacky and needs refactoring. E.g. a lot of normal use cases are not handled at all, e.g. nested calls.

Error message:

CoolPropMATLAB_wrap.cxx(14944): fatal error C1061: compiler limit : blocks nested too deeply [C:\Users\ian\slave\MATLAB-windows32\build\build\CoolPropMATLAB_wrap.vcxproj]

Cause:
In the conditional tree in swigConstant(), MSVC has a limit on the number of levels you can nest namespaces, which, here, means also the number of elseif branches. As far as I can tell, the limit is 128 elseif branches. A map (string->int) would fix this problem, but I gather we don't want to go that route.

Another solution is X macros and a switch, but I think you might have the same problem with being limited to 128 keys in the switch. Or perhaps not...

I try to pass const & to std::vector<double> to my function, and when I do so, I get the error:

Warning: asptr for seq 
Warning: in method 'AbstractState_set_mole_fractions', argument 2 of type
'std::vector< double,std::allocator< double > > const &' 

I don't see any examples that pass STL containers, and I wonder if this is just something that isn't implemented yet? Actually I am passing a DoubleVector instance which is just a wrapper of a std::vector

Just like in Python.

I'm testing your work to get a matlab support in swig. That's a really useful thing! Thanks!

It seems that the "outdir" argument is not taken into account.

A fix has been done on another language for the same problem:
swig@138af71

Title pretty much says it all. Probably need to do some documentation before merging to swig/swig.

It would be very useful to add functions as MATLAB code directly in the proxy classes, cf. %pythoncode in the Python module.

It makes sense to redirect std::cout to a stream that prints to mexPrintf. Such a stream can be implemented as follows:

#include <streambuf>
#include <ostream>
  namespace swig {
    /** Stream buffer to allow printing to mex conveniently
        Adapted from answer to stackoverflow question 243696.
    */
    class mex_buf : public std::streambuf {
    public:
      mex_buf() {}
    protected:
      virtual int_type overflow(int_type ch) {
        if(ch != traits_type::eof()) {
          mexPrintf("%c", static_cast<char>(ch));
        }
        return ch;
      }
      /* virtual int sync() { // needed?
         mexEvalString("drawnow;");
         return 0;
      } */
      virtual std::streamsize xsputn(const char* s, std::streamsize num) {
        mexPrintf("%.*s", static_cast<int>(num), s);
        return num;
      }
    };

    // Corresponding output stream
    class mexostream : public std::ostream {
    protected:
      mex_buf buf;
    public:
      mexostream() : std::ostream(&buf) {}
    };
  } // namespace swig

The redirection can be done in the mexFunction function:

void mexFunction(int resc, mxArray *resv[], int argc, const mxArray *argv[]){
   ...
   swig::mexostream mex_cout;
   std::streambuf *cout_backup = std::cout.rdbuf(&mex_cout);
   ...
    std::cout.rdbuf(cout_backup); // restore
   ..
}

In a similar way, std::cerr can be made to print MATLAB warning messages (don't think you want to print errors since that will interrupt the execution).

MATLAB mex defines mexAtExit for cleaning up persistent memory, cf. http://es.mathworks.com/help/matlab/apiref/mexatexit.html and #22.

we have a C++ member function wrapped that takes no arguments. One of our users used to be able to call this in matlab without brackets, but this now throws an error:
One or more output arguments not assigned during call to "voidvoidtest_wrap".
It does work with brackets though. It seems that in the recent update to subsref this behaviour changed.
Note that it still works without brackets for stand-alone functions or static member functions.

To reproduce this:

%module testit
%{
#include <stdio.h>
%}

%inline %{
void testfunction() { printf("OK\n"); };

class testclass
{
public:
static void statictestfunction() { printf("OK\n"); };
testclass() {}
// this is the one with the problem
void membertestfunction() { printf("OK\n"); };
};
%}

With the above interface file, I get in matlab

a=testit.testclass;
a.membertestfunction()
OK
a.membertestfunction
OK
One or more output arguments not assigned during call to "voidvoidtest_wrap".

Error in testit.testclass/testfunction (line 13)
[varargout{1:nargout}] = voidvoidtest_wrap(6, self, varargin{:});

Error in SwigRef/subsref (line 13)
[varargout{1}] = builtin('subsref',self,substruct('.',s.subs,'()',{}));

Consider the Matlab output duality of size:

size(a) % n m

[m,n] = size(a)

I propose a %feature 'optionalunpack' that brings this behavour to a wrapped function.

Default interface file example_wrap.cxx is cpp format. This means the compiler is expecting manged function name (i.e. "int __cdecl gcd(int,int)" instead of "int gcd(int,int)"). The default version triggers a LNK2019 error using MSVC2010. This can be fixed by adding extern "C" into example.i file.

BTW, is there a reason that the default output file format is cpp format? Thanks!

The typemaps for std::vector and std::map do not work properly. For my own project, I have redefined these typemaps, so this is not really an issue for me, but we should get this working with default typemaps also.

I think it makes sense to write input typemaps from MATLAB arrays (row vectors or column vectors) and cell arrays (row vectors or column vectors).

As for output typemaps, my suggest to use normal row vector (not cell array) whenever possible (i.e. for built-in datatypes such as doubles or integer types) and cell array row vector otherwise.

In CasADi, I have mapped MATLAB structs to std::map<std::string, T> also.

CasADi's typemaps for std::vector and std::map are available here: https://github.com/casadi/casadi/blob/develop/swig/casadi.i.