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.

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

Suppress unused variables warnings in release mode #646

Merged
merged 1 commit into from
Mar 28, 2016

Conversation

mkoval
Copy link
Collaborator

@mkoval mkoval commented Mar 27, 2016

Building DART in release mode disables assertions. There are a few parameters in DART that are only used in assertions, so this generates -Wunused-parameter warnings. This pull request adds a DART_UNUSED that suppresses those warnings. See this StackOverflow answer for more explanation.

@jslee02 jslee02 added this to the DART 6.0.0 milestone Mar 27, 2016
@jslee02
Copy link
Member

jslee02 commented Mar 27, 2016

👍

@jslee02 jslee02 merged commit a309306 into dartsim:master Mar 28, 2016
@psigen
Copy link
Collaborator

psigen commented Mar 28, 2016

It seems a bit strange to require arguments that are only used in assertions.

Are these parameters required as part of some standardized spec? Is there a reason we force them to be passed when they are apparently not strictly necessary for the function?

@jslee02
Copy link
Member

jslee02 commented Mar 28, 2016

This is a reasonable concern.

In DartTNLP case, we should follow the same function prototype as it overrides Ipopt::TNLP functions. We could comment out, but I would like to leave _m uncommented to be used in the assertion for debugging purpose (checking the dimensionality).

We could remove _numDir for ODELCPSolver case. One thing we should note is that this class isn't used anywhere (at least in our code base) so I would be fine to remove _numDir. Personally, I'm a bit inclined to leave it until we decide whether remove this unused class ODELCPSolver or improve to use somewhere. I think at some point we need to refactor the constraint solving and can figure out what we will do with ODELCPSolver at that moment.

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