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.

Sign up for GitHub

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

Jump to bottom

Work around VS2015's broken support for expression SFINAE #122

Merged
merged 4 commits into from
Oct 13, 2015
Merged

Work around VS2015's broken support for expression SFINAE #122

merged 4 commits into from
Oct 13, 2015

Conversation

esteve
Copy link
Member

@esteve esteve commented Oct 12, 2015

This works around VS2015's broken support for expression SFINAE and adds tests for function_traits

@esteve esteve added the in progress Actively being worked on (Kanban column) label Oct 12, 2015
@esteve
Copy link
Member Author

esteve commented Oct 12, 2015

CI job for Windows:

http://ci.ros2.org/job/ros2_batch_ci_windows/472/

This is the testsuite for function_traits:

http://ci.ros2.org/job/ros2_batch_ci_windows/472/testReport/%28root%29/TestFunctionTraits/

@esteve esteve added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Oct 12, 2015
@esteve
Copy link
Member Author

esteve commented Oct 12, 2015

This basically means that we can get rid of static_assert and get SFINAE working for functions that accept callbacks with arbitrary number of arguments and check the types of the callbacks' arguments.

dirk-thomas
dirk-thomas reviewed Oct 12, 2015
View reviewed changes
rclcpp/test/test_function_traits.cpp

int func_one_int(int x)
{
(void)x;
Copy link
Member

Choose a reason for hiding this comment

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

If we never intend to document the function we could remove the variable name from the signature and then also this line.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@dirk-thomas
Copy link
Member

Nice 👍

@wjwwood
Copy link
Member

wjwwood commented Oct 12, 2015

lgtm (I don't claim to understand it), but the tests look very thorough.

@esteve
Copy link
Member Author

esteve commented Oct 12, 2015

I noticed that I hadn't pushed the changes to any_subscription_callback.hpp

CI jobs for Windows:

http://ci.ros2.org/job/ros2_batch_ci_windows/473/

@esteve esteve self-assigned this Oct 12, 2015
@esteve esteve mentioned this pull request Oct 12, 2015
Merged
@esteve
Copy link
Member Author

esteve commented Oct 12, 2015

My latest change broke the build, working on it.

@esteve
Copy link
Member Author

esteve commented Oct 12, 2015

Errors fixed. New CI job:

http://ci.ros2.org/job/ros2_batch_ci_windows/474/

@esteve
Copy link
Member Author

esteve commented Oct 13, 2015

Moving back to in progress, there's an error in any_subscription_callback.hpp

@esteve esteve added in progress Actively being worked on (Kanban column) and removed in review Waiting for review (Kanban column) labels Oct 13, 2015
@esteve esteve added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Oct 13, 2015
@esteve
Copy link
Member Author

esteve commented Oct 13, 2015

CI job builds and passes:

http://ci.ros2.org/job/ros2_batch_ci_windows/477/

@esteve
Copy link
Member Author

esteve commented Oct 13, 2015

@wjwwood I'm afraid neither will I after this gets merged 😄 I've added a few comments on how this write-only code works.

@esteve
Copy link
Member Author

esteve commented Oct 13, 2015

New CI job after #119 got merged:

http://ci.ros2.org/job/ros2_batch_ci_windows/479/

esteve added a commit that referenced this pull request Oct 13, 2015
@esteve
Merge pull request #122 from ros2/expression-sfinae-windows
ae682ba 
Work around VS2015's broken support for expression SFINAE
@esteve esteve merged commit ae682ba into master Oct 13, 2015
@esteve esteve removed the in review Waiting for review (Kanban column) label Oct 13, 2015
@esteve esteve deleted the expression-sfinae-windows branch October 13, 2015 21:28
@dirk-thomas dirk-thomas mentioned this pull request Dec 9, 2016
Closed
nnmm pushed a commit to ApexAI/rclcpp that referenced this pull request Jul 9, 2022
@wjwwood
move allocator and error handling to c utilities (ros2#122)
90176d9 
* moved allocator to c_utilities

* moved error_handling to c_utilities

* refactor uses of RCL_SET_ERROR_MSG with allocator

* add missing guard condition functions and tests

* add missing timer functions

* refactor rcl_lifecycyle

* missed an instance of RCL_SET_ERROR_MSG

* fix segfaults in error cases for rcl_lifecycle

* remove extra header

* check return code of rcl_lifecycle_init_default_state_machine
DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this pull request Aug 5, 2022
@Karsten1987
remove disclaimer (ros2#122)
49b1255 
Signed-off-by: Karsten Knese <[email protected]>
mauropasse pushed a commit to mauropasse/rclcpp that referenced this pull request Nov 1, 2023
@alsora
Merge pull request ros2#122 from mauropasse/mauro/humble-fix-data-rac…
679ab77 
…e-on-params-event

Humble: Fix data race on parameter event
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@esteve esteve

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@esteve @dirk-thomas @wjwwood