-Wunused-parameter should not warn about virtual method declarations with bodies

[llvmbot]

Bugzilla Link	10983
Resolution	INVALID
Resolved on	Sep 22, 2011 10:33
Version	trunk
OS	Linux
Reporter	LLVM Bugzilla Contributor
CC	@DougGregor

Extended Description

In the following code:

struct X { virtual void f (int a) { } };

One gets a warning from -Wunused-parameter, "unused parameter 'a'".

While this is understandable, I think it's undesirable -- clearly the parameter
'a' will often be used by overrides of X::f, and the parameter name 'a' serves
a valuable documentation purpose here, even if it's not strictly required.

The basic problem of course, is that in this case, f is both a virtual method
declaration which covers overriding methods as well, and a definition of X's
definition of it.

One can avoid this by defining X::f outside the class definition, but for
trivial definitions like the above, this would seem like annoying make-work if
done only to shut up -Wunused-parameter.

My current method of avoiding the warning is to comment out the parameter name:

virtual void f (int /* a */) { }

but this again seems like an annoying wart, rather than natural code -- it's
both less readable (especially when the parameter has a funny type like a
function pointer) and yields a slightly confusing inconsistency ("why do you
comment-out some method parameter names but not others?!" "oh it's just to shut
up -Wunused-parameter... :(").

Anyway, the current behavior seems wrong to me; I think -Wunused-parameter
should stay silent for method virtual definitions inside the class definition.

[If this were a very rare scenario, I suppose I wouldn't care about the need to
work around it, but it seems to happen all the time for me...]

Thanks,

-Miles

p.s. gcc does the same thing; I just posted an identical bug to gcc's bugzilla ... :]

[DougGregor]

In the following code:

struct X { virtual void f (int a) { } };

One gets a warning from -Wunused-parameter, "unused parameter 'a'".

While this is understandable, I think it's undesirable -- clearly the parameter
'a' will often be used by overrides of X::f, and the parameter name 'a' serves
a valuable documentation purpose here, even if it's not strictly required.

The basic problem of course, is that in this case, f is both a virtual method
declaration which covers overriding methods as well, and a definition of X's
definition of it.

One can avoid this by defining X::f outside the class definition, but for
trivial definitions like the above, this would seem like annoying make-work if
done only to shut up -Wunused-parameter.

My current method of avoiding the warning is to comment out the parameter name:

virtual void f (int /* a */) { }

but this again seems like an annoying wart, rather than natural code -- it's
both less readable (especially when the parameter has a funny type like a
function pointer) and yields a slightly confusing inconsistency ("why do you
comment-out some method parameter names but not others?!" "oh it's just to shut
up -Wunused-parameter... :(").

Anyway, the current behavior seems wrong to me; I think -Wunused-parameter
should stay silent for method virtual definitions inside the class definition.

I disagree. I would find it very weird---and very annoying---if I lost out on this warning just because my function was virtual. Granted, some virtuals can be fixed (this parameter is never needed) and some cannot (this parameter is only important for some of the virtual overrides), but I think the principle of least surprise says that we shouldn't silence the warning for parameters of virtuals. Plus, there are three (!) ways to silence this warning:

• comment out the parameter name
• add attribute((unused) to the parameter, after the parameter name
• add "(void)a;" to the function body, to silence the warning

The latter two both provide better documentation that you intended not to use this parameter than your suggestion of just turning off the warning.

p.s. gcc does the same thing; I just posted an identical bug to gcc's bugzilla
... :]

IIRC, this discussion came up regarding GCC's behavior once, something like 10 years ago, and they rejected it. Maybe you'll have more luck this time :)