Yarp: [Bindings] New template definition `Vector` is shadowing previous extensions of yarp::sig::Vector

[nunoguedelha]

Describe the bug
We run in Matlab the following commands for creating and reading a yarp Vector:

>> aVec=yarp.Vector(3)

aVec = 

  3 Vector array with no properties.

>> aVec.get(0)
No appropriate method, property, or field 'get' for class
'yarp.Vector'.

Error in SwigRef/subsref (line 33)
        [varargout{1:nargout}] = builtin('subsref',self,s);

Initial comments here by @lrapetti who spotted the issue.

To Reproduce
Robotology-superbuild with compiled Yarp bindings.

Expected behavior
Matlab should return an empty value or 0 instead of an error.

Configuration (please complete the following information):

• OS: Linux or MacOS
• yarp version: https://github.com/robotology/yarp/commit/8c421d66b57d767f7385a56fe0b7611c8c39f975
  
  
  
  • yarp-matlab-bindings: https://github.com/robotology-playground/yarp-matlab-bindings/tree/bindings/regenerate
  
  

• SWIG: https://github.com/robotology-dependencies/swig/tree/matlab

Root cause
The line https://github.com/robotology/yarp/blob/15c3328681fc8d3da5bb3bef76d686b8fbc81d64/bindings/yarp.i#L567 is shadowing the further extensions (%extend) of the class yarp::sig::Vector, such that the extra get, set, fromMatlab, toMatlab method definitions are ignored.
For more details, please refer to the analysis here.

[nunoguedelha]

This issue was introduced by https://github.com/robotology/yarp/commit/2417c7d5e50aebca2ca8532fb40e7084262d50e8.
Hi @Nicogene , because of this code:
https://github.com/robotology/yarp/blob/15c3328681fc8d3da5bb3bef76d686b8fbc81d64/src/libYARP_sig/include/yarp/sig/Vector.h#L25-L39
SWIG doesn't see the typedef and so no Vector class is defined under the yarp::sig namespace and all lines regarding that class in yarp.i are ignored. Any ideas to fix this are welcome.

[nunoguedelha]

Fixed by https://github.com/robotology/yarp/pull/1811.

[nunoguedelha]

Some explanations

After trying several combinations of:

• masking or not of the typedef in yarp/sig/Vector.h (SWIG flag),
• adding, removing or moving the line %template(Vector) yarp::sig::VectorOf<double> in yarp.i,
• in yarp.i, replacing the instances of yarp::sig::Vector by the defined template Vector,
• in yarp.i, replacing the instances of yarp::sig::Vector by yarp::sig::VectorOf<double> which is a straightforward solution but I wanted to avoid it,

I came to the following conclusions:

• without the wrapping %template(Vector) yarp::sig::VectorOf<double>, the binding class yarp.Vector is not created at all and the template class VectorOf is not wrapped either in yarpMATLAB_wrap.cxx. Actually this works for the iDyntree bindings as long as we instanciate the class in the iDyntree code. Maybe for the Yarp bindings it's not working because of the flag _MSC_VER as shown below
  https://github.com/robotology/yarp/blob/15c3328681fc8d3da5bb3bef76d686b8fbc81d64/src/libYARP_sig/include/yarp/sig/Vector.h#L483-L485

• as soon as we wrap the template VectorOf<double> in yarp.i, the typedef in https://github.com/robotology/yarp/blob/15c3328681fc8d3da5bb3bef76d686b8fbc81d64/src/libYARP_sig/include/yarp/sig/Vector.h#L36 is useless for SWIG (all operations on yarp::sig:Vector like %extend are ignored). Moving the %template definition to the end of the file won't help. So rather keep the flag added by @Nicogene for obvious reasons.
• using %extend on the template wrapper Vector doesn't work.

The simplest solution is to use directly yarp::sig:VectorOf<double> instead of yarp::sig:Vector in yarp.i. But this means we have to keep this in mind every time we use the pattern wrapping a class instance like https://github.com/robotology/yarp/blob/15c3328681fc8d3da5bb3bef76d686b8fbc81d64/src/libYARP_sig/include/yarp/sig/Vector.h#L36

[drdanz]

Reopening since #1811 is not merged yet.
@nunoguedelha Please, do not close issues until the relative PR is merged...

[nunoguedelha]

Reopening since #1811 is not merged yet.
@nunoguedelha Please, do not close issues until the relative PR is merged...

@drdanz sure, sorry for that.