ethz-asl / programming_guidelines

This repository contains style-guides, discussions, eclipse/emacs auto-formatter for commonly used programming languages
139 stars 38 forks source link

Unused function parameters - declare (how) or just disable warning? #17

Closed HannesSommer closed 9 years ago

HannesSommer commented 10 years ago

I currently wander what to do with unused parameters. Especially when it comes to overriding they are clearly unavoidable. But the compiler warning seem quit e useful (enabled implicitly with -Wextra)

With gcc there seem to be three options :

  1. have no name for the unused parameters Contra: bad when you want to start using it, maybe irritating when reading Pro: no macro needed (to create compiler independent code)
  2. declare it as unused (see http://stackoverflow.com/questions/3599160/unused-parameter-warnings-in-c-code for ideas) Contra: need macro and that macro needs a place (header file ...) Pro: can be nice to read and self documenting
  3. -Wno-unused-parameter Contra: killing useful warning Pro: simple

What do you think / how do you currently handle that?

stephanemagnenat commented 10 years ago

Personally I prefer the unused macro, as it makes things explicit.

simonlynen commented 10 years ago

I think the cleanest solution is to use of the following:

void Process(int parameterA, bool /*parameterB*/) {
}
void Process(int parameterA, bool parameterB) {
  static_cast<void>(parameterB);
}

The following is bad, since not portable

void Process(int parameterA, bool parameterB __attribute__((__unused__))) {
}
stephanemagnenat commented 10 years ago

I did not know about the static_cast trick, not a bad solution but a bit geeky. Maybe have the name in comments is the simplest solution after all.

markusachtelik commented 10 years ago

+1 for Simons first solution.

Am 30.04.2014 um 12:10 schrieb "Simon Lynen" notifications@github.com<mailto:notifications@github.com>:

I think the cleanest solution is to use of the following:

void Process(int parameterA, bool /parameterB/) { }

void Process(int parameterA, bool parameterB) { static_cast(parameterB); }

The following is bad, since not portable

void Process(int parameterA, bool parameterB attribute((unused))) { }

— Reply to this email directly or view it on GitHubhttps://github.com/ethz-asl/programming_guidelines/issues/17#issuecomment-41780461.

HannesSommer commented 10 years ago

I like Simon's first solution as well.

I guess we basically agreed on that. We should include it in the guideline. Any objections?

furgalep commented 10 years ago

Yes, please include it in the guidelines!

And anyone feel free to update current code!

HannesSommer commented 10 years ago

I've just pushed a small perl script (to this rep) to do convert existing code based on the compiler warnings (e.g. Jenkin's raw output)

It is NOT perfect. So check the result after use (e.g. git diff).

HannesSommer commented 10 years ago

Reopening to not forget the entry in the guidelines.

furgalep commented 9 years ago

@HannesSommer , can you update the programming guidelines?

HannesSommer commented 9 years ago

Yes, I will do that. The big problem with the guidelines is that it needs cleaning up and the github wiki language/interface is just a bit too limited to support that well. This is blocking me to work on it - maybe I manage to find a good way to improve the situation this time...

furgalep commented 9 years ago

Feel free to use Latex if that helps. For now, just add this to the list and close the issue.

HannesSommer commented 9 years ago

Added the essence as paragraph 68b to the style guide. https://github.com/ethz-asl/programming_guidelines/wiki/Cpp-Coding-Style-Guidelines#68b-unused-function-parameters-should-have-a-name-comment-instead-of-a-name