Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix error: redefinition of ‘struct eigenpy::NumpyEquivalentType<... #488

Merged
merged 5 commits into from
Aug 13, 2024

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Jul 31, 2024

The ROS Noetic armhf build for eigenpy is failing - likely caused by #455, and it's causing regressions blocking the Noetic Sync. May I request a release to Noetic when this PR or another fix for the issue is merged?

This fixes the issue by using SFINAE and partial template specialization(as suggested here) to remove the specializations of NumpyEquivalentType<long long> and NumpyEquivalentType<unsigned long long> when they're the same as int64_t and uint64_t respectively.

https://build.ros.org/view/Nbin_ufhf_uFhf/job/Nbin_ufhf_uFhf__eigenpy__ubuntu_focal_armhf__binary/96

23:06:42 /usr/lib/ccache/c++  -Deigenpy_EXPORTS -I/tmp/binarydeb/ros-noetic-eigenpy-3.7.0/.obj-arm-linux-gnueabihf -I/tmp/binarydeb/ros-noetic-eigenpy-3.7.0/include -isystem /tmp/binarydeb/ros-noetic-eigenpy-3.7.0/.obj-arm-linux-gnueabihf/include -isystem /usr/include/eigen3 -isystem /usr/lib/python3/dist-packages/numpy/core/include -isystem /usr/include/python3.8  -Wno-long-long -Wall -Wextra -Wcast-align -Wcast-qual -Wformat -Wwrite-strings -Wconversion -g -O2 -fdebug-prefix-map=/tmp/binarydeb/ros-noetic-eigenpy-3.7.0=. -fstack-protector-strong -Wformat -Werror=format-security -DNDEBUG -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC   -Wno-conversion -std=gnu++14 -o CMakeFiles/eigenpy.dir/src/solvers/preconditioners.cpp.o -c /tmp/binarydeb/ros-noetic-eigenpy-3.7.0/src/solvers/preconditioners.cpp
23:06:50 In file included from /tmp/binarydeb/ros-noetic-eigenpy-3.7.0/include/eigenpy/fwd.hpp:87,
23:06:50                  from /tmp/binarydeb/ros-noetic-eigenpy-3.7.0/include/eigenpy/solvers/BasicPreconditioners.hpp:11,
23:06:50                  from /tmp/binarydeb/ros-noetic-eigenpy-3.7.0/src/solvers/preconditioners.cpp:8:
23:06:50 /tmp/binarydeb/ros-noetic-eigenpy-3.7.0/include/eigenpy/numpy.hpp:143:8: error: redefinition of ‘struct eigenpy::NumpyEquivalentType<long long int>’
23:06:50   143 | struct NumpyEquivalentType<long long> {
23:06:50       |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
23:06:50 /tmp/binarydeb/ros-noetic-eigenpy-3.7.0/include/eigenpy/numpy.hpp:115:8: note: previous definition of ‘struct eigenpy::NumpyEquivalentType<long long int>’
23:06:50   115 | struct NumpyEquivalentType<int64_t> {
23:06:50       |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
23:06:50 /tmp/binarydeb/ros-noetic-eigenpy-3.7.0/include/eigenpy/numpy.hpp:147:8: error: redefinition of ‘struct eigenpy::NumpyEquivalentType<long long unsigned int>’
23:06:50   147 | struct NumpyEquivalentType<unsigned long long> {
23:06:50       |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
23:06:50 /tmp/binarydeb/ros-noetic-eigenpy-3.7.0/include/eigenpy/numpy.hpp:119:8: note: previous definition of ‘struct eigenpy::NumpyEquivalentType<long long unsigned int>’
23:06:50   119 | struct NumpyEquivalentType<uint64_t> {
23:06:50       |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
23:06:50 make[4]: *** [CMakeFiles/eigenpy.dir/build.make:66: CMakeFiles/eigenpy.dir/src/solvers/preconditioners.cpp.o] Error 1

I tested that eigenpy builds with this PR on an amd64 machine, but that of course won't show the build failure. Instead I made sure the method for the fix works using godbolt.org with the 32bit ARM GCC 9.3.0 compiler option with the code below:

#include <cstdint>
#include <type_traits>

template<typename S, typename Enable = void>
struct FoobarType{};

template<>
struct FoobarType<int64_t>{};

template<typename S>
struct FoobarType<S, 
std::enable_if_t<
!std::is_same<int64_t, long long>::value &&
std::is_same<S, long long>::value>>{};

template<>
struct FoobarType<uint64_t>{};

template<typename S>
struct FoobarType<S, 
std::enable_if_t<
!std::is_same<uint64_t, unsigned long long>::value &&
std::is_same<S, unsigned long long>::value>>{};

int main() {
    FoobarType<long long> bar;
    FoobarType<int64_t> baz;

    FoobarType<unsigned long long> ubar;
    FoobarType<uint64_t> ubaz;
}

@sloretz
Copy link
Contributor Author

sloretz commented Aug 7, 2024

@wxmerkt Mind reviewing this one? This bug is causing regressions in Noetic on armhf, and that's blocking the Noetic sync.

wxmerkt
wxmerkt previously approved these changes Aug 7, 2024
Copy link
Member

@wxmerkt wxmerkt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for putting this together and the ping. Looks good - let's wait for CI to pass

wxmerkt
wxmerkt previously approved these changes Aug 7, 2024
Copy link
Member

@wxmerkt wxmerkt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good from my end. @jcarpent is this good to merge?

@jcarpent
Copy link
Contributor

Thanks a lot, @sloretz, for providing this fix, and thanks a lot, @wxmerkt, for reviewing this PR carefully.
I will provide a new release soon.

@jcarpent jcarpent merged commit e93e1a7 into stack-of-tasks:devel Aug 13, 2024
35 checks passed
@sloretz sloretz deleted the sfinae_NumpyEquivalentType branch August 14, 2024 18:18
nim65s added a commit to nim65s/robotpkg that referenced this pull request Oct 8, 2024
    ## [3.8.2] - 2024-08-26

    ### Fixed
    - Fix function signature on Windows (stack-of-tasks/eigenpy#494)

    ## [3.8.1] - 2024-08-25

    ### Fixed
    - Fix compatibility issue with NumPy 2.x on Windows (stack-of-tasks/eigenpy#492)

    ## [3.8.0] - 2024-08-14

    ### Added
    - Add compatibility with jrl-cmakemodules workspace (stack-of-tasks/eigenpy#485)
    - Remove support of Python 3.7 (stack-of-tasks/eigenpy#490)

    ### Fixed
    - Remove CMake CMP0167 warnings (stack-of-tasks/eigenpy#487)
    - Fix compilation error on armhf (stack-of-tasks/eigenpy#488)
nim65s added a commit to nim65s/robotpkg that referenced this pull request Oct 9, 2024
    ## [3.10.0] - 2024-09-26

    ### Added

    - `GenericMapPythonVisitor`/`StdMapPythonVisitor` can now take an extra visitor argument in the `expose()` method, similar to `StdVectorPythonVisitor`

    ### Changed

    - Move `GenericMapPythonVisitor` to its own header `eigenpy/map.hpp`
    - Rename `overload_base_get_item_for_std_map` to `overload_base_get_item_for_map`, move out of `eigenpy::details` namespace
    - Move `EmptyPythonVisitor` to new header `eigenpy/utils/empty-visitor.hpp`

    ## [3.9.1] - 2024-09-19

    ### Added

    - Add test returning reference of std::pair stack-of-tasks/eigenpy#503
    - Add more general visitor `GenericMapPythonVisitor` for map types test `boost::unordered_map<std::string, int>` stack-of-tasks/eigenpy#504
    - Support for non-[default-contructible](https://en.cppreference.com/w/cpp/named_req/DefaultConstructible) types in map types stack-of-tasks/eigenpy#504
    - Add type_info helpers stack-of-tasks/eigenpy#502
    - Add NumPy 2 support stack-of-tasks/eigenpy#496

    ### Changed

    - Move `StdMapPythonVisitor` out of `eigenpy::python` namespace, which was a mistake stack-of-tasks/eigenpy#504

    ## [3.9.0] - 2024-08-31

    ### Changed
    - The `exposeStdVectorEigenSpecificType()` template function now takes the vector allocator as a template parameter stack-of-tasks/eigenpy#500

    ### Added
    - Add bp::dist to std::map converter stack-of-tasks/eigenpy#499

    ## [3.8.2] - 2024-08-26

    ### Fixed
    - Fix function signature on Windows stack-of-tasks/eigenpy#494

    ## [3.8.1] - 2024-08-25

    ### Fixed
    - Fix compatibility issue with NumPy 2.x on Windows stack-of-tasks/eigenpy#492

    ## [3.8.0] - 2024-08-14

    ### Added
    - Add compatibility with jrl-cmakemodules workspace stack-of-tasks/eigenpy#485
    - Remove support of Python 3.7 stack-of-tasks/eigenpy#490

    ### Fixed
    - Remove CMake CMP0167 warnings stack-of-tasks/eigenpy#487
    - Fix compilation error on armhf stack-of-tasks/eigenpy#488
nim65s added a commit to nim65s/robotpkg that referenced this pull request Oct 11, 2024
    ## [3.10.0] - 2024-09-26

    ### Added

    - `GenericMapPythonVisitor`/`StdMapPythonVisitor` can now take an extra visitor argument in the `expose()` method, similar to `StdVectorPythonVisitor`

    ### Changed

    - Move `GenericMapPythonVisitor` to its own header `eigenpy/map.hpp`
    - Rename `overload_base_get_item_for_std_map` to `overload_base_get_item_for_map`, move out of `eigenpy::details` namespace
    - Move `EmptyPythonVisitor` to new header `eigenpy/utils/empty-visitor.hpp`

    ## [3.9.1] - 2024-09-19

    ### Added

    - Add test returning reference of std::pair stack-of-tasks/eigenpy#503
    - Add more general visitor `GenericMapPythonVisitor` for map types test `boost::unordered_map<std::string, int>` stack-of-tasks/eigenpy#504
    - Support for non-[default-contructible](https://en.cppreference.com/w/cpp/named_req/DefaultConstructible) types in map types stack-of-tasks/eigenpy#504
    - Add type_info helpers stack-of-tasks/eigenpy#502
    - Add NumPy 2 support stack-of-tasks/eigenpy#496

    ### Changed

    - Move `StdMapPythonVisitor` out of `eigenpy::python` namespace, which was a mistake stack-of-tasks/eigenpy#504

    ## [3.9.0] - 2024-08-31

    ### Changed
    - The `exposeStdVectorEigenSpecificType()` template function now takes the vector allocator as a template parameter stack-of-tasks/eigenpy#500

    ### Added
    - Add bp::dist to std::map converter stack-of-tasks/eigenpy#499

    ## [3.8.2] - 2024-08-26

    ### Fixed
    - Fix function signature on Windows stack-of-tasks/eigenpy#494

    ## [3.8.1] - 2024-08-25

    ### Fixed
    - Fix compatibility issue with NumPy 2.x on Windows stack-of-tasks/eigenpy#492

    ## [3.8.0] - 2024-08-14

    ### Added
    - Add compatibility with jrl-cmakemodules workspace stack-of-tasks/eigenpy#485
    - Remove support of Python 3.7 stack-of-tasks/eigenpy#490

    ### Fixed
    - Remove CMake CMP0167 warnings stack-of-tasks/eigenpy#487
    - Fix compilation error on armhf stack-of-tasks/eigenpy#488
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants