Build failure on Fedora 30 (using GCC9)

[cottsay]

I'm seeing a lot of messages related to -Werror=deprecated-copy. Adding --cmake-args -DCMAKE_CXX_FLAGS=-Wno-error=deprecated-copy allows the build to proceed. The problem might be in Eigen - I'm not sure yet.

Example failure:

In file included from /usr/include/eigen3/Eigen/Core:461,
                 from /usr/include/eigen3/Eigen/Dense:1,
                 from /opt/ros_src/dashing/src/ros2/rviz/rviz_rendering/include/rviz_rendering/objects/covariance_visual.hpp:37,
                 from /opt/ros_src/dashing/src/ros2/rviz/rviz_rendering/src/rviz_rendering/objects/covariance_visual.cpp:31:
/usr/include/eigen3/Eigen/src/Core/CwiseBinaryOp.h: In instantiation of ‘Eigen::CwiseBinaryOp<BinaryOp, Lhs, Rhs>::CwiseBinaryOp(const Lhs&, const Rhs&, const BinaryOp&) [with BinaryOp = Eigen::internal::scalar_product_op<double, double>; LhsType = const Eigen::Transpose<const Eigen::Block<const Eigen::Transpose<const Eigen::Block<const Eigen::Matrix<double, 2, 2>, -1, 1, false> >, 1, -1, true> >; RhsType = const Eigen::Block<const Eigen::Block<Eigen::Block<Eigen::Matrix<double, 2, 2>, -1, -1, false>, -1, -1, false>, -1, 1, true>; Eigen::CwiseBinaryOp<BinaryOp, Lhs, Rhs>::Lhs = Eigen::Transpose<const Eigen::Block<const Eigen::Transpose<const Eigen::Block<const Eigen::Matrix<double, 2, 2>, -1, 1, false> >, 1, -1, true> >; Eigen::CwiseBinaryOp<BinaryOp, Lhs, Rhs>::Rhs = Eigen::Block<const Eigen::Block<Eigen::Block<Eigen::Matrix<double, 2, 2>, -1, -1, false>, -1, -1, false>, -1, 1, true>]’:
/usr/include/eigen3/Eigen/src/Core/../plugins/MatrixCwiseBinaryOps.h:25:10:   required from ‘const Eigen::CwiseBinaryOp<Eigen::internal::scalar_product_op<typename Eigen::internal::traits<T>::Scalar, typename Eigen::internal::traits<OtherDerived>::Scalar>, const Derived, const OtherDerived> Eigen::MatrixBase<Derived>::cwiseProduct(const Eigen::MatrixBase<OtherDerived>&) const [with OtherDerived = Eigen::Block<const Eigen::Block<Eigen::Block<Eigen::Matrix<double, 2, 2>, -1, -1, false>, -1, -1, false>, -1, 1, true>; Derived = Eigen::Transpose<const Eigen::Block<const Eigen::Transpose<const Eigen::Block<const Eigen::Matrix<double, 2, 2>, -1, 1, false> >, 1, -1, true> >; typename Eigen::internal::traits<OtherDerived>::Scalar = double; typename Eigen::internal::traits<T>::Scalar = double]’
/usr/include/eigen3/Eigen/src/Core/ProductEvaluators.h:563:72:   required from ‘const CoeffReturnType Eigen::internal::product_evaluator<Eigen::Product<Lhs, Rhs, 1>, ProductTag, Eigen::DenseShape, Eigen::DenseShape>::coeff(Eigen::Index) const [with Lhs = Eigen::Transpose<const Eigen::Block<const Eigen::Matrix<double, 2, 2>, -1, 1, false> >; Rhs = Eigen::Block<Eigen::Block<Eigen::Matrix<double, 2, 2>, -1, -1, false>, -1, -1, false>; int ProductTag = 3; typename Eigen::internal::traits<typename Eigen::Product<Lhs, Rhs, 1>::Rhs>::Scalar = double; typename Eigen::internal::traits<typename Eigen::Product<Lhs, Rhs, 1>::Lhs>::Scalar = double; Eigen::internal::product_evaluator<Eigen::Product<Lhs, Rhs, 1>, ProductTag, Eigen::DenseShape, Eigen::DenseShape>::CoeffReturnType = double; Eigen::Index = long int]’
/usr/include/eigen3/Eigen/src/Core/AssignEvaluator.h:637:5:   required from ‘void Eigen::internal::generic_dense_assignment_kernel<DstEvaluatorTypeT, SrcEvaluatorTypeT, Functor, Version>::assignCoeff(Eigen::Index) [with DstEvaluatorTypeT = Eigen::internal::evaluator<Eigen::Map<Eigen::Matrix<double, 1, -1, 1, 1, 2>, 0, Eigen::Stride<0, 0> > >; SrcEvaluatorTypeT = Eigen::internal::evaluator<Eigen::Product<Eigen::Transpose<const Eigen::Block<const Eigen::Matrix<double, 2, 2>, -1, 1, false> >, Eigen::Block<Eigen::Block<Eigen::Matrix<double, 2, 2>, -1, -1, false>, -1, -1, false>, 1> >; Functor = Eigen::internal::assign_op<double, double>; int Version = 0; Eigen::Index = long int]’
/usr/include/eigen3/Eigen/src/Core/AssignEvaluator.h:497:7:   required from ‘static void Eigen::internal::dense_assignment_loop<Kernel, 1, 0>::run(Kernel&) [with Kernel = Eigen::internal::generic_dense_assignment_kernel<Eigen::internal::evaluator<Eigen::Map<Eigen::Matrix<double, 1, -1, 1, 1, 2>, 0, Eigen::Stride<0, 0> > >, Eigen::internal::evaluator<Eigen::Product<Eigen::Transpose<const Eigen::Block<const Eigen::Matrix<double, 2, 2>, -1, 1, false> >, Eigen::Block<Eigen::Block<Eigen::Matrix<double, 2, 2>, -1, -1, false>, -1, -1, false>, 1> >, Eigen::internal::assign_op<double, double>, 0>]’
/usr/include/eigen3/Eigen/src/Core/AssignEvaluator.h:741:37:   required from ‘void Eigen::internal::call_dense_assignment_loop(DstXprType&, const SrcXprType&, const Functor&) [with DstXprType = Eigen::Map<Eigen::Matrix<double, 1, -1, 1, 1, 2>, 0, Eigen::Stride<0, 0> >; SrcXprType = Eigen::Product<Eigen::Transpose<const Eigen::Block<const Eigen::Matrix<double, 2, 2>, -1, 1, false> >, Eigen::Block<Eigen::Block<Eigen::Matrix<double, 2, 2>, -1, -1, false>, -1, -1, false>, 1>; Functor = Eigen::internal::assign_op<double, double>]’
/usr/include/eigen3/Eigen/src/Core/AssignEvaluator.h:879:31:   [ skipping 16 instantiation contexts, use -ftemplate-backtrace-limit=0 to disable ]
/usr/include/eigen3/Eigen/src/Eigenvalues/Tridiagonalization.h:449:11:   required from ‘static void Eigen::internal::tridiagonalization_inplace_selector<MatrixType, Size, IsComplex>::run(MatrixType&, DiagonalType&, SubDiagonalType&, bool) [with DiagonalType = Eigen::Matrix<double, 2, 1>; SubDiagonalType = Eigen::Matrix<double, 1, 1, 0, 1, 1>; MatrixType = Eigen::Matrix<double, 2, 2>; int Size = 2; bool IsComplex = false]’
/usr/include/eigen3/Eigen/src/Eigenvalues/Tridiagonalization.h:430:55:   required from ‘void Eigen::internal::tridiagonalization_inplace(MatrixType&, DiagonalType&, SubDiagonalType&, bool) [with MatrixType = Eigen::Matrix<double, 2, 2>; DiagonalType = Eigen::Matrix<double, 2, 1>; SubDiagonalType = Eigen::Matrix<double, 1, 1, 0, 1, 1>]’
/usr/include/eigen3/Eigen/src/Eigenvalues/SelfAdjointEigenSolver.h:437:39:   required from ‘Eigen::SelfAdjointEigenSolver<MatrixType>& Eigen::SelfAdjointEigenSolver<_MatrixType>::compute(const Eigen::EigenBase<OtherDerived>&, int) [with InputType = Eigen::Matrix<double, 2, 2>; _MatrixType = Eigen::Matrix<double, 2, 2>]’
/usr/include/eigen3/Eigen/src/Eigenvalues/SelfAdjointEigenSolver.h:168:7:   required from ‘Eigen::SelfAdjointEigenSolver<_MatrixType>::SelfAdjointEigenSolver(const Eigen::EigenBase<OtherDerived>&, int) [with InputType = Eigen::Matrix<double, 2, 2>; _MatrixType = Eigen::Matrix<double, 2, 2>]’
/opt/ros_src/dashing/src/ros2/rviz/rviz_rendering/src/rviz_rendering/objects/covariance_visual.cpp:118:3:   required from ‘std::tuple<typename SolverParams::EigenvaluesType, typename SolverParams::EigenvectorsType> rviz_rendering::{anonymous}::diagonalizeCovariance(const Eigen::Matrix<double, SolverParams::dimension, SolverParams::dimension>&) [with SolverParams = rviz_rendering::{anonymous}::Cov2DSolverParams; typename SolverParams::EigenvaluesType = Ogre::Vector2; typename SolverParams::EigenvectorsType = Ogre::Matrix3]’
/opt/ros_src/dashing/src/ros2/rviz/rviz_rendering/src/rviz_rendering/objects/covariance_visual.cpp:190:56:   required from here
/usr/include/eigen3/Eigen/src/Core/CwiseBinaryOp.h:105:49: error: implicitly-declared ‘Eigen::Transpose<const Eigen::Block<const Eigen::Transpose<const Eigen::Block<const Eigen::Matrix<double, 2, 2>, -1, 1, false> >, 1, -1, true> >::Transpose(const Eigen::Transpose<const Eigen::Block<const Eigen::Transpose<const Eigen::Block<const Eigen::Matrix<double, 2, 2>, -1, 1, false> >, 1, -1, true> >&)’ is deprecated [-Werror=deprecated-copy]
  105 |       : m_lhs(aLhs), m_rhs(aRhs), m_functor(func)
      |                                                 ^

[richmattes]

I think it's a combination of rviz_rendering setting -Werror, and the addition of -Wdeprecated-copy in gcc-9. It appears that Eigen triggers this new warning because it defines an assignment operator without a copy constructor in a bunch of its classes. There's a couple failures in assimp types too.

rviz_rendering should remove -Werror from the default build flags.

[cottsay]

Looks like assimp has the same issue. Making things worse, I'm seeing some sort of compiler problem that causes messages like this:

cc1plus: error: unrecognized command line option ‘-Wno-gnu-zero-variadic-macro-arguments’ [-Werror]

...whenever any -Wdeprecated-copy messages come up, even if they're just warnings. Strangely, suppressing the deprecated-copy messages entirely with -Wno-deprecated-copy seems to work around the quirk, but -Wno-error=deprecated-copy is not enough.

I'm not suggesting we merge these changes, but I hacked around the problems in the Wdeprecated-copy branch. Hopefully we can use that as a guide to properly fixing the one occurrence within the RViz code and also develop a plan for dealing with the Eigen and Assimp occurrences.

[richmattes]

I'm seeing the same things with disabling deprecated-copy via $CXXFLAGS. From the manual:

When an unrecognized warning option is requested (e.g., -Wunknown-warning), GCC emits a diagnostic stating that the option is not recognized. However, if the -Wno- form is used, the behavior is slightly different: no diagnostic is produced for -Wno-unknown-warning unless other diagnostics are being produced. This allows the use of new -Wno- options with old compilers, but if something goes wrong, the compiler warns that an unrecognized option is present.

Basically, gcc suppresses the -Wno-gnu-zero-variadic-macro-arguments unless another diagnostic happens to be printed. Disabling the error with -Wno-error=deprecated-copy still prints the warning diagnostic, which causes the unrecognized option diagnostic to also be printed and cause an error. -Wno-deprecated-copy suppresses the diagnostic completely, and the unrecognized option diagnostic is also suppressed.

The variadic macro warning is tested here. It should be testing for the positive -Wgnu-zero-variadic-macro-arguments, which does fail the check on gcc-9. Reproduce with the following:

$ cat << EOF > test.cpp
int main(return 0;}
EOF
$ g++ -c test.cpp -Wno-gnu-zero-variadic-macro-arguments
$ g++ -c test.cpp -Wgnu-zero-variadic-macro-arguments
g++: error: unrecognized command line option ‘-Wgnu-zero-variadic-macro-arguments’

[cottsay]

That's some great investigation, @richmattes. I never would have expected that behavior. Let's try switching to the positive argument test and see if that passes CI.

Since you discovered the root cause, would you like to make the PR?

[allenh1]

@cottsay I recently built ROS 2 with GCC 10 (off svn). I had to add two patches to make it build, so I'll just paste them here as a workaround instead of PRing them in (since they don't solve the root cause).

diff --git a/rviz_common/CMakeLists.txt b/rviz_common/CMakeLists.txt
index 4a460c49..8010209f 100644
--- a/rviz_common/CMakeLists.txt
+++ b/rviz_common/CMakeLists.txt
@@ -26,10 +26,11 @@ if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   add_compile_options(-Wall -Wextra -Wpedantic -Werror)
   # TODO(wjwwood): try to remove this -- currently needed to pass on CI
   include(CheckCXXCompilerFlag)
-  check_cxx_compiler_flag(-Wno-gnu-zero-variadic-macro-arguments HAS_W_FLAG)
-  if(HAS_W_FLAG)
-    add_compile_options(-Wno-gnu-zero-variadic-macro-arguments)
-  endif()
+  add_compile_options(-Wno-error=deprecated-copy)
+#  check_cxx_compiler_flag(-Wno-gnu-zero-variadic-macro-arguments HAS_W_FLAG)
+#  if(HAS_W_FLAG)
+#    add_compile_options(-Wno-gnu-zero-variadic-macro-arguments)
+#  endif()
 endif()

---

diff --git a/rviz_rendering/CMakeLists.txt b/rviz_rendering/CMakeLists.txt
index 7f7769ed..1522474a 100644
--- a/rviz_rendering/CMakeLists.txt
+++ b/rviz_rendering/CMakeLists.txt
@@ -8,7 +8,7 @@ if(NOT CMAKE_CXX_STANDARD)
 endif()

 if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
-  add_compile_options(-Wall -Wextra -Wpedantic -Werror)
+  add_compile_options(-Wall -Wextra -Wpedantic -Werror -Wno-error=deprecated-copy)
 endif()

 # Tests currently only run on OS X @ OSRF jenkins

---

@wjwwood Is there a reason to have -Werror on? If the CI counts warnings, could the flag be removed so that warnings continue to show up while building locally, but so that the build don't fail as a result of them? I'm only advocating for this because GCC tends to add new warnings into new compiler versions, and this tends to block RViz builds in both ROS 1 & ROS 2 when the flag is there.

[cottsay]

Those are the same problems we're seeing with GCC9, @allenh1. Looks like there isn't anything new in GCC 10, but please try building the Wdeprecated-copy branch to confirm.

[wjwwood]

Nah, there's no good reason, other than I was trying to make sure it didn't have warnings, but now I do that with CFLAGS and CXXFLAGS. Removing -Werror is fine.

[richmattes]

I can PR a fix for the -Wno-gnu-zero-variadic-macro-arguments test, but if it's no longer needed it might be worthwhile to just remove the whole section as part of #420.