borglab / gtsam

GTSAM is a library of C++ classes that implement smoothing and mapping (SAM) in robotics and vision, using factor graphs and Bayes networks as the underlying computing paradigm rather than sparse matrices.
http://gtsam.org
Other
2.55k stars 754 forks source link

Build failure for tests with GCC 13 #1738

Open ottojo opened 5 months ago

ottojo commented 5 months ago

Description

testStdOptionalSerialization.cpp fails to build with error: invalid use of incomplete type ‘struct boost::serialization::U’.

Steps to reproduce

foo@bar:~$ g++ --version
g++ (GCC) 13.2.1 20230801
mkdir build && cd build
cmake ..
make check

Build failure:

[ 40%] Building CXX object gtsam/base/tests/CMakeFiles/testStdOptionalSerialization.dir/testStdOptionalSerialization.cpp.o
In file included from /usr/include/c++/13.2.1/bits/stl_pair.h:60,
                 from /usr/include/c++/13.2.1/bits/stl_algobase.h:64,
                 from /usr/include/c++/13.2.1/bits/specfun.h:43,
                 from /usr/include/c++/13.2.1/cmath:3699,
                 from /home/jonas/Workspace/gtsam/CppUnitLite/Test.h:25,
                 from /home/jonas/Workspace/gtsam/CppUnitLite/TestHarness.h:23,
                 from /home/jonas/Workspace/gtsam/gtsam/base/tests/testStdOptionalSerialization.cpp:1:
/usr/include/c++/13.2.1/type_traits: In instantiation of ‘constexpr const bool std::is_trivially_copy_constructible_v<boost::serialization::U>’:
/usr/include/boost/serialization/split_free.hpp:58:13:   required from ‘static void boost::serialization::free_loader<Archive, T>::invoke(Archive&, T&, unsigned int) [with Archive = boost::archive::text_iarchive; T = std::optional<int>]’
/usr/include/boost/serialization/split_free.hpp:74:18:   required from ‘void boost::serialization::split_free(Archive&, T&, unsigned int) [with Archive = boost::archive::text_iarchive; T = std::optional<int>]’
/home/jonas/Workspace/gtsam/gtsam/base/std_optional_serialization.h:103:35:   required from ‘void boost::serialization::serialize(Archive&, std::optional<_Up>&, unsigned int) [with Archive = boost::archive::text_iarchive; T = int]’
/usr/include/boost/serialization/serialization.hpp:109:14:   required from ‘void boost::serialization::serialize_adl(Archive&, T&, unsigned int) [with Archive = boost::archive::text_iarchive; T = std::optional<int>]’
/usr/include/boost/archive/detail/iserializer.hpp:189:40:   [ skipping 3 instantiation contexts, use -ftemplate-backtrace-limit=0 to disable ]
/usr/include/boost/archive/detail/iserializer.hpp:463:22:   required from ‘static void boost::archive::detail::load_non_pointer_type<Archive>::invoke(Archive&, T&) [with T = std::optional<int>; Archive = boost::archive::text_iarchive]’
/usr/include/boost/archive/detail/iserializer.hpp:626:18:   required from ‘void boost::archive::load(Archive&, T&) [with Archive = text_iarchive; T = std::optional<int>]’
/usr/include/boost/archive/detail/common_iarchive.hpp:67:22:   required from ‘void boost::archive::detail::common_iarchive<Archive>::load_override(T&) [with T = std::optional<int>; Archive = boost::archive::text_iarchive]’
/usr/include/boost/archive/basic_text_iarchive.hpp:70:52:   required from ‘void boost::archive::basic_text_iarchive<Archive>::load_override(T&) [with T = std::optional<int>; Archive = boost::archive::text_iarchive]’
/usr/include/boost/archive/text_iarchive.hpp:82:52:   required from ‘void boost::archive::text_iarchive_impl<Archive>::load_override(T&) [with T = std::optional<int>; Archive = boost::archive::text_iarchive]’
/usr/include/boost/archive/detail/interface_iarchive.hpp:68:36:   required from ‘Archive& boost::archive::detail::interface_iarchive<Archive>::operator>>(T&) [with T = std::optional<int>; Archive = boost::archive::text_iarchive]’
/home/jonas/Workspace/gtsam/gtsam/base/tests/testStdOptionalSerialization.cpp:26:9:   required from here
/usr/include/c++/13.2.1/type_traits:3279:7: error: invalid use of incomplete type ‘struct boost::serialization::U’ [-fpermissive]
 3279 |     = __is_trivially_constructible(_Tp, __add_lval_ref_t<const _Tp>);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/boost/serialization/shared_ptr.hpp:29,
                 from /home/jonas/Workspace/gtsam/gtsam/base/serialization.h:31,
                 from /home/jonas/Workspace/gtsam/gtsam/base/serializationTestHelpers.h:27,
                 from /home/jonas/Workspace/gtsam/gtsam/base/tests/testStdOptionalSerialization.cpp:8:
/usr/include/boost/serialization/shared_ptr_helper.hpp:45:16: note: forward declaration of ‘struct boost::serialization::U’
   45 |     SPT< class U > &t,
      |                ^
/usr/include/c++/13.2.1/type_traits: In instantiation of ‘constexpr const bool std::is_trivially_move_constructible_v<boost::serialization::U>’:
/usr/include/boost/serialization/split_free.hpp:58:13:   required from ‘static void boost::serialization::free_loader<Archive, T>::invoke(Archive&, T&, unsigned int) [with Archive = boost::archive::text_iarchive; T = std::optional<int>]’
/usr/include/boost/serialization/split_free.hpp:74:18:   required from ‘void boost::serialization::split_free(Archive&, T&, unsigned int) [with Archive = boost::archive::text_iarchive; T = std::optional<int>]’
/home/jonas/Workspace/gtsam/gtsam/base/std_optional_serialization.h:103:35:   required from ‘void boost::serialization::serialize(Archive&, std::optional<_Up>&, unsigned int) [with Archive = boost::archive::text_iarchive; T = int]’
/usr/include/boost/serialization/serialization.hpp:109:14:   required from ‘void boost::serialization::serialize_adl(Archive&, T&, unsigned int) [with Archive = boost::archive::text_iarchive; T = std::optional<int>]’
/usr/include/boost/archive/detail/iserializer.hpp:189:40:   [ skipping 3 instantiation contexts, use -ftemplate-backtrace-limit=0 to disable ]
/usr/include/boost/archive/detail/iserializer.hpp:463:22:   required from ‘static void boost::archive::detail::load_non_pointer_type<Archive>::invoke(Archive&, T&) [with T = std::optional<int>; Archive = boost::archive::text_iarchive]’
/usr/include/boost/archive/detail/iserializer.hpp:626:18:   required from ‘void boost::archive::load(Archive&, T&) [with Archive = text_iarchive; T = std::optional<int>]’
/usr/include/boost/archive/detail/common_iarchive.hpp:67:22:   required from ‘void boost::archive::detail::common_iarchive<Archive>::load_override(T&) [with T = std::optional<int>; Archive = boost::archive::text_iarchive]’
/usr/include/boost/archive/basic_text_iarchive.hpp:70:52:   required from ‘void boost::archive::basic_text_iarchive<Archive>::load_override(T&) [with T = std::optional<int>; Archive = boost::archive::text_iarchive]’
/usr/include/boost/archive/text_iarchive.hpp:82:52:   required from ‘void boost::archive::text_iarchive_impl<Archive>::load_override(T&) [with T = std::optional<int>; Archive = boost::archive::text_iarchive]’
/usr/include/boost/archive/detail/interface_iarchive.hpp:68:36:   required from ‘Archive& boost::archive::detail::interface_iarchive<Archive>::operator>>(T&) [with T = std::optional<int>; Archive = boost::archive::text_iarchive]’
/home/jonas/Workspace/gtsam/gtsam/base/tests/testStdOptionalSerialization.cpp:26:9:   required from here
/usr/include/c++/13.2.1/type_traits:3282:7: error: invalid use of incomplete type ‘struct boost::serialization::U’ [-fpermissive]
 3282 |     = __is_trivially_constructible(_Tp, __add_rval_ref_t<_Tp>);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/boost/serialization/shared_ptr_helper.hpp:45:16: note: forward declaration of ‘struct boost::serialization::U’
   45 |     SPT< class U > &t,
      |                ^
make[3]: *** [gtsam/base/tests/CMakeFiles/testStdOptionalSerialization.dir/build.make:76: gtsam/base/tests/CMakeFiles/testStdOptionalSerialization.dir/testStdOptionalSerialization.cpp.o] Error 1
make[2]: *** [CMakeFiles/Makefile2:4347: gtsam/base/tests/CMakeFiles/testStdOptionalSerialization.dir/all] Error 2
make[1]: *** [CMakeFiles/Makefile2:2488: CMakeFiles/check.dir/rule] Error 2
make: *** [Makefile:199: check] Error 2
CMake configuration ``` CMake Deprecation Warning at CMakeLists.txt:1 (cmake_minimum_required): Compatibility with CMake < 3.5 will be removed from a future version of CMake. Update the VERSION argument value or use a ... suffix to tell CMake that the project does not need compatibility with older versions. -- The CXX compiler identification is GNU 13.2.1 -- The C compiler identification is GNU 13.2.1 -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Check for working CXX compiler: /usr/bin/c++ - skipped -- Detecting CXX compile features -- Detecting CXX compile features - done -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Check for working C compiler: /usr/bin/cc - skipped -- Detecting C compile features -- Detecting C compile features - done -- GTSAM is a shared library due to GTSAM_FORCE_SHARED_LIB -- GTSAM_POSE3_EXPMAP=ON, enabling GTSAM_ROT3_EXPMAP as well -- Performing Test COMPILER_HAS_WSUGGEST_OVERRIDE -- Performing Test COMPILER_HAS_WSUGGEST_OVERRIDE - Success -- Performing Test COMPILER_HAS_WMISSING_OVERRIDE -- Performing Test COMPILER_HAS_WMISSING_OVERRIDE - Failed -- Found Boost: /usr/lib/cmake/Boost-1.83.0/BoostConfig.cmake (found suitable version "1.83.0", minimum required is "1.65") found components: serialization system filesystem thread program_options date_time timer chrono regex -- Found Eigen version: 3.4.0 CMake Deprecation Warning at gtsam/3rdparty/metis/CMakeLists.txt:1 (cmake_minimum_required): Compatibility with CMake < 3.5 will be removed from a future version of CMake. Update the VERSION argument value or use a ... suffix to tell CMake that the project does not need compatibility with older versions. -- Looking for execinfo.h -- Looking for execinfo.h - found -- Looking for getline -- Looking for getline - found -- checking for thread-local storage - found -- Could NOT find MKL (missing: MKL_LIBRARIES) -- Found OpenMP_C: -fopenmp (found version "4.5") -- Found OpenMP_CXX: -fopenmp (found version "4.5") -- Found OpenMP: TRUE (found version "4.5") -- Found Google perftools: -- Building 3rdparty -- Could NOT find GeographicLib (missing: GeographicLib_LIBRARY_DIRS GeographicLib_LIBRARIES GeographicLib_INCLUDE_DIRS) -- Building base -- Building basis -- Building geometry -- Building inference -- Building symbolic -- Building discrete -- Building hybrid -- Building linear -- Building nonlinear -- Building sam -- Building sfm -- Building slam -- Building navigation -- GTSAM Version: 4.3a0 -- Install prefix: /usr/local -- Building GTSAM - as a SHARED library -- Building base_unstable -- Building geometry_unstable -- Building linear_unstable -- Building discrete_unstable -- Building dynamics_unstable -- Building nonlinear_unstable -- Building slam_unstable -- Building partition_unstable -- GTSAM_UNSTABLE Version: 4.3a0 -- Install prefix: /usr/local -- Wrote /home/jonas/Workspace/gtsam/build_13/GTSAMConfig.cmake -- Wrote /home/jonas/Workspace/gtsam/build_13/GTSAM_UNSTABLEConfig.cmake -- Found Doxygen: /usr/bin/doxygen (found version "1.10.0") found components: doxygen dot -- =============================================================== -- ================ Configuration Options ====================== -- CMAKE_CXX_COMPILER_ID type : GNU -- CMAKE_CXX_COMPILER_VERSION : 13.2.1 -- CMake version : 3.28.3 -- CMake generator : Unix Makefiles -- CMake build tool : /usr/bin/make -- Build flags -- Build Tests : Enabled -- Build examples with 'make all' : Enabled -- Build timing scripts with 'make all' : Disabled -- Build Docs : Enabled -- Build shared GTSAM libraries : Enabled -- Put build type in library name : Enabled -- Build libgtsam_unstable : Enabled -- Build GTSAM unstable Python : Enabled -- Build MATLAB Toolbox for unstable : Disabled -- Build for native architecture : Disabled -- Build type : Release -- C compilation flags : -O3 -DNDEBUG -- C++ compilation flags : -O3 -DNDEBUG -- Enable Boost serialization : ON -- GTSAM_COMPILE_FEATURES_PUBLIC : cxx_std_17 -- GTSAM_COMPILE_OPTIONS_PUBLIC : -- GTSAM_COMPILE_DEFINITIONS_PUBLIC : -- GTSAM_COMPILE_OPTIONS_PUBLIC_RELEASE : -- GTSAM_COMPILE_DEFINITIONS_PUBLIC_RELEASE : -- Use System Eigen : OFF (Using version: 3.4.0) -- Use System Metis : OFF -- Using Boost version : 1.83.0 -- Use Intel TBB : Yes (Version: 2021.11.0) -- Eigen will use MKL : MKL not found -- Eigen will use MKL and OpenMP : OpenMP found but GTSAM_WITH_EIGEN_MKL is disabled -- Default allocator : TBB -- Cheirality exceptions enabled : YES -- Build with ccache : Yes -- Packaging flags -- CPack Source Generator : TGZ -- CPack Generator : TGZ -- GTSAM flags -- Quaternions as default Rot3 : Disabled -- Runtime consistency checking : Disabled -- Build with Memory Sanitizer : Disabled -- Rot3 retract is full ExpMap : Enabled -- Pose3 retract is full ExpMap : Enabled -- Enable branch merging in DecisionTree : Enabled -- Allow features deprecated in GTSAM 4.3 : Enabled -- Metis-based Nested Dissection : Enabled -- Use tangent-space preintegration : Enabled -- MATLAB toolbox flags -- Install MATLAB toolbox : Disabled -- Python toolbox flags -- Build Python module with pybind : Disabled -- =============================================================== -- Configuring done (4.5s) -- Generating done (2.8s) -- Build files have been written to: /home/jonas/Workspace/gtsam/build_13 ```

Expected behavior

Successful build

Environment

varunagrawal commented 5 months ago

Seems like GCC fixed a bug it had because we had a conditional compilation step in gtsam/base/serialization.h. The fix should be simple: check if the GCC version is >=7 and <13 for that block. Can you make a PR? I can help land it.

ottojo commented 5 months ago

I think this relates to the workaround in https://github.com/borglab/gtsam/blob/develop/gtsam/base/std_optional_serialization.h#L30 rather than serialization.h, since it seems to correlate with including <boost/serialization/shared_ptr.hpp>, as mentioned in the comment there.

From what i can tell, GCC still exhibits the bug described in the comment (trying to instantiate objects of type T<boost::serialization::U> when boost::serialization::U doesn't exist).

But GCC<13 accepted the incomplete type U in the specializations of the form

namespace boost { namespace serialization { struct U; } }
namespace std { template<> struct is_trivially_default_constructible<boost::serialization::U> : std::false_type {}; }

which GCC 13 does not anymore. It seems gcc rejects using incomplete types here since version 13, and clang since version 16, so perhaps it was erroneously accepted before.

The issue seems to disappear if U is fully declared

- namespace boost { namespace serialization { struct U; } }
+ namespace boost { namespace serialization { struct U{}; } }

but I'm unsure if this has any unintended side effects.

Interestingly, both GCC 12.3 and 13.2.1 don't actually seem to require the type traits, only the declaration/definition of U, but i haven't tested any older GCC versions...

varunagrawal commented 5 months ago

That's a great catch and analysis. I think you've found the core of the problem and described it very nicely (with a potential fix). I agree that it is uncertain what a full declaration of U will end up doing. I will need to look into this extensively, but I unfortunately don't have the time this week. I can get back to this after the 29th of March if that works?

ottojo commented 5 months ago

No rush at all, thank you for looking into the issue! I'm not depending on using GCC13 anytime soon, it just happened to be the default on my system.