ros2 / rmw_iceoryx

rmw implementation for iceoryx
Apache License 2.0
153 stars 27 forks source link

c++ 17 for Windows (msvc)? #93

Closed felixf4xu closed 1 year ago

felixf4xu commented 1 year ago

Hi,

I'm trying to build rmw_iceoryx on Win11 with Visual studio 2022 and I had a compiling error.

C:/ros/humble/install/include/iceoryx/v2.0.3\iceoryx_hoofs/cxx/type_traits.hpp(63,28): error C2039: 'invoke_result': is not a member of 'std' 
(compiling source file C:\ros\humble\src\ros2\rmw_iceoryx\rmw_iceoryx_cpp\src\rmw_init.cpp) 
[C:\ros\humble\src\ros2\rmw_iceoryx\rmw_iceoryx_cpp\build\rmw_iceoryx_cpp\rmw_iceoryx_cpp.vcxproj]

the code is actually from iceoryx itself but it's included in rmw_init.cpp. The source code type_traits.hpp(63,28) is:

// windows defines __cplusplus as 199711L
#if __cplusplus < 201703L && !defined(_WIN32)
template <typename C, typename... Cargs>
using invoke_result = std::result_of<C(Cargs...)>;
#elif __cplusplus >= 201703L || defined(_WIN32)
template <typename C, typename... Cargs>
using invoke_result = std::invoke_result<C, Cargs...>;
#endif

In my case, my __cplusplus is greater than 201703L and so std::invoke_result is used.

But std::invoke_result is a feature in c++ 17.

The current project is c++ 14 as in cmake https://github.com/ros2/rmw_iceoryx/blob/humble/rmw_iceoryx_cpp/CMakeLists.txt#LL9C1-L12C8:

# Default to C++14
if(NOT CMAKE_CXX_STANDARD)
  set(CMAKE_CXX_STANDARD 14)
endif()

the code above is flexible and I can set CMAKE_CXX_STANDARD.

Then I checked the source code of iceoryx itself,

the older version (v2.0.3 as in ros2 humble) of code is :

if(LINUX)
    set(ICEORYX_CXX_STANDARD 14)
elseif(QNX)
    set(ICEORYX_CXX_STANDARD 14)
elseif(WIN32)
    set(ICEORYX_CXX_STANDARD 17)
elseif(APPLE)
    set(ICEORYX_CXX_STANDARD 17)
elseif(UNIX)
   set(ICEORYX_CXX_STANDARD 17)
else()
   set(ICEORYX_CXX_STANDARD 17)
endif()

and the latest code is moved to https://github.com/eclipse-iceoryx/iceoryx/blob/master/iceoryx_platform/win/IceoryxPlatformSettings.cmake#L18 and is:

set_global(VAR ICEORYX_PLATFORM_STRING      VALUE "Windows")
set_global(VAR ICEORYX_CXX_STANDARD         VALUE 17)

So, my question is: does it make sense to set c++ 17 in windows build just like in iceoryx?

mossmaurice commented 1 year ago

@felixf4xu Thanks for reporting this issue!

Before we start having a closer look one question: Which branches/tags did you use from rmw_iceoryx and iceoryx? humble and v2.0.3, right?

So, my question is: does it make sense to set c++ 17 in windows build just like in iceoryx?

I think that makes a lot of sense. We should use the same C++ standard for both rmw_iceoryx and iceoryx. Did you check if it builds correctly with MSVC if you set CMAKE_CXX_STANDARD to 17 in rmw_iceoryx?

felixf4xu commented 1 year ago

During my test, rmw_iceoryx is at branch humble; iceoryx is at tag of v2.0.3 (default tag in ros2 humble).

If I set CMAKE_CXX_STANDARD to 17, this issue is fixed. (there is some other issue, though, I am still investigating into it)

mossmaurice commented 1 year ago

If I set CMAKE_CXX_STANDARD to 17, this issue is fixed.

Great, I will create a PR for that.