Skip to main content
GameDev.net gamedev.net
🔒 Locked

Unused param

Started by Alundra Apr 5, 2015 at 6:54 PM 15 replies 3.9k views
Original Post
Alundra
Alundra

Hi,

I always used this macro to remove warning when needed :


#define UNUSED(x) static_cast<void>(x)

But I recently see, a template version could be used like that :


template<class T> INLINE void UNUSED(const T&){}

What is the best to use and why ? I read Intel compiler didn't like static_cast to remove warning, but apparently now it does from other post.

Thanks

iMalc
iMalc

Unless I'm in the situation where the parameter is used in only debug or only release build, I tend to just comment out the parameter name.

I've also seen people just mention the variable name in the function body on a line by itself, without any cast.

SeanMiddleditch
SeanMiddleditch

hat is the best to use and why ?


The template version requires more work out of the compiler and doesn't really accomplish anything extra. In general, I'd lean towards avoiding the template version unless you can qualify exactly what benefit you get from it. The template version is also technically generating a function call which can have a big runtime overhead in some build configuration, though that's not likely to be relevant to this use case.

A macro can of course forward to a template if things like ICC requires it. It's very common to put all the definitions for things like UNUSED_PARAM into a header file that has a bunch of compiler/platform detection (or uses a configuration header that has that pre-generated) to use the most appropriate implementation, as some of these things will vary heavily by compiler or platform.

Remember, C++ as a language has absolutely no notion of an unused variable warning. That's something that compilers add on to try to be helpful. Because it's not part of C++, there's no standard on how those warnings work or how they should be suppressed. As with most anything that isn't firmly locked down and etched in stone, there isn't a single "best" way to do it.

Another option I'm somewhat in favor of is to just turn off the unused parameter warning. I haven't seen it actually be useful in solving either a correctness or a performance problem in... well, ever that I can recall.
Sean Middleditch – Game Systems Engineer – Join my team!
Hodgman
Hodgman
The template you posted would itself cause a slightly different warning on one compiler I use, as it is itself a function with an unused argument.
L. Spiro
L. Spiro
void FunkySmell( int _iUnused ) {
}
// Warning, unused parameter.

void FunkySmell( int _iUnused ) {
    static_cast<void>(_iUnused);
}
// Warning, useless statement (no side-effect).

void FunkySmell( int /*_iUnused*/ ) {
}
// Correct.

L. Spiro
I restore Nintendo 64 video-game OST’s into HD! https://www.youtube.com/channel/UCCtX_wedtZ5BoyQBXEhnVZw/playlists?view=1&sort=lad&flow=grid
Hodgman
Hodgman

void FunkySmell( int _iUnused ) {
}
// Warning, unused parameter.

void FunkySmell( int _iUnused ) {
    static_cast<void>(_iUnused);
}
// Warning, useless statement (no side-effect).

void FunkySmell( int /*_iUnused*/ ) {
}
// Correct.
L. Spiro

The 3rd is my favorite too, but... on one of the compilers we use, the 3rd one also produces a warning (unused unnamed argument).
Our solution is:
(void)(_iUnused);

or: #define UNUSED(x) ((void)(x))
...which AFAIK doesn't cause any no-side-effect warnings.

L. Spiro
L. Spiro

I see.
But I am fairly sure (entirely open to correction) that your cast produces the same result as my static cast, which generates a warning in Xcode.
No matter what, Coverity is going to flag these things.


I’m not sure of any way to avoid these kinds of warnings entirely (and we are not having a discussion on why we are putting “useless” parameters into functions because there are so many legitimate reasons why it happens), so your best bet is just to figure out what reduces the number of warnings you yourself get while compiling.

Hodgman and I have our favorite ways. They don’t always eliminate the problems, especially when you are supporting lots of platforms or even just overloading operator new. Right now your best course of action is simply to do what it takes to remove the warnings that you are encountering. I stand by my proposal, but I literally don’t think any course of action will pacify every compiler out there. For now it makes more sense to just do what pacifies most of them, and if you encounter some remaining issues then ask again, specifying which compiler you are using and what warning you aim to avoid.

L. Spiro

I restore Nintendo 64 video-game OST’s into HD! https://www.youtube.com/channel/UCCtX_wedtZ5BoyQBXEhnVZw/playlists?view=1&sort=lad&flow=grid
Alundra
Alundra

If no real solution, an if-else checking the compilator can always gives the trick, actually static_cast remove all of these warnings on msvc,gcc and clang.

I didn't test myself intel compilator but apparently now the template version is not needed, the static_cast seems to remove the warning too.

collie
collie

With VC++ I use the macro UNREFERENCED_PARAMETER(x).

I've just had a quick peek and it's defined as the following.

#define UNREFERENCED_PARAMETER(P) (P)

swiftcoder
swiftcoder




and we are not having a discussion on why we are putting “useless” parameters into functions because there are so many legitimate reasons why it happens

I'm not sure I buy the 'legitimate' part of that. As I see it, there are usually two cases where we end up with unused parameters:

  • When the implementation has changed such that the public API no longer accurately reflects the provided functionality.
  • When you abuse polymorphism such that a child must override a parent method to do something unrelated.

Neither of those are particularly legitimate - the former can be addressed via deprecation, and the latter by refactoring.

Tristam MacDonald. Ex-BigTech Software Engineer. Future farmer. [https://trist.am]
Hodgman
Hodgman

Neither of those are particularly legitimate - the former can be addressed via deprecation, and the latter by refactoring.

We work in games though, where we're shipping products that are over-ambitious and under-budgeted. Software engineering becomes less about perfect ideals, and more about an optimal strategy to deal with sub-optimal resources wink.png ..... which means it's perfectly legitimate to choose to form a strategy that manages -- not rewrites -- bad code sad.png

I see UNUSED macros more often used for local variables than arguments though. Often there's some debug code that's only required for an assertion -- e.g.


bool ok = Frobnicate();
ASSERT( ok );
UNUSED( ok );

Say that the only reason Frobnicate can fail is due to a logic error (and we can't ship the game if there's a logic error) so the retail version has no need to examine the return code.
In the development version, ASSERT validates our assumption that the function can't fail, and in the retail version UNUSED suppresses the warning resulting from ignoring the return code (because we're responsible engineers and use warnings-as-errors and warning-level-4 biggrin.png).

Often unused params to functions serve the same purpose -- e.g. with your example of abused polymorphism, where one implementation doesn't quite match the interface: Often in such cases, this implementation will perform assertions on the otherwise unused argument, but still need to suppress the unused-argument warning in retail builds.

e.g. maybe most platforms require this parameter, but one platform doesn't require it... but that platform can still provide a nice development environment by checking that the value will also work fine for the rest of your platforms.

swiftcoder
swiftcoder




because we're responsible engineers and use warnings-as-errors and warning-level-4

That's already a fallacy. You've explicitly decided to accept a bunch of restrictive warnings as errors, and rather than fixing those errors, you are willing to paper them over and ignore them.

There's no earthly reason to enable the unused parameter warning as an error, if you aren't going to treat it as if it were an actual error condition. At that point you are just pandering to the foibles of the tool at hand.

Tristam MacDonald. Ex-BigTech Software Engineer. Future farmer. [https://trist.am]
SmkViper
SmkViper


because we're responsible engineers and use warnings-as-errors and warning-level-4

That's already a fallacy. You've explicitly decided to accept a bunch of restrictive warnings as errors, and rather than fixing those errors, you are willing to paper them over and ignore them.

There's no earthly reason to enable the unused parameter warning as an error, if you aren't going to treat it as if it were an actual error condition. At that point you are just pandering to the foibles of the tool at hand.



As a responsible engineer, it is my responsibility to determine if the compiler is being overly strict, or whether it caught a potential bug. It is also my responsibility to estimate the cost of fixing the warning vs the cost of "papering over" the warning. At that point I can make an informed choice, or inform a producer who can make the call based on the state of the project.

Sometimes you need to ship a product and the risk of re-architecting your game to make warnings go away isn't worth the benefit of removing a warning that is showing you something harmless.

You don't ignore the tool - but you don't let the tool control you either smile.png
swiftcoder
swiftcoder

When was the last time any of you had an unused parameter warning that actually represented a bug? I can't recall such an instance, myself.

In which case, adding an -Wno-unused-variable (or equivalent) to your build system seems a lot less disruptive than littering the code with UNUSED() macros (or any of the other less discoverable alternatives).

Tristam MacDonald. Ex-BigTech Software Engineer. Future farmer. [https://trist.am]
Erik Rufelt
Erik Rufelt

I usually enable unused parameter/variable warnings, and use an UNUSED macro or template to suppress them. One frequent use is during development, where I leave the warnings for partially completed functionality or where I'm not sure yet if I want to use a particular variable. After completing the function and realizing the parameter actually isn't needed I add UNUSED to disable warnings if I can't just remove the parameter entirely.

Of course I could just add #pragma warnings when creating the function instead.. but it's nice to know I didn't forget any variables, and only set them as unused when having properly checked the function. Also might leave parameters for future use where it's far enough into the future that I don't want a constant warning, but know I will sometime come back to that function.

I actually would like an unused-attribute that gives an error if the parameter marked unused is used later, to force me to remove the UNUSED if I later add functionality that uses the parameter. Those UNUSED-markers is a pretty nice way of documenting what a function does and doesn't do sometimes.

(For this case commenting out the name with /* name */ would work well).

Also useful for mock-interfaces or debug functions.

Like virtual result networkRequest(url, port, protocol), and the testing-interface just returns a static result and uses UNUSED on parameters not used, or for base-classes that do nothing in some virtual method that I still don't want pure for efficiency in writing simple classes that uses the base.

I'm sure there will be a theoretically better way pretty much every time, but I find both that it's often efficient to allow some parameters or return values to remain unused, and that the warning itself is useful.

In the case of an unused return variable it's a helpful documentation in the calling code to whoever reads the code the next time, and to give errors or warnings if the return type was to be changed at a later time... though in that case I have a tendency to comment out the return variable instead of using UNUSED.

SmkViper
SmkViper

When was the last time any of you had an unused parameter warning that actually represented a bug? I can't recall such an instance, myself.

In which case, adding an -Wno-unused-variable (or equivalent) to your build system seems a lot less disruptive than littering the code with UNUSED() macros (or any of the other less discoverable alternatives).


Comes up all the time when I'm making changes to an already existing function and modify it in such a way that I don't need some of the parameters that used to be passed to it. It's a good reminder to remove the parameter and clean up the callers.
Hodgman
Hodgman


because we're responsible engineers and use warnings-as-errors and warning-level-4

That's already a fallacy. You've explicitly decided to accept a bunch of restrictive warnings as errors, and rather than fixing those errors, you are willing to paper them over and ignore them.

There's no earthly reason to enable the unused parameter warning as an error, if you aren't going to treat it as if it were an actual error condition. At that point you are just pandering to the foibles of the tool at hand.


That would be fair, if you always ignored the error except to treat it as an instruction to insert the UNUSED macro.
This is not the case.
As with all warnings, usually you take their advice, but sometimes you overrule them and document that you're intentionally doing something smelly.

Compiler errors tell you that toy do have a syntax or logic error.
Warnings tell you that you probably have a logic error, but maybe dont.

If a warning is a false alarm, there's usualy a very small tweak that can be made that not only supresses the warning, but also documents your assumtion for other programmers.
e.g.
u32 a = 0x12345678;
u16 b = (u16)a; // warning - possible data loss
u16 c = (u16)(a & 0xFFFF); // no warning, plus exact same code generation (the AND is optimized out)
//The no-warning version also explicitly documents the programmer's intention, making the slicing obvious, making the logic more readable
This goes for static code analysis too:
void* buffer = malloc( sizeof(Header) + sizeof(Content));
memset(buffer, 0, sizeof(Header)); // warning! - partial buffer fill
....
So we add a comment to disable that static warning, and explain the smell to fellow coders who might also pick up on it:
....
memset(buffer, 0, sizeof(Header)); //-W1234 -- deliberately only clearing the header, as it's assumed the caller will complete overwite the content section.
Having the warnings enabled in the first place is important, so that these potential bugs/smells are pointed out.

Having warnings alone is often useless though, as once more than a handful are comitted to the project, people start to ignore them, and ignore any new ones they write, defeating the purpose.
So warnings-as-errors is used to ensure they're not ignored - you must clean up any automatically detected code smells before you can even commit your code to the main branch.

When was the last time any of you had an unused parameter warning that actually represented a bug?

Similar to what you said earlier, they're not usually bugs, but code smells -- "you've refactored this code so a parameter is no longer needed, are you sure you don't want to clean up the interface too?".

As above, if after giving thought to that automated question you decide that, no, you don't want to clean up the interface for whatever reason, then you can leave behind an UNUSED macro and a comment documenting your decision, so that the compiler and your colleagues know it was an intentional choice.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.