Bug 7384 – Turn off -Wno-unused-param for JavaScriptCore and get rid of unused params

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Maciej Stachowiak]

Turn off -Wno-unused-param for JavaScriptCore and get rid of unused params. We should to do this because KDE has the warning enabled and we don't want to make them spew warnings.

[Maciej Stachowiak]

Created attachment 6659 [details]
turn on unused param warning, fix warnings

[Darin Adler]

Comment on attachment 6659 [details]
turn on unused param warning, fix warnings

What I don't like about this pattern:

    (void)other

is that you can then subsequently use the "other" variable; so such a line doesn't mean that the variable is unused, it just means that someone quieted an unused variable warning at some point.

I much prefer the technique of leaving out the parameter because it really *does* mean that the variable is unused.

I see the (void) technique being used here, presumably in cases where the ifdef would be more complicated without it. I'd prefer not to use it at all.

That having been said, the patch seems fine to land. We can revisit that later.

r=me

[Maciej Stachowiak]

(In reply to comment #2)
> (From update of attachment 6659 [details] [edit])
> What I don't like about this pattern:
> 
>     (void)other

I don't like it either. I only used it when the parameter could not be removed, for one of the following reasons:

- it was a plain C file so omitting parameters would be illegal (I think)
- it was an ObjC method parameter, those can't be omitted
- it was a parameter used in only one branch of an ifdef

Do you have alternate recommendations for any of these cases? For ifdefs I could ifdef the whole function, for the other cases I don't think there is much I can do.

[David Faure]

Qt defines the Q_UNUSED macro for this (and ACE defines a similar ACE_UNUSED),
so that from reading the source it's clear what the line is for.
Yes it doesn't mean that the variable is actually unused (e.g. in all cases),
but it simply means "it's ok if this variable is unused in this method".
Of course if it's used that's ok too.

But I prefer this rather than omitting argument names, since doing that obfuscates the code (in the worst case you could have a foo(int,int,int,int,int) method, e.g. as a virtual method reimplementation so you can't remove the unused args).

So my suggestion would be to define a macro like KXML_UNUSED or WK_UNUSED.