Setting CMAKE_CXX_STANDARD in melodic

[wjwwood]

Following up on ros-infrastructure/rep#139 (comment).

Basically, some packages now use C++11/14 stuff in their headers, and if downstream packages do not set the compiler standard themselves, but use those headers you get errors like this on macOS:

/Users/william/melodic/install/include/class_loader/class_loader_core.hpp:300:8: warning: 'auto' type specifier is a C++11 extension [-Wc++11-extensions]
  for (auto & it : factory_map) {
       ^

/Users/william/melodic/install/include/class_loader/class_loader.hpp:82:23: warning: alias declarations are a C++11 extension [-Wc++11-extensions]
  using DeleterType = std::function<void(Base *)>;
                      ^
/Users/william/melodic/install/include/class_loader/class_loader.hpp:85:21: warning: alias declarations are a C++11 extension [-Wc++11-extensions]
  using UniquePtr = std::unique_ptr<Base, DeleterType<Base>>;
                    ^

/Users/william/melodic/install/include/class_loader/class_loader.hpp:85:59: error: a space is required between consecutive right angle brackets (use '> >')
  using UniquePtr = std::unique_ptr<Base, DeleterType<Base>>;
                                                          ^~
                                                          > >

These occur because even though the compiler on macOS supports C++11/14/17, it still defaults to C++03. This might change in the future, but there's no ETA on this. So for now, source builds are broken without intervention.

---

In the above linked discussion, there seemed to be a preference for doing something like:

if(NOT DEFINED CMAKE_CXX_STANDARD)
  set(CMAKE_CXX_STANDARD 14)
endif()

In one of the "core" packages. I also think this is the best thing we can do for now.

I believe catkin makes the most sense for this conditional setting, for a few reasons:

• It has ~100% coverage as a dependency in ROS 1
• It is find_package()'ed by almost everything that depends on it

We (@mikaelarguedas and I) considered other packages like cmake_modules, roslib, or ros_environment.
However, none of these options are find_package'ed by all of ROS. In order for the above logic to work, every package needs to either directly or indirectly find_package the package which contains the logic.
ros_environment is used by roslib and gets into almost every workspace, but many packages would not have their cmake logic affected.

So, I think catkin makes the most sense. However, it would require us to fork for melodic-devel (in my opinion).

But I wanted to get feedback on the idea before spending more time on it.

---

An alternative, but less attractive (from a user point of view), solution would be to have people building on machines where this is an issue (essentially just on macOS right now) include the cmake argument -DCMAKE_CXX_STANDARD=14 anywhere they invoke a build. That would be with catkin_make, catkin_make_isolated, catkin_tools, or just plain invoking cmake.

I'm interested to know what you think about this option @mikepurvis.

[NikolausDemmel]

At least OSX (with homebrew have a rolling release) this is already an issue for all distros, not just for melodic, since dependencies like yaml-cpp (e.g. used by camera_calibration_parsers in image_common) now needs at least C++11: ros-perception/image_common#78 . I haven't checked if desktop_full compiles with -DCMAKE_CXX_STANDARD=14 but that seems to be a reasonable solution, probably better than compiling individual packages with different standards (aren't there ABI incompatibilities between STL for C++03 and C++11/14, e.g. with string and list, ...).

So, I think catkin makes the most sense. However, it would require us to fork for melodic-devel (in my opinion).

I agree that setting this in catkin makes sense, but do we really need to fork? Could this not be conditional on ROS_DISTRO, or is that not necessarily available?

There might be an argument to enable this by default not only for melodic, at least on some platforms that are currently already broken. Ofc it needs to be carefully considered, but my suspicion is that no one will complain if we enable this for OSX also for older distros to make builds succeed without need to manually pass -DCMAKE_CXX_STANDARD=14 or other workarounds.

[NikolausDemmel]

For the record, I've had success with passing additionally -DCMAKE_CXX_STANDARD=14 in catkin config to compile lunar on macos Sierra (10.12).

[mikepurvis]

In terms of building on Mac, I had been picturing that I would have to handle this with another CMake arg passed in here (as described by @NikolausDemmel above):

https://github.com/mikepurvis/ros-install-osx/blob/4e342f604960e291320e6acba62ff349747707d5/install#L176-L184

However, assuming this is going to affect other rolling release systems, I would agree with handling it centrally in catkin. From the perspective of Clearpath, we're building lots of Melodic versions of things on a Xenial base system, so anywhere a package assumes >=C++11 without having an explicit flag for it is going to cause trouble on GCC 5— we can handle our bundle build case with global CMake args, but then it trips up developers doing local source builds.

tl;dr 👍 from me to this.

[dirk-thomas]

I am not sure if hard coding any kind of value (even if only for macOS) is the best approach. Some higher level packages might not even work with the changed setting? Wouldn't it be easier to add this to the from-source instructions for macOS?

And the end of the day I don't mind how it is done for macOS. If you would like to add a conditional block which set the flag in catkin for macOS that is fine.

[kunaltyagi]

Sorry for being late to the party. My perspective is of a person compiling the code for a platform Melodic is not officially not released on, ie. one of the people for whom handling the issue centrally would help, no matter the platform (macOS or not)

Even if the value is not hard coded, throwing a proper error makes sense to inform the user that the issue would not be handled by the maintainers. Simply adding this on the wiki/Installation/Source would also resolve the issue (instead of one redirection away at REP 3).

One alternative to the CMake solution might be official C++ macros to check the version/capabilities as used in core packages. This might increase the load on the maintainers but allows more compatibility (such as fallback on boost if std version is not enough).

Another potential instead of catkin would be rosdep which anyways checks for dependencies, and a C++ compiler with default standard 14 is a dependency. This has the benefit of not forcing the standard to a lower value in future (because why force the user's code to not use C++17/2a if the compiler supports that by default, on the other end of the spectrum), but does nothing to resolve the error of broken builds (which I believe is a clang issue. clang has default standard at C++98, while gcc has a different default standard).

For me, the best solution might be a CMake one itself. It'd be a complicated one which checks for default standard, if it's less than 14, then sets it to 14 with a warning message. This would be possible with try_compile but requires a cmake_minimum_version(3.0.2) while ROS is mostly using 2.8.3 (I might be wrong) which is long overdue for a change. (Actually cmake_cxx_standard itself requires 3.1.3 without which the command fails with no side effects and no errors)

[dirk-thomas]

Closing since there is nothing to be done in this repo.