Activate compiler warnings on unused parameters

[ricab]

We currently add -Wno-unused-parameter in cmake. This makes it easy to leave unused parameters around, sometimes propagating them to a lot of code.

For instance, there is an unused grpc::ServerContext* parameter that is propagated from the Rpc to all daemon handling methods and beyond (this particular case is currently being fixed in #682).

[townsend2010]

I'm not sure we want to do this because DaemonRpc still overrides the Rpc::Service class and ServerContext is not used there as well.

[ricab]

The warning is perfectly compatible with external signatures that can't be changed (it would be pretty useless otherwise) Actually, I think that is when it is most useful.

What you do in those cases is simply not name the parameter. For instance, DaemonRpc would declare:

grpc::Status create(grpc::ServerContext*, const CreateRequest* request,
                    grpc::ServerWriter<CreateReply>* reply) override;
...

This immediately indicates, both to the compiler and the reader, that the parameter is not used. And it actually prevents it from being used inadvertently (like passing it around, as happened there).

IOW, the warning exists to promote an explicit decision: you either detect the param is a leftover and should be removed, or explicitly indicate it needs to be maintained as part of the signature even though it is not used.

In the cases when one feels the param name conveys relevant information even when unused, that information can be put in a comment. It is not the case in the current example, as the name simply repeats what the type already says, but if it were, one should do simply

grpc::Status create(grpc::ServerContext* /*context*/, const CreateRequest* request,
                    grpc::ServerWriter<CreateReply>* reply) override;
...

[townsend2010]

@ricab,

Ok, thanks for the explanation and makes perfect sense.