unused variables

[YaronCT]

When there's an unused variable (e.g. parameter), the compiler usually gives a warning. In Qt, for instance, there's a macro "Q_UNUSED" that can be used to suppress this warning:

Code: Select all

Q_UNUSED(myVar);

Which is defined something like:

Code: Select all

#define Q_UNUSED(var) (static_cast<void>(var))

Is there something similar in CEGUI? And if not, would u mind If I added one?

[Ident]

First we would need to evaluate where this happens and why the parameters are not used. I would first assume the method was not designed correctly if its not all parameters are necessary. Do you get a lot of these warnings? You may wanna put them here so we can look at them / discuss them one after the other. If it turns out there is actual need for the macro you described then we should consider adding it.

[YaronCT]

It's not something that rare, and it most often doesn't mean u do anything wrong. Often, u re-implement a virtual method and not use all its parameters. Another situation, that I currently encounter, is when calling C functions from Java, there are some mandatory parameters which I don't use. And if u want a 3rd scenario, consider:

Code: Select all

bool is_success(my_func());
assert(is_success);

When doing a Release build, the "assert" line would expand to nothing, and the compiler gives a warning that "is_success" is set but not used.

Imo an unused variable is a pretty common situation, and I could really use such a macro in my current project. If there's isn't such a macro, and u don't mind, I'll add one; it shouldn't cost anything...

[Ident]

If I understand correctly you would want to add this macro at a lot of places inside CEGUI to prevent those warnings, right?

[YaronCT]

You're asking a good question. As a matter of fact, currently, I wasn't thinking of it, but only to use it in my new code.

However, regardless of the unused variables warnings, I thought in general that it would be a great idea to make CEGUI build in GCC/Clang with extra warnings turned on ("-Wall -Wextra"). This way we could, other than cleaning the code, catch some hidden bugs, and not less important, make it practical for a developer like me (that works with GCC) to build my CEGUI development projects with extra warnings turned on, and find possible problems with my own code without being flooded by warnings from existing CEGUI code.

So, yes, my idea is not only to eliminate all unused variables warnings from CEGUI, but eliminate all warnings from CEGUI. I've already started working on it...

[Ident]

I am all for that, especially if it does not involve adding new macros of any kind.

I actually always clean up warnings for Visual Studio, such as type conversions etc. Currently there is only one type of warning left on default for me which is related to glm and i already filed a bug report for that in the glm repo. So yes, eliminating warnings, if they are safe to be ignored and can't be fixed, is always a good thing. Fixing them is of course better.

[YaronCT]

Well, but I would like to add the aforementioned macro:

Code: Select all

#define CEGUI_UNUSED(var) (static_cast<void>(var))

Do u have any objection? For some of the places this is necessary to eliminate the unused variables warnings. Of course, I'm checking all cases one by one to make sure that the variable/parameter can't be totally removed (e.g. because we're overriding a virtual function from a base class).

[Ident]

Ah ok, so we would indeed have to do this for CEGUI code. Why is the macro better than just using static_cast<void>(...) ? Length-wise they are almost equal. Is this supposed to help readbility?

[YaronCT]

I definitely think it improved readability. I think many ppl wouldn't understand the logic behind:

Code: Select all

static_cast<void>(my_var)

but:

Code: Select all

CEGUI_UNUSED(my_var)

is much clearer, and further explanation can be given near the definition of the "CEGUI_UNUSED" macro. Afaik Qt defines such a macro, and many other libraries do too.

[Ident]

We will need to see about Martin's and timotei's opinions on this.

[YaronCT]

Ok, plz update me.

Btw I've finished eliminating all warnings with GCC, so when this issue is resolved, I'll open a PR.

[YaronCT]

Just wanted to add: using a macro makes it more flexible. Say that someday the C++ standard adds a mechanism to indicate that a variable is unused. Then we can change the macro definition to that mechanism, instead of the cast-to-void hack.

[iceiceice]

thumbs up to this whole thread

[Kulik]

We use:

Code: Select all

void someFunction(T /*a*/, T* /*b*/)
{}


in some places. However I think the CEGUI_UNUSED macro is a better idea, so I am all for it And replacing the above with it.

This can go even to v0-8 as it won't break any compatibility.

[Ident]

If Kulik has no objections then neither do I. Feel free to make a PR.