C++11 error on ROS buildfarm for kinetic about geometric_shapes HOT 18 CLOSED

Oops - I'll submit a PR to fix this. I didn't realize that packages could export the C++11 flag.

from geometric_shapes.

@tfoote I'll fix geometric_shapes to export the -std=c++11 flag; is that OK though? I believe geometric_shapes is part of desktop_full, and according to REP 0003 "the API of the packages included in desktop-full will not use any C++11-specific feature.". The use of std::shared_ptr in geometric_shapes is in the constructor of the octomap class, which seems like part of the API: https://github.com/ros-planning/geometric_shapes/blob/1a020ba1969b7de8d7772e2a82693411733cad65/include/geometric_shapes/shapes.h#L254

Does that need to be backed out? It looks like geometric_shapes was switched to use std::shared_ptr to make it easier to interoperate with FCL 0.5, but I'm guessing that there is some way to do that without exposing it in the API?

from geometric_shapes.

It looks like collada_urdf (which depends on geometric_shapes) doesn't even build with -std=c++11:

/home/jbinney/ws/transplanting/src/iron_ox/third_party/robot_model/collada_urdf/src/collada_urdf.cpp: In member fu
nction ‘void collada_urdf::ColladaWriter::_WriteRobot(int)’:
/home/jbinney/ws/transplanting/src/iron_ox/third_party/robot_model/collada_urdf/src/collada_urdf.cpp:85:46: error:
 ‘typeof’ was not declared in this scope
 #define FOREACH(it, v) for(typeof((v).begin())it = (v).begin(); it != (v).end(); (it)++)
                                              ^
/home/jbinney/ws/transplanting/src/iron_ox/third_party/robot_model/collada_urdf/src/collada_urdf.cpp:841:9: note: 
in expansion of macro ‘FOREACH’
         FOREACH(it, _ikmout->vkinematicsbindings) {
         ^
/home/jbinney/ws/transplanting/src/iron_ox/third_party/robot_model/collada_urdf/src/collada_urdf.cpp:841:17: error
: ‘it’ was not declared in this scope
         FOREACH(it, _ikmout->vkinematicsbindings) {
                 ^
/home/jbinney/ws/transplanting/src/iron_ox/third_party/robot_model/collada_urdf/src/collada_urdf.cpp:85:65: note: 
in definition of macro ‘FOREACH’
 #define FOREACH(it, v) for(typeof((v).begin())it = (v).begin(); it != (v).end(); (it)++)
                                                                 ^
/home/jbinney/ws/transplanting/src/iron_ox/third_party/robot_model/collada_urdf/src/collada_urdf.cpp:844:146: erro
r: ‘str’ was not declared in this scope
             daeSafeCast<domKinematics_newparam::domSIDREF>(abm->add(COLLADA_ELEMENT_SIDREF))->setValue(str(boost:
:format("%s/%s")%askid%it->first).c_str());

I can update collada_urdf to fix that, but I think the right answer is to remove std::shared_ptr from the geometric_shapes API.

from geometric_shapes.

I went ahead and created a PR that exports the flag, in case we decide to go that route: #50

from geometric_shapes.

I'd like to keep the C++11 support so that downstream packages in Kinetic can use C++11 as stated in the Kinetic minimum requirements. We converted geometric_shapes because it uses boost::variant that has c++11/c++03 incompatibilities in this issue.

Still, I see how this is tricky...

from geometric_shapes.

Ah, I hadn't seen that bit about the boost variant issue. In that case I think we definitely need guidance from the ROS core team.

from geometric_shapes.

Look at the error messages in the first message:

01:11:58 In file included from /tmp/binarydeb/ros-kinetic-collada-urdf-1.12.3/src/collada_urdf.cpp:52:0:
01:11:58 /opt/ros/kinetic/include/resource_retriever/retriever.h:81:38: warning: defaulted and deleted functions only available with -std=c++11 or -std=gnu++11
01:11:58 Retriever(const Retriever & ret) = delete;

The same bug also exists in the resource_retriever module, that is already released in kinetic. @jacquelinekay you introduced the delete there, so I suppose you are interested in this issue too.

Regarding geometric_shapes, this boils down to the following:
@tfoote We currently use C++11 in the geometric_shapes API, because

• it's a mess to mix boost::shared_ptr and std::shared_ptr and we need std::shared_ptr for FCL
• we use boost::variant in the interface and if this library is compiled with -std=c++98 no package using c++11 will be able to link to geometric_shapes. On the other hand, compiling this library with c++11 makes it impossible to link the library from any project that does not use c++11.

We would therefore like to ask for your permission to break the kinetic REP and expose a c++11 dependency.

from geometric_shapes.

@v4hn It sounds like this might be a reasonable time to make an exception from the REP. Can you start a thread on http://discourse.ros.org/c/release/kinetic proposing the exception and people can follow up there.

from geometric_shapes.

from geometric_shapes.

The email in of topic creation is experimental because the experience of creating tags and categories cleanly is hard to provide a good user experience. It's recommended to start the thread by using the web UI. But you can try using: [email protected]

All notification emails support replies by email.

from geometric_shapes.

it looks like that domain does not exist.
This has nothing to do with c++11 flags, so let's continue this here.

from geometric_shapes.

Can you start a thread on http://discourse.ros.org/c/release/kinetic proposing the exception and people can follow up there.

@v4hn I think this was forgotten?

from geometric_shapes.

On Mon, Aug 08, 2016 at 02:44:29PM -0700, Dave Coleman wrote:

@v4hn I think this was forgotten?

No it was not. Somebody who has the time and the interest in this should start a discussion.

1. I already expressed my opinion on this: Either someone who has an interest in this(e.g. @de-vri-es)
   steps up and provides a pull-request that removes the boost-variant and the std-shared-ptr from this module's headers
   (this would hopefully make the interface c++98 compatible) or we should add the standard flag via @jonbinney 's request.
   Gazebo moved to C++11 in their public headers long ago and chose the explicit mechanism.
   It was a pain to add the appropriate flags to all dependant modules, e.g. see here.
2. For the moment I'm not too interested in the kinetic release myself (although I agree that it should happen soon)

from geometric_shapes.

See http://discourse.ros.org/t/std-shared-ptr-in-the-geometric-shapes-public-api/395

from geometric_shapes.

Thanks @de-vri-es

Gazebo moved to C++11 in their public headers long ago and chose the explicit mechanism.

Good point. Having a package in desktop-full that needs to play nice with FCL and octomap (who don't care so much about desktop-full) is a pain. Easy fix: create moveit_geometric_shapes and be done with it

from geometric_shapes.

Easy fix: create moveit_geometric_shapes and be done with it

No.

from geometric_shapes.

Sorry I'm not following but looks like 0.5.1 for Kinetic seems to be built successfully. Can this issue be closed?

from geometric_shapes.

Yep, this should be resolved now that packages using geometric_shapes use the c++11 flag.

from geometric_shapes.