Page 1 of 2
unused variables
Posted: Fri Aug 21, 2015 07:25
by 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:
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?
Re: unused variables
Posted: Sat Aug 22, 2015 13:59
by 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.
Re: unused variables
Posted: Sat Aug 22, 2015 15:56
by 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...
Re: unused variables
Posted: Sat Aug 22, 2015 17:25
by Ident
If I understand correctly you would want to add this macro at a lot of places inside CEGUI to prevent those warnings, right?
Re: unused variables
Posted: Sun Aug 23, 2015 15:26
by 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...
Re: unused variables
Posted: Sun Aug 23, 2015 15:34
by 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.
Re: unused variables
Posted: Sun Aug 23, 2015 15:38
by 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).
Re: unused variables
Posted: Sun Aug 23, 2015 15:46
by 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?
Re: unused variables
Posted: Sun Aug 23, 2015 15:57
by YaronCT
I definitely think it improved readability. I think many ppl wouldn't understand the logic behind:
but:
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.
Re: unused variables
Posted: Sun Aug 23, 2015 16:45
by Ident
We will need to see about Martin's and timotei's opinions on this.
Re: unused variables
Posted: Sun Aug 23, 2015 16:47
by 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.
Re: unused variables
Posted: Sun Aug 23, 2015 19:26
by 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.
Re: unused variables
Posted: Tue Aug 25, 2015 00:55
by iceiceice
thumbs up to this whole thread
Re: unused variables
Posted: Thu Aug 27, 2015 12:23
by 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.
Re: unused variables
Posted: Thu Aug 27, 2015 15:10
by Ident
If Kulik has no objections then neither do I. Feel free to make a PR.