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

New class for return values of interface methods PART2 #3167

Merged
merged 7 commits into from
Jan 29, 2025

Conversation

randaz81
Copy link
Member

sequel of #3051.
All navigation interfaces/devices have been updated to use yarp::dev::ReturnValue

@randaz81 randaz81 added this to the YARP 3.11.0 milestone Jan 23, 2025
@randaz81 randaz81 self-assigned this Jan 23, 2025
@randaz81 randaz81 requested a review from elandini84 January 23, 2025 15:04
corresponding .cpp and .h files regenerated
Localization2D_nws_yarp
Localization2D_nwc_yarp
Map2D_nws_yarp
Map2D_nwc_yarp
MobileBaseVelocityControl_nws_yarp
MobileBaseVelocityControl_nwc_yarp
Navigation2D_nwc_yarp
Navigation2D_nws_yarp
Odometry2D_nws_yarp
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
44.6% Coverage on New Code (required ≥ 80%)
7.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@randaz81 randaz81 merged commit 331d2e0 into robotology:master Jan 29, 2025
70 of 73 checks passed
@randaz81 randaz81 deleted the returnValue_nav branch January 29, 2025 10:06
@PeterBowman
Copy link
Member

Hello, @randaz81. After the merge, I am getting build errors on Ubuntu 22.04 + Clang 14.0.0 (CI logs):

[ 33%] Building CXX object src/libYARP_dev/src/CMakeFiles/YARP_dev.dir/yarp/dev/ISpeechSynthesizer.cpp.o
In file included from /home/runner/work/tools/tools/.deps/yarp/src/libYARP_dev/src/yarp/dev/ISpeechSynthesizer.cpp:6:
In file included from /home/runner/work/tools/tools/.deps/yarp/src/libYARP_dev/src/yarp/dev/ISpeechSynthesizer.h:11:
/home/runner/work/tools/tools/.deps/yarp/src/libYARP_dev/src/yarp/dev/ReturnValue.h:76:63: error: no type named 'source_location' in namespace 'std'
inline ReturnValue YARP_METHOD_NOT_YET_IMPLEMENTED(const std::source_location& location = std::source_location::current())
                                                         ~~~~~^
/home/runner/work/tools/tools/.deps/yarp/src/libYARP_dev/src/yarp/dev/ReturnValue.h:76:96: error: no member named 'source_location' in namespace 'std'
inline ReturnValue YARP_METHOD_NOT_YET_IMPLEMENTED(const std::source_location& location = std::source_location::current())
                                                                                          ~~~~~^
/home/runner/work/tools/tools/.deps/yarp/src/libYARP_dev/src/yarp/dev/ReturnValue.h:81:54: error: no type named 'source_location' in namespace 'std'
inline ReturnValue YARP_METHOD_DEPRECATED(const std::source_location& location = std::source_location::current())
                                                ~~~~~^
/home/runner/work/tools/tools/.deps/yarp/src/libYARP_dev/src/yarp/dev/ReturnValue.h:81:87: error: no member named 'source_location' in namespace 'std'
inline ReturnValue YARP_METHOD_DEPRECATED(const std::source_location& location = std::source_location::current())
                                                                                 ~~~~~^
4 errors generated.
gmake[2]: *** [src/libYARP_dev/src/CMakeFiles/YARP_dev.dir/build.make:577: src/libYARP_dev/src/CMakeFiles/YARP_dev.dir/yarp/dev/ISpeechSynthesizer.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:4888: src/libYARP_dev/src/CMakeFiles/YARP_dev.dir/all] Error 2
gmake: *** [Makefile:171: all] Error 2
Error: Process completed with exit code 2.

Basically: no type named 'source_location' in namespace 'std'. According to cppreference, this is a C++20 feature.

@randaz81
Copy link
Member Author

randaz81 commented Jan 29, 2025

Here is the issue: clang 14 is supposed to support c++20 (in fact this is checked by yarp code), but not all its features.
While we consider if this is an issue to manage (or update minimum requirements), I recommend to switch to clang 17, which is successfully tested on ubuntu 22.04 by yarp CI.

Btw, the standard gcc compiler (v11) shipped with Ubuntu 22.04 is not affected by this issue.

@PeterBowman
Copy link
Member

Oops, YARP indeed requires C++20 at least since v3.9 (I thought it was still at C++17): 8f45be9. Upgrading compilers seems justified.

By the way, @randaz81, shouldn't target_compiler_features() be updated as well to point at cxx_std_20? See search results. Currently, C++20 is required to compile YARP, but dependent projects only need C++17 due to this.

@randaz81
Copy link
Member Author

By the way, @randaz81, shouldn't target_compiler_features() be updated as well to point at cxx_std_20? See search results. Currently, C++20 is required to compile YARP, but dependent projects only need C++17 due to this.

Thank you! That piece of the code was indeed forgotten and not updated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants