How can/why is ShapeShifter::instantiate const?

[krisklau]

Hi,

Regarding the function https://github.com/facontidavide/ros_type_introspection/blob/06c3321fbe256c2c294ba62f3d4479dfb8d6a872/include/ros_type_introspection/utils/shape_shifter.hpp#L78

Note the const qualifier.

I suddenly had trouble compiling PlotJuggler with ROS, giving me build errors of the form:

In file included from <..>/plotjuggler/ws_plotjuggler/src/PlotJuggler/plugins/ROS/RosoutPublisher/rosout_publisher.cpp:3:
In file included from <..>/plotjuggler/ws_plotjuggler/src/PlotJuggler/plugins/ROS/RosoutPublisher/../shape_shifter_factory.hpp:4:
/opt/ros/kinetic/include/ros_type_introspection/utils/shape_shifter.hpp:207:31: error: 
      no matching constructor for initialization of
      'ros::serialization::IStream'
  ros::serialization::IStream s(msgBuf_.data(), msgBuf_.size() );
                              ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/ros/kinetic/include/ros/serialization.h:749:3: note: candidate constructor
      not viable: 1st argument ('const unsigned char *') would lose const
      qualifier
  IStream(uint8_t* data, uint32_t count)
  ^

Which was odd, as I had it compiling fine before, without any obvious changes to my setup. As far as I can understand though, the compiler correctly asserts that accessing the vector data in the implementation

https://github.com/facontidavide/ros_type_introspection/blob/06c3321fbe256c2c294ba62f3d4479dfb8d6a872/include/ros_type_introspection/utils/shape_shifter.hpp#L207

through the vector<>::data() function from within the const method instantiate indeed returns the const pointer const uint8_t*:

https://en.cppreference.com/w/cpp/container/vector/data

Which constructs an IStream object which (for kinetic at least), in serialization.h is not const: http://docs.ros.org/kinetic/api/roscpp_serialization/html/serialization_8h_source.html#l00709

Removing the const qualifier from instantiate resulted in a working build.

Now, obviously there is something with my setup that triggers this now as I had it building before and no one else has had this issue compiling introspection or plotjuggler. However, should this method really be const?

Best, Kristian

[facontidavide]

This is kind of weird, because, as you mentioned, my compiler doesn't complain about it.

• It makes sense for instantiate() to be const because it is not supposed to modify the original object.
• On the other hand, I do not understand why IStream constructor won't require a const pointer, in theory (but you are right, it doesn't).
• I have no idea how it could have been working so far....

Which compiler are you using?

[facontidavide]

More surprisingly, the same should be true for the original version of ShapeShfter...

http://docs.ros.org/jade/api/topic_tools/html/classtopic__tools_1_1ShapeShifter.html#a345259ecbb55e0f5d2db7269c81ff164

[facontidavide]

practically speaking I can fix the problem adding mutable to msgBuf_, but I wonder why I have never detected this before

[krisklau]

Hmm, thanks for the inputs. Indeed, I find this strange.

Currently I am working on a plugin for plotjuggler, which is also built with catkin.

I got back to a working build with a fresh workspace and upstream plotjuggler (before I had upstream but an old workspace, probably some cmake cache remained after a clean), so I suspect that my new plugin somehow influenced it. (however no custom compiler flags or anything special in the cmake file). I will try to reproduce in the new workspace. Edit: This at least explains why it triggered on my machine with upstream build.

As far as I can see, nothing actually calls the instantiate-method? Could that cause the template not to be generated in some cases?

The system is Ubuntu 16.04, default compiler, ros kinetic installed with apt-get.

[facontidavide]

just pushed a "fix" replacing the old instantiate method with a new one

[krisklau]

Update: Reproduced error on fresh workspace and upstream plotjuggler.

Was indeed compiler choice. When compiling with clang-6.0, the errors occurs.

[krisklau]

Anyhow, thanks for looking into it and fixing the issue :+1: