-
Notifications
You must be signed in to change notification settings - Fork 421
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
Removed test_timers_manager clang warning #2479
Removed test_timers_manager clang warning #2479
Conversation
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
@@ -375,7 +375,7 @@ TEST_F(TestTimersManager, check_one_timer_cancel_doesnt_affect_other_timers) | |||
// since typical users aren't going to mess around with the timer manager. | |||
t1 = TimerT::make_shared( | |||
1ms, | |||
[&t1_runs, &t1, cancel_iter]() { | |||
[&t1_runs, &t1]() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose I knew this, but had to look it up again.
https://en.cppreference.com/w/cpp/language/lambda
A lambda expression can read the value of a variable without capturing it if the variable has const non-volatile integral or enumeration type and has been initialized with a constant expression
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had no idea :). Today I learned something :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MSVC doesn't seem to like it, though:
**09:25:11 C:\ci\ws\src\ros2\rclcpp\rclcpp\test\rclcpp\test_timers_manager.cpp(380,1): error C3493: 'cancel_iter' cannot be implicitly captured because no default capture mode has been specified [C:\ci\ws\build\rclcpp\test\rclcpp\test_timers_manager.vcxproj]
**
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we set the default capture mode as it says?
[&t1_runs, &t1]() { | |
[=, &t1_runs, &t1]() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good with green CI.
@@ -375,7 +375,7 @@ TEST_F(TestTimersManager, check_one_timer_cancel_doesnt_affect_other_timers) | |||
// since typical users aren't going to mess around with the timer manager. | |||
t1 = TimerT::make_shared( | |||
1ms, | |||
[&t1_runs, &t1, cancel_iter]() { | |||
[&t1_runs, &t1]() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had no idea :). Today I learned something :).
Windows doesn't like the change
|
See our conversation in #2479 (comment) |
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
This specific warning is removed from the clang builds, Merging |
Related with this warning https://ci.ros2.org/view/nightly/job/nightly_linux_clang_libcxx/1846/gcc/new/folder.-1481343024/