Yarp: wrong SWIG wrapper generation of VOCABs

Created on 1 Jun 2018 · 23Comments · Source: robotology/yarp

Description
After the fixes https://github.com/robotology/yarp/pull/1655 and https://github.com/robotology/yarp/pull/1696 have been merged into branch master and devel, we are still observing a prior issue on the bindings generation:
- SWIG not honoring int casts, leaving a simple char
.. and this despite the implicit cast due to the multiplication, as in the example below:

.../robotology-superbuild/robotology/yarp-matlab-bindings/matlab/autogenerated/yarpMATLAB_wrap.cxx:125658:125: error: use of undeclared identifier 'i'
  case 95: *resv = SWIG_Matlab_SetConstant(module_ns,"VOCAB_FRAMEGRABBER_IMAGE",SWIG_From_int(static_cast< int >(0*16777216+i*65536+g*256+f)));; break;

To Reproduce
The issue occurred while compiling the robotology-superbuild on MacOS (Sierra)

Update the robotology-superbuild to the latest master and devel branches
Enable Core and Dynamics packages (profile CMake options ROBOTOLOGY_ENABLE_CORE and ROBOTOLOGY_ENABLE_DYNAMICS)
Enable Gazebo and Matlab dependencies (dependencies options ROBOTOLOGY_USES_GAZEBO and ROBOTOLOGY_USES_MATLAB
yarp-matlab-bindings and YARP in devel branch
Enable YARP_GENERATE_MATLAB option in yarp-matlab-bindings

Expected behavior
Compilation successful.

Screenshots
Build error messages:

CompileC /Users/nunoguedelha/dev/robotology-superbuild/build/robotology/yarp-matlab-bindings/matlab/yarp-matlab-bindings.build/Release/yarpMEX.build/Objects-normal/x86_64/yarpMATLAB_wrap.o matlab/autogenerated/yarpMATLAB_wrap.cxx normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler
    cd /Users/nunoguedelha/dev/robotology-superbuild/robotology/yarp-matlab-bindings
    export LANG=en_US.US-ASCII
    /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang -x c++ -arch x86_64 -fmessage-length=0 -fdiagnostics-show-note-include-stack -fmacro-backtrace-limit=0 -Wno-trigraphs -fpascal-strings -O3 -Wno-missing-field-initializers -Wno-missing-prototypes -Wno-return-type -Wno-non-virtual-dtor -Wno-overloaded-virtual -Wno-exit-time-destructors -Wno-missing-braces -Wparentheses -Wswitch -Wno-unused-function -Wno-unused-label -Wno-unused-parameter -Wno-unused-variable -Wunused-value -Wno-empty-body -Wno-uninitialized -Wno-unknown-pragmas -Wno-shadow -Wno-four-char-constants -Wno-conversion -Wno-constant-conversion -Wno-int-conversion -Wno-bool-conversion -Wno-enum-conversion -Wno-float-conversion -Wno-non-literal-null-conversion -Wno-objc-literal-conversion -Wno-shorten-64-to-32 -Wno-newline-eof -Wno-c++11-extensions -DCMAKE_INTDIR=\"Release\" -DyarpMEX_EXPORTS -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk -fasm-blocks -fstrict-aliasing -Wdeprecated-declarations -Winvalid-offsetof -mmacosx-version-min=10.12 -Wno-sign-conversion -Wno-infinite-recursion -Wno-move -Wno-comma -Wno-block-capture-autoreleasing -Wno-strict-prototypes -Wno-range-loop-analysis -I/Users/nunoguedelha/dev/robotology-superbuild/build/robotology/yarp-matlab-bindings/matlab/Release/include -I/Applications/MATLAB_R2017b.app/extern/include -isystem /Users/nunoguedelha/dev/robotology-superbuild/build/install/include -I/Users/nunoguedelha/dev/robotology-superbuild/build/robotology/yarp-matlab-bindings/matlab/yarp-matlab-bindings.build/Release/yarpMEX.build/DerivedSources/x86_64 -I/Users/nunoguedelha/dev/robotology-superbuild/build/robotology/yarp-matlab-bindings/matlab/yarp-matlab-bindings.build/Release/yarpMEX.build/DerivedSources -Wmost -Wno-four-char-constants -Wno-unknown-pragmas -F/Users/nunoguedelha/dev/robotology-superbuild/build/robotology/yarp-matlab-bindings/matlab/Release -std=c++11 -DNDEBUG -fPIC -std=gnu++11 -MMD -MT dependencies -MF /Users/nunoguedelha/dev/robotology-superbuild/build/robotology/yarp-matlab-bindings/matlab/yarp-matlab-bindings.build/Release/yarpMEX.build/Objects-normal/x86_64/yarpMATLAB_wrap.d --serialize-diagnostics /Users/nunoguedelha/dev/robotology-superbuild/build/robotology/yarp-matlab-bindings/matlab/yarp-matlab-bindings.build/Release/yarpMEX.build/Objects-normal/x86_64/yarpMATLAB_wrap.dia -c /Users/nunoguedelha/dev/robotology-superbuild/robotology/yarp-matlab-bindings/matlab/autogenerated/yarpMATLAB_wrap.cxx -o /Users/nunoguedelha/dev/robotology-superbuild/build/robotology/yarp-matlab-bindings/matlab/yarp-matlab-bindings.build/Release/yarpMEX.build/Objects-normal/x86_64/yarpMATLAB_wrap.o
/Users/nunoguedelha/dev/robotology-superbuild/robotology/yarp-matlab-bindings/matlab/autogenerated/yarpMATLAB_wrap.cxx:125658:125: error: use of undeclared identifier 'i'
  case 95: *resv = SWIG_Matlab_SetConstant(module_ns,"VOCAB_FRAMEGRABBER_IMAGE",SWIG_From_int(static_cast< int >(0*16777216+i*65536+g*256+f)));; break;
                                                                                                                            ^
/Users/nunoguedelha/dev/robotology-superbuild/robotology/yarp-matlab-bindings/matlab/autogenerated/yarpMATLAB_wrap.cxx:125658:133: error: use of undeclared identifier 'g'
  case 95: *resv = SWIG_Matlab_SetConstant(module_ns,"VOCAB_FRAMEGRABBER_IMAGE",SWIG_From_int(static_cast< int >(0*16777216+i*65536+g*256+f)));; break;
                                                                                                                                    ^
/Users/nunoguedelha/dev/robotology-superbuild/robotology/yarp-matlab-bindings/matlab/autogenerated/yarpMATLAB_wrap.cxx:125658:139: error: use of undeclared identifier 'f'
  case 95: *resv = SWIG_Matlab_SetConstant(module_ns,"VOCAB_FRAMEGRABBER_IMAGE",SWIG_From_int(static_cast< int >(0*16777216+i*65536+g*256+f)));; break;
                                                                                                                                          ^
/Users/nunoguedelha/dev/robotology-superbuild/robotology/yarp-matlab-bindings/matlab/autogenerated/yarpMATLAB_wrap.cxx:125659:117: error: use of undeclared identifier 'r'
  case 96: *resv = SWIG_Matlab_SetConstant(module_ns,"VOCAB_FRAMEGRABBER_IMAGERAW",SWIG_From_int(static_cast< int >(r*16777216+i*65536+g*256+f)));; break;
                                                                                                                    ^
/Users/nunoguedelha/dev/robotology-superbuild/robotology/yarp-matlab-bindings/matlab/autogenerated/yarpMATLAB_wrap.cxx:125659:128: error: use of undeclared identifier 'i'
  case 96: *resv = SWIG_Matlab_SetConstant(module_ns,"VOCAB_FRAMEGRABBER_IMAGERAW",SWIG_From_int(static_cast< int >(r*16777216+i*65536+g*256+f)));; break;
                                                                                                                               ^
/Users/nunoguedelha/dev/robotology-superbuild/robotology/yarp-matlab-bindings/matlab/autogenerated/yarpMATLAB_wrap.cxx:125659:136: error: use of undeclared identifier 'g'
  case 96: *resv = SWIG_Matlab_SetConstant(module_ns,"VOCAB_FRAMEGRABBER_IMAGERAW",SWIG_From_int(static_cast< int >(r*16777216+i*65536+g*256+f)));; break;

Configuration (please complete the following information):

OS: MacOS (Sierra) 10.12.6 , (CMAKE_OSX_SYSROOT: .../MacOSX10.13.sdk)
yarp version: YARP version 2.3.73.4+20180531.17+gitfd087b27b
- compiler: Xcode 9.2, build version 9C40b

Additional context
N/A.

YARP v2.3.72 Bindings Bug Upstream

Source

nunoguedelha

Most helpful comment

CC @jgvictores @drdanz @Nicogene @traversaro @jeljaik

nunoguedelha on 1 Jun 2018

👍2

All 23 comments

CC @jgvictores @drdanz @Nicogene @traversaro @jeljaik

nunoguedelha on 1 Jun 2018

👍2

@jgvictores Any idea?

traversaro on 1 Jun 2018

@nunoguedelha Thanks for identifying this. A bit difficult as I do not have a Mac machine, but let's see what we can do. Most imporantly, which SWIG version is being used?

PS: It would also be great if you identified if this was working before, and at which commit it broke.

jgvictores on 1 Jun 2018

Most importantly, which SWIG version is being used?

It is a SWIG fork that we use to generate the Matlab bindings (I think that is the problem, nothing Mac-specific), see the docs here: https://github.com/robotology-playground/yarp-matlab-bindings#regenerate-the-matlab-bindings-not-necessary-to-use-the-bindings .
It also ok for us to us the matlab branch of the upstream swig repository : https://github.com/swig/swig/tree/matlab .

traversaro on 1 Jun 2018

@jgvictores thanks for the prompt answer. I'm using the SWIG commit 260ed47c4414e61c66ae84a639707b1fef916ba8.
It's the first time I'm building the robotology-superbuild, but I'm used to generate the bindings within the codyco-superbuild now deprecated, using the same SWIG commit.

nunoguedelha on 1 Jun 2018

I think the issue should be reproducible even by using directly the https://github.com/robotology-playground/yarp-matlab-bindings repo.

traversaro on 1 Jun 2018

Testing now the compilation on Ubuntu for comparing the generated yarpMATLAB_wrap.cxx.

nunoguedelha on 1 Jun 2018

Got the same issue on Ubuntu:

/usr/local/src/robot/robotology-superbuild/robotology/yarp-matlab-bindings/matlab/autogenerated/yarpMATLAB_wrap.cxx: In function ‘int swigConstant(int, mxArray**, int, mxArray**)’:
/usr/local/src/robot/robotology-superbuild/robotology/yarp-matlab-bindings/matlab/autogenerated/yarpMATLAB_wrap.cxx:124848:125: error: ‘i’ was not declared in this scope
   case 95: *resv = SWIG_Matlab_SetConstant(module_ns,"VOCAB_FRAMEGRABBER_IMAGE",SWIG_From_int(static_cast< int >(0*16777216+i*65536+g*256+f)));; break;
                                                                                                                             ^
/usr/local/src/robot/robotology-superbuild/robotology/yarp-matlab-bindings/matlab/autogenerated/yarpMATLAB_wrap.cxx:1171:52: note: in definition of macro ‘SWIG_Matlab_SetConstant’
 #define SWIG_Matlab_SetConstant(dummy1,dummy2,pm) (pm)
                                                    ^
/usr/local/src/robot/robotology-superbuild/robotology/yarp-matlab-bindings/matlab/autogenerated/yarpMATLAB_wrap.cxx:124848:133: error: ‘g’ was not declared in this scope
   case 95: *resv = SWIG_Matlab_SetConstant(module_ns,"VOCAB_FRAMEGRABBER_IMAGE",SWIG_From_int(static_cast< int >(0*16777216+i*65536+g*256+f)));; break;
                                                                                                                                     ^
/usr/local/src/robot/robotology-superbuild/robotology/yarp-matlab-bindings/matlab/autogenerated/yarpMATLAB_wrap.cxx:1171:52: note: in definition of macro ‘SWIG_Matlab_SetConstant’
 #define SWIG_Matlab_SetConstant(dummy1,dummy2,pm) (pm)
                                                    ^
/usr/local/src/robot/robotology-superbuild/robotology/yarp-matlab-bindings/matlab/autogenerated/yarpMATLAB_wrap.cxx:124848:139: error: ‘f’ was not declared in this scope
   case 95: *resv = SWIG_Matlab_SetConstant(module_ns,"VOCAB_FRAMEGRABBER_IMAGE",SWIG_From_int(static_cast< int >(0*16777216+i*65536+g*256+f)));; break;

YARP version: 2.3.73.4+20180530.51+git90856b8 (branch devel)
yarp-matlab-bindings commit: a93d4a8339b3e36939154e12a975c745c5d68d61 (branch devel)

nunoguedelha on 1 Jun 2018

Ok, on Ubuntu, Python bindings also break with https://github.com/swig/swig/tree/260ed47c4414e61c66ae84a639707b1fef916ba8 which identifies itself as 3.0.11.

jgvictores on 1 Jun 2018

@jgvictores Could you specify the version of SWIG, and overall configuration you formerly used for compiling and testing the bindings successfully, for checking the delta with our own configuration? Thanks

nunoguedelha on 1 Jun 2018

Sure. All tests on Ubuntu 16.04 (Xenial) machines. SWIG versions:

3.0.8 (default Xenial): works, as does not use the macro.
3.0.11: works with macro variant introduced at #1696.
3.0.12 (default Bionic): works with macro variant introduced at #1696.
4.0.0 (master at time of writing): works with macro variant introduced at #1696.

Languages: Python, Lua. Also C# but only compilation.

Note this current yarp.i line that traces the threshold on 3.0.11. This was after tests with the versions and due to issues such as https://sourceforge.net/p/swig/bugs/1168/ that state it "was fixed in 3.0.11".

jgvictores on 1 Jun 2018

@traversaro I've just tried https://github.com/swig/swig/tree/matlab (https://github.com/swig/swig/commit/c05f17fe312841574fc82ac06559a4abc4dd50c4) but, similar to https://github.com/swig/swig/tree/260ed47c4414e61c66ae84a639707b1fef916ba8, it identifies itself as 3.0.11 but still breaks even with the simple Python test on Xenial.

Since I see there is no simple way to distinguish between SWIG 3.0.11 variants, I wouldn't mind bumping the required version of SWIG for the macro variant to 3.0.12 (thus ready for Bionic). You could simply hot-fix this current yarp.i line from 0x030011 to 0x030012 (PS: also update the comment in the Lua test).
@traversaro @nunoguedelha What would your opinion be?

jgvictores on 1 Jun 2018

But then all VOCAB definitions will resolve to the value 0. Doesn't this break some interface features? Do the enums then take over. I don't know the details of the Yarp implementation on this.

nunoguedelha on 1 Jun 2018

But then all VOCAB definitions will resolve to the value 0. Doesn't this break some interface features? Do the enums then take over. I don't know the details of the Yarp implementation on this.

In theory, the 0 value is only enforced in C# (see 3, below). : )
For the rest of languages, VOCABs defined within enums (which AFAIK is becoming a common practice) should be generated thanks to #1655 (gives you things like VOCAB_PIXEL_RGB).
For VOCABs defined in the global scope (e.g. VOCAB_CM_POSITION is now required for control mode change), well.. #1696 was for that, but it requires a more modern SWIG to cope with even that simplified version the macro. For SWIG below 3.0.11 the VOCABs are not generated (rather than a 0), as an attempt to make your code break at validation rather than sending bogus 0s at run-time (xD). The fact is that,, before #1696, this issue was unattended: hacks were only to avoid compilation errors, but bogus and null VOCABs were being generated. On a side note: the errors you are seeing are not chars, but chars becoming variables because of SWIG.

Ways to really fix this and correctly generate all VOCABs are difficult:

Find an even simpler macro variant, see if low versions of SWIG do not choke on it...
Merge SWIG fixes into matlab branch or vice-versa.

jgvictores on 1 Jun 2018

Ways to really fix this and correctly generate all VOCABs are difficult:
Find an even simpler macro variant, see if low versions of SWIG do not choke on it...
Merge SWIG fixes into matlab branch or vice-versa.

@jgvictores , thanks for the explanation. I took a closer look at the issue https://github.com/swig/swig/issues/737, which is still unsolved, and understood how your fix https://github.com/robotology/yarp/pull/1696 gets around it.

Hot fix...

In deed, after changing the SWIG version check from 0x030011 to 0x030012 in yarp.i, the VOCAB constants are generated with wrong values, but the methods Vocab::encode and Vocab::decode seem to be correctly generated, we just have to warn Matlab bindings users to use these methods instead of the constant bindings like yarp.VOCAB_CM_POSITION, yarp.VOCAB_CM_PWM, etc. I still haven't tested this yet.

Issue of chars becoming variables...

Recalling the issue: SWIG generates, for instance, the code SWIG_From_int(static_cast< int >(0*16777216+i*65536+g*257+f)) instead of SWIG_From_int(static_cast< int >('0'*16777216+'i'*65536+'g'*256+'f')). While the first syntax lures the C++ compiler to think that f, g, i are variables, the second syntax should work fine for the constant global scope definition.
I haven't found the respective issue in tracked in https://github.com/swig/swig. If you have a link to share, I would be glad. The issue is still present in the latest matlab branch of @jaeandersson's fork. https://github.com/jaeandersson/swig.git.

As a side note, while doing some tests, I realised that the proper global definition syntax is generated by the SWIG preprocessor, the conversion of chars into variables happens next when generating the C++ file. I depicted below a simple example.

#define ABUGGEDTAG 'x'*2

is converted to...

%constant int ABUGGEDTAG = 'x'*2;

by the preprocessor, then to...

SWIG_Matlab_SetConstant(module_ns,"ABUGGEDTAG",SWIG_From_int(static_cast< int >(x*2)));

in the generated C++ file, where the char became a variable. We can reproduce the issue using directly %constant int ABUGGEDTAG = 'x'*2; in the .i input file.

nunoguedelha on 4 Jun 2018

If you have a link to share, I would be glad

Tracing back, here's a SWIG fix that was merged in around 3.0.11->3.0.12 transition (even has a PACK macro very similar to VOCAB), ...note that it mentions enum a bit too much (and we want globals too): issues/737 -> bugs/1168 -> bugs/668 -> pull/781

TLDR:((Even going through https://github.com/roboticslab-uc3m/kinematics-dynamics/issues/150 where we initially saw this, I usually keep bumping into SWIG issues/737 and bugs/1168. It is a bug that is extremely difficult to trace for me, see https://github.com/roboticslab-uc3m/kinematics-dynamics/issues/150#issuecomment-391408359 where on a small example I even got << working with a smaller example (probably on SWIG 3.0.12) and thought the bug might be due to some typedef in yarp.i...)).

we just have to warn Matlab bindings users to use these methods instead of the constant bindings like yarp.VOCAB_CM_POSITION, yarp.VOCAB_CM_PWM, etc.

Yes, that would be the way before the latest PRs. The PRs where intended to avoid this, as it forces API users to look through the code, and manually encode VOCABs (which are officially non stable).

the VOCAB constants are generated with wrong values

Just for the record, the intention of the PRs was for them to be not generated. Some fw+bw compat code @PeterBowman proposed (IMHO, perhaps good for production code, but better simple comments in examples):

vocab = yarp.VOCAB_CM_POSITION or yarp.Vocab_encode('pos') -- lua

vocab = hasattr(yarp, VOCAB_CM_POSITION) and yarp.VOCAB_CM_POSITION or yarp.Vocab_encode('pos') # python

jgvictores on 4 Jun 2018

I did not followed in detail the discussion, but if I understood correctly I guess the easiest solution is to merge the matlab branch of SWIG with a recent version of SWIG, push it to a SWIG fork and use the resulting branch for generating the yarp-matlab-bindings . I can have a look to it in this week.

traversaro on 4 Jun 2018

👍1

@traversaro Yes, that most definitely seems like the most elegant solution.

jgvictores on 4 Jun 2018

Hi! Noticed that I got tagged in this issue. If you have some fixes to our MATLAB branch, especially in bringing it up-to-date, that'd be great. We're using the branch all the time for our project, but merging it with SWIG master is been lagging behind.

jaeandersson on 5 Jun 2018

Hi @jaeandersson , thanks for dropping by. Yes, we've just merged in our fork of SWIG a fix for the issue tracked here: the issue was causing the wrapping of global defines (#define or %define) to fail, converting input char to variables. The fix (https://github.com/swig/swig/pull/781) for that issue was already integrated in SWIG master branch . We didn't merge the actual master branch to the matlab branch, but just rebased the fix https://github.com/swig/swig/pull/781.

We're currently testing it. We can pull request it to your fork as soon as it's ready.

nunoguedelha on 5 Jun 2018

👍1

This has been fixed by:

https://github.com/robotology-dependencies/swig/pull/1: Fix wrapping of %define and #define expressions containing chars
https://github.com/robotology-dependencies/swig/pull/2: Restore subsref and subsasgn methods in SwigRef class
https://github.com/robotology-playground/yarp-matlab-bindings/pull/26: Regenerate the bindings with the new SWIG robotology

nunoguedelha on 6 Jun 2018

@traversaro should we move this issue to yarp-matlab-bindings before closing or just leave it?

nunoguedelha on 6 Jun 2018

@nunoguedelha I think we can directly close it.

traversaro on 6 Jun 2018

👍1

Was this page helpful?

0 / 5 - 0 ratings

Related issues

yarp devel does not compile on macOS Sierra with CMake 3.10.0

traversaro · 4Comments

BufferedPort has method isClosed() and Port has method isOpen()

Nicogene · 3Comments

Unable to include LogStream with CasAdi

Giulero · 3Comments

#1000

drdanz · 3Comments

Network.h contains methods that should be somewhere else

Nicogene · 3Comments