unused variables

Discussion regarding the development of CEGUI itself - as opposed to questions about CEGUI usage that should be in the help forums.

Moderators: CEGUI MVP, CEGUI Team

YaronCT
CEGUI Team
Posts: 448
Joined: Fri Jun 19, 2015 12:18
Location: Kiryat-Bialik, Israel

unused variables

Postby YaronCT » Fri Aug 21, 2015 07:25

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?

User avatar
Ident
CEGUI Team
Posts: 1894
Joined: Sat Oct 31, 2009 13:57
Location: Austria

Re: unused variables

Postby Ident » Sat Aug 22, 2015 13:59

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.
CrazyEddie: "I don't like GUIs"

YaronCT
CEGUI Team
Posts: 448
Joined: Fri Jun 19, 2015 12:18
Location: Kiryat-Bialik, Israel

Re: unused variables

Postby YaronCT » Sat Aug 22, 2015 15:56

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...

User avatar
Ident
CEGUI Team
Posts: 1894
Joined: Sat Oct 31, 2009 13:57
Location: Austria

Re: unused variables

Postby Ident » Sat Aug 22, 2015 17:25

If I understand correctly you would want to add this macro at a lot of places inside CEGUI to prevent those warnings, right?
CrazyEddie: "I don't like GUIs"

YaronCT
CEGUI Team
Posts: 448
Joined: Fri Jun 19, 2015 12:18
Location: Kiryat-Bialik, Israel

Re: unused variables

Postby YaronCT » Sun Aug 23, 2015 15:26

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...

User avatar
Ident
CEGUI Team
Posts: 1894
Joined: Sat Oct 31, 2009 13:57
Location: Austria

Re: unused variables

Postby Ident » Sun Aug 23, 2015 15:34

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.
CrazyEddie: "I don't like GUIs"

YaronCT
CEGUI Team
Posts: 448
Joined: Fri Jun 19, 2015 12:18
Location: Kiryat-Bialik, Israel

Re: unused variables

Postby YaronCT » Sun Aug 23, 2015 15:38

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).

User avatar
Ident
CEGUI Team
Posts: 1894
Joined: Sat Oct 31, 2009 13:57
Location: Austria

Re: unused variables

Postby Ident » Sun Aug 23, 2015 15:46

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?
CrazyEddie: "I don't like GUIs"

YaronCT
CEGUI Team
Posts: 448
Joined: Fri Jun 19, 2015 12:18
Location: Kiryat-Bialik, Israel

Re: unused variables

Postby YaronCT » Sun Aug 23, 2015 15:57

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.

User avatar
Ident
CEGUI Team
Posts: 1894
Joined: Sat Oct 31, 2009 13:57
Location: Austria

Re: unused variables

Postby Ident » Sun Aug 23, 2015 16:45

We will need to see about Martin's and timotei's opinions on this.
CrazyEddie: "I don't like GUIs"

YaronCT
CEGUI Team
Posts: 448
Joined: Fri Jun 19, 2015 12:18
Location: Kiryat-Bialik, Israel

Re: unused variables

Postby YaronCT » Sun Aug 23, 2015 16:47

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
CEGUI Team
Posts: 448
Joined: Fri Jun 19, 2015 12:18
Location: Kiryat-Bialik, Israel

Re: unused variables

Postby YaronCT » Sun Aug 23, 2015 19:26

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
Not too shy to talk
Not too shy to talk
Posts: 33
Joined: Thu Jul 09, 2015 10:20

Re: unused variables

Postby iceiceice » Tue Aug 25, 2015 00:55

thumbs up to this whole thread :)

User avatar
Kulik
CEGUI Team
Posts: 1382
Joined: Mon Jul 26, 2010 18:47
Location: Czech Republic
Contact:

Re: unused variables

Postby Kulik » Thu Aug 27, 2015 12:23

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.

User avatar
Ident
CEGUI Team
Posts: 1894
Joined: Sat Oct 31, 2009 13:57
Location: Austria

Re: unused variables

Postby Ident » Thu Aug 27, 2015 15:10

If Kulik has no objections then neither do I. Feel free to make a PR.
CrazyEddie: "I don't like GUIs"


Return to “CEGUI Library Development Discussion”

Who is online

Users browsing this forum: No registered users and 1 guest