RFC: Using `-Wno-unused-parameter` in Plasma projects

[Vlad Zahorodnii]

Hi,

The main issue with -Wunused-parameter is that it makes development more 
painful. For example, say that you have an observer class and you need 
to override a dozen of methods to handle various events, but you don't 
really care about arguments... In every method, you would need to add 
Q_UNUSED() just to shut the compiler up. Besides more work, it produces 
more (boilterplate) code. There are ways to make code compact, e.g.

   class ActivityObserver : public Observer
   {
   public:
       void pointerEvent(const QPoint &pos, int timestamp) override
       {
           Q_UNUSED(pos)
           Q_UNUSED(timestamp)
           notifyActivity();
       }

       void buttonEvent(int button, ButtonState state, int timestamp) 
override
       {
           Q_UNUSED(pos)
           Q_UNUSED(state)
           Q_UNUSED(timestamp)
           notifyActivity();
       }
   };

(a) omit parameter names

   class ActivityObserver : public Observer
   {
   public:
       void pointerEvent(const QPoint &, int) override
       {
           notifyActivity();
       }

       void buttonEvent(int, ButtonState, int) override
       {
           notifyActivity();
       }
   };

The advantages:

* more compact code

The disadvantages:

* code is less readable
* in case you need to use some parameter, you would need to look it up 
in the base class, which is not convenient
* adds more work: you would need to copy method prototype and remove 
each argname manually

(b) comment out argnames

   class ActivityObserver : public Observer
   {
   public:
       void pointerEvent(const QPoint &/*pos*/, int /*timestamp*/) override
       {
           notifyActivity();
       }

       void buttonEvent(int /*button*/, ButtonState /*state*/, int 
/*timestamp*/) override
       {
           notifyActivity();
       }
   };

The advantages:

* compact code
* more readable than just removing argnames

The disadvantages:

* adds more work: you would need to copy method prototype and comment 
out each argname manually


(c) disable -Wunused-parameter

   class ActivityObserver : public Observer
   {
   public:
       void pointerEvent(const QPoint &pos, int timestamp) override
       {
           notifyActivity();
       }

       void buttonEvent(int button, ButtonState state, int timestamp) 
override
       {
           notifyActivity();
       }
   };

The advantages:

* compact code
* no need to manually remove or comment out argnames

The disadvantages:

* ?

Besides adding more work and code, it's easy to forget to remove 
Q_UNUSED() once it actually becomes readable; it litters code with false 
information. Many compilers won't print a warning if an argument is used 
but it's marked as Q_UNUSED.

One could argue that the main benefit of -Wunused-parameter is that it 
makes easy to spot when an argument becomes unused and thus it can be 
removed. But I doubt that it's ever been the case. Speaking from my 
personal experience, if I see an unused argument warning, my natural 
instinct is to add a Q_UNUSED() macro without analyzing every possible 
usage of that method or function.

One could also argue that -Wunused-parameter may help to catch bugs 
caused by not using some parameter. Similar to the case above, the 
likelihood of that being true is slim. If an argument is unused, it's 
more likely that it will be marked as Q_UNUSED() without doing an in 
detail review.

Note that this is not about -Wunused-variable. I believe that it's still 
very useful for the purpose of keeping code clean of unused variables.

So my question is - would it be okay to disable -Wunused-parameter 
throughout all Plasma projects except maybe header-only libraries if 
there are any? If not, would it be acceptable to disable 
-Wunused-parameter per project instance?

Cheers,
Vlad