Skip to content

Conversation

@jwang11
Copy link

@jwang11 jwang11 commented Oct 9, 2017

The commit use classic c preprocessor approch to handle variadic parameter. By define
RMW interface implementation and handle ARG values inside RMW_INTERFACE_FN macro,
it obviously reduce code lines and complexity of rmw interface implementation.

The commit use classic c preprocessor approch to handle variadic parameter, which define
RMW interface implementation and handle ARG values inside RMW_INTERFACE_FN macro.
It obviously reduce code lines and complexity of rmw interface implementation.
@mikaelarguedas mikaelarguedas added the in review Waiting for review (Kanban column) label Oct 11, 2017
@dirk-thomas
Copy link
Member

Thank you for the patch. That indeed significantly decreased the code size.

I applied some minor changes on top in c685508. I created #29 and triggered some CI builds. It looks like the code doesn't compile on Windows as-is. Could you have a look on how it can be made to work on Windows since we need to support all platforms?

@dirk-thomas dirk-thomas added in progress Actively being worked on (Kanban column) and removed in review Waiting for review (Kanban column) labels Oct 12, 2017
@jwang11
Copy link
Author

jwang11 commented Oct 12, 2017

The reason for Windows compile failure lie in the MACRO parse order in a MACRO function and MACRO paramater, like MACRO_FUNC(MACRO_PARAM). In VS, it parse MACRO_FUNC first, but GCC parse MACRO_PARAM firstly. It can be easily resolved #define EXPAND(x) x, which cater both VS and GCC.

@dirk-thomas
Copy link
Member

Can you commit a patch to this branch? I can then cherry-pick it into the other PR.

The compile issue is caused by different MACRO parse order between VS and GCC.
VS first parse macro function, but GCC take macro parameter priority. Use a empty
EXPAND macro to cater both of them.
@mikaelarguedas
Copy link
Member

closing this since #29 has been merged.
Thanks @jwang11 for the contribution and for iterating on this!

@mikaelarguedas mikaelarguedas removed the in progress Actively being worked on (Kanban column) label Oct 16, 2017
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