Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

fix warnings #40

Merged
merged 8 commits into from
Jun 16, 2015
Merged

fix warnings #40

merged 8 commits into from
Jun 16, 2015

Conversation

wjwwood
Copy link
Member

@wjwwood wjwwood commented Jun 2, 2015

Fixes some warnings (hopefully) about unknown pragmas and arguments. See:

ee775b8#commitcomment-11295655

Connects to ros2/ros2#53

@wjwwood wjwwood added the in progress Actively being worked on (Kanban column) label Jun 2, 2015
@wjwwood wjwwood force-pushed the wjwwood_warnings_cleanup branch 2 times, most recently from 52f25b6 to 532132f Compare June 2, 2015 06:53
"-Wno-return-type-c-linkage "
"-Wno-sometimes-uninitialized "
"-Wno-tautological-compare "
"-Wno-unused-variable "
Copy link
Member

Choose a reason for hiding this comment

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

The string values should not have a trailing space inside. Same below for the GNU flags.

Copy link
Member Author

Choose a reason for hiding this comment

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

When I did this it would pass the flags to gcc all concatenated together. See (search for unrecognized command line option):

http://54.183.26.131:8080/job/ros2_batch_ci_linux/124/consoleFull

cc1plus: warning: unrecognized command line option "-Wno-tautological-compare-Wno-return-type-c-linkage-Wno-deprecated-register" [enabled by default]

Copy link
Member

Choose a reason for hiding this comment

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

The set command creates a list here. This implies that the items are separated by a semicolon. Therefore each item should not have a trailing space.

The additional problem here is that the replace call in line 79 has a bug and does not what it is supposed to do, namely replace these semicolons with space. That is because the value of the replace call is not quoted. It should be:

string(REPLACE ";" " " _connext_compile_flags "${_connext_compile_flags}")

@dirk-thomas
Copy link
Member

This will also suppress warnings in the code the rosidl_typesupport_connext_cpp generates (which is not being generated by Connext's idlpp). Is there a better way to maintain the warnings in "our" code but suppress the warnings in Connext code?

@wjwwood
Copy link
Member Author

wjwwood commented Jun 2, 2015

You can put #pragma GCC diagnostic ignore "..." around our code where we include their headers. We can also be more selective on which source files we add these flags to.

@dirk-thomas
Copy link
Member

Sounds good. I can update the CMake block after we merged the refactoring PR and only apply the properties for the generated files coming from the Connext code generator.

wjwwood added a commit that referenced this pull request Jun 16, 2015
@wjwwood wjwwood merged commit 5aac5b0 into master Jun 16, 2015
@wjwwood wjwwood removed the in progress Actively being worked on (Kanban column) label Jun 16, 2015
@wjwwood wjwwood deleted the wjwwood_warnings_cleanup branch June 16, 2015 20:04
@dirk-thomas dirk-thomas mentioned this pull request Jun 16, 2015
3 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants