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

Macro expansion leads to a bare expression #6614

Merged
merged 1 commit into from Apr 19, 2018

Conversation

Projects
None yet
5 participants
@pauluap

pauluap commented Apr 11, 2018

Description

Compile: lwip_stack.c
In file included from ../features/FEATURE_LWIP/lwip-interface/lwip_stack.c:41:0:
../features/FEATURE_LWIP/lwip-interface/lwip_stack.c: In function 'mbed_lwip_bringup_2':
../features/FEATURE_LWIP/lwip-interface/ppp_lwip.h:58:44: warning: statement with no effect [-Wunused-value]
 #define ppp_lwip_disconnect()              ERR_IF
                                            ^
../features/FEATURE_LWIP/lwip-interface/lwip_stack.c:858:21: note: in expansion of macro 'ppp_lwip_disconnect'
                     ppp_lwip_disconnect();
                     ^~~~~~~~~~~~~~~~~~~
../features/FEATURE_LWIP/lwip-interface/ppp_lwip.h:58:44: warning: statement with no effect [-Wunused-value]
 #define ppp_lwip_disconnect()              ERR_IF
                                            ^
../features/FEATURE_LWIP/lwip-interface/lwip_stack.c:875:21: note: in expansion of macro 'ppp_lwip_disconnect'
                     ppp_lwip_disconnect();

                     ^~~~~~~~~~~~~~~~~~~

If the implementation used functions, a function returning a bare value wouldn't be a problem, but here the 'return' is done by macro expansion. Since the value isn't assigned to anything, it turns out to be a bare expression, thus the warning message.

Cast to void to ignore, technique distilled from https://stackoverflow.com/questions/777261/avoiding-unused-variables-warnings-when-using-assert-in-a-release-build

Pull request type

[X] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 12, 2018

@pauluap Thanks for the contributions, I'll review them today.

Please use PR type, use X to select what PR type is it to clarify the intent in your commits

Second, use master as destination branch, not mbed-os-5.8 (we apply patches when release is prepared).

@0xc0170 0xc0170 requested a review from kjbracey-arm Apr 12, 2018

Paul Thompson
Explicitly ignore return value or bare expression (macro expands to a…
… number)

Compile: lwip_stack.c
In file included from ../features/FEATURE_LWIP/lwip-interface/lwip_stack.c:41:0:
../features/FEATURE_LWIP/lwip-interface/lwip_stack.c: In function 'mbed_lwip_bringup_2':
../features/FEATURE_LWIP/lwip-interface/ppp_lwip.h:58:44: warning: statement with no effect [-Wunused-value]
 #define ppp_lwip_disconnect()              ERR_IF
                                            ^
../features/FEATURE_LWIP/lwip-interface/lwip_stack.c:858:21: note: in expansion of macro 'ppp_lwip_disconnect'
                     ppp_lwip_disconnect();
                     ^~~~~~~~~~~~~~~~~~~
../features/FEATURE_LWIP/lwip-interface/ppp_lwip.h:58:44: warning: statement with no effect [-Wunused-value]
 #define ppp_lwip_disconnect()              ERR_IF
                                            ^
../features/FEATURE_LWIP/lwip-interface/lwip_stack.c:875:21: note: in expansion of macro 'ppp_lwip_disconnect'
                     ppp_lwip_disconnect();

                     ^~~~~~~~~~~~~~~~~~~

@pauluap pauluap force-pushed the pauluap:compiler_warning_macro_expression branch from c3d5063 to 449541c Apr 12, 2018

@pauluap pauluap changed the base branch from mbed-os-5.8 to master Apr 12, 2018

@pauluap

This comment has been minimized.

pauluap commented Apr 12, 2018

I'll definitely use master as the merge target in the future.

I did mark the PR as a fix. Does it need to be at the top of the PR or something?

@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented Apr 13, 2018

Equivalent changes are already on the feature-emac branch. No objection to having this on master now though - will get overwritten in merge.

@0xc0170 0xc0170 removed the do not merge label Apr 13, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 13, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Apr 13, 2018

Build : SUCCESS

Build number : 1737
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6614/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 16, 2018

/morph mbed2-build

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 16, 2018

Travis run, but did not report status back. I restarted it https://travis-ci.org/ARMmbed/mbed-os/builds/365358118 but might not still report it back :( We can try close/open

@cmonr cmonr added needs: CI and removed needs: review labels Apr 17, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Apr 17, 2018

sigh

Going to close and reopen this to see if we can't get travis-ci to report it's success.

@cmonr cmonr closed this Apr 17, 2018

@cmonr cmonr reopened this Apr 17, 2018

@cmonr cmonr removed the needs: CI label Apr 17, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Apr 17, 2018

Interesting.
If I had to bet, I'd say thing will work this time around.

@0xc0170 @pauluap It looks like restarting the ci build wasn't working because the destination branch inside of travis somehow didn't get the message that this PR was change from mbed-os-5.8 to master.
Good to know for the future.

@mbed-ci

This comment has been minimized.

mbed-ci commented Apr 17, 2018

Build : SUCCESS

Build number : 1777
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6614/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci

This comment has been minimized.

1 similar comment
@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 18, 2018

Pipe closed, restarting exporters

/morph export-build

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Apr 19, 2018

@0xc0170 0xc0170 merged commit a37ba4b into ARMmbed:master Apr 19, 2018

12 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 9984 cycles (+411 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/littlefs Passed, code size is 10092B (+0.00%)
Details
travis-ci/tools Local tools testing has passed
Details

@pauluap pauluap deleted the pauluap:compiler_warning_macro_expression branch Apr 19, 2018

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