[MPlayer-dev-eng] [PATCH] GCC: Do not warn about unused paramters (was Re: [MPlayer-cvslog] r36944 - in trunk/gui/win32: interface.c) playlist.c playlist.h

[Alexander Strasser]

Hi Ingo,

On 2014-02-25 23:53 +0100, Ingo Brückl wrote:
> Reimar Döffinger wrote on Tue, 25 Feb 2014 23:23:04 +0100:
> 
> > On 25.02.2014, at 23:14, Ingo Brückl <ib at wupperonline.de> wrote:
> >> Reimar Döffinger wrote on Tue, 25 Feb 2014 22:44:55 +0100:
> >>
> >>> On 25.02.2014, at 22:29, Ingo Brückl <ib at wupperonline.de> wrote:
> >>>> Reimar Döffinger wrote on Tue, 25 Feb 2014 20:43:51 +0100:
> >>>>
> >>>>> On Tue, Feb 25, 2014 at 07:39:10PM +0100, Ingo Brückl wrote:
> >>>>>> Reimar Döffinger wrote on Tue, 25 Feb 2014 19:01:27 +0100:
> >>>>>>
> >>>>>>>> @@ -501,6 +501,8 @@ static unsigned __stdcall GuiThread(void
> >>>>>>>> {
> >>>>>>>>    MSG msg;
> >>>>>>>>
> >>>>>>>> +    (void) param;
> >>>>>>
> >>>>>>> This is a bit ugly and might not work in all compilers.
> >>>>>>
> >>>>>> I'd say it'll work on most (all?) compilers. Yes, it's ugly.
> >>>>>>
> >>>>>>> I think using av_unused on the parameter would be nicer,
> >>>>>>
> >>>>>> I don't mind using it, but it would be nice if gcc would warn if such a
> >>>>>> parameter *will* be used later. (This is the only thing I dislike about the
> >>>>>> (void) solution. It'll look strange if you forget to remove it later. With
> >>>>>> av_unused it'll even look stranger.)
> >>>>
> >>>>> I don't think gcc warns, in part because it is sometimes used for
> >>>>> variables that will only be used under some #ifdef.
> >>>>> But I never actually checked...
> >>>>
> >>>> Do we have a chance to change the definition of av_unused to something like
> >>>>
> >>>> #define av_unused(v) __attribute__((unused)) v##_unused
> >>>>
> >>>> or should we define a mp_unused like this?
> >>
> >>> Couldn't you just rename the variable itself when you add the av_unused?
> >>> Doesn't seem to me like there's that much benefit in having it hidden in
> >>> a macro.
> >>
> >> The idea behind it is to only add or remove a mp_unused() without changing
> >> any identifiers (and to automatically disable the usage of those marked
> >> mp_unused()).
> 
> > My point is: what is the big advantage of a diff from
> > int somevar;
> > to
> > int mp_unused(somevar);
> > instead of
> > int av_unused somevar_unused;
> 
> Not much difference in a diff I have to admit, but I think the first one
> looks nicer and if you are going to mark a variable unused you don't have to
> remember to rename the variable as well (or you won't notice if it may be
> still used). The mp_unused() is a bit more "fail-safe" and easier to use,
> IMO.

  IMHO just keep things in the spirit of the GCC attribute documentation:

  unused
    This attribute, attached to a variable, means that the variable is meant
    to be possibly unused. GCC does not produce a warning for this variable.


  BTW @ all:
  Why don't we have -Wno-unused-parameter in CFLAGS?

  I can hardly even think of a case where an unused parameter warning
would be relevant. Also I cannot remember a single occasion where it
mattered in practice.

  Adding the above mentioned flag eliminates 2000+ unused parameter
warnings here if I did not mess up my measurements.

  Patch attached.


  Alexander
-------------- next part --------------
From 21185b247d0505fe5716fb9f3b045c868b5ed56f Mon Sep 17 00:00:00 2001
Message-Id: <21185b247d0505fe5716fb9f3b045c868b5ed56f.1393424283.git.eclipse7 at gmx.net>
From: Alexander Strasser <eclipse7 at gmx.net>
Date: Wed, 26 Feb 2014 15:15:19 +0100
Subject: [PATCH] build: GCC: Do not warn about unused function parameters

This only effects debug builds because they use the -W (AKA -Wextra)
flag which amongst other things enables the typically only noise
generating warning about unused parameters.

Signed-off-by: Alexander Strasser <eclipse7 at gmx.net>
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 363179e..afe84ed 100755
--- a/configure
+++ b/configure
@@ -2756,7 +2756,7 @@ if test -z "$CFLAGS" || test "$_profile" != "" || test "$_debug" != ""; then
     CFLAGS="-O2 $_march $_mcpu $_pipe"
   else
     CFLAGS="-O4 $_march $_mcpu $_pipe -ffast-math -fomit-frame-pointer"
-    WARNFLAGS="-Wall -Wno-switch -Wno-parentheses -Wpointer-arith -Wredundant-decls"
+    WARNFLAGS="-Wall -Wno-unused-parameter -Wno-switch -Wno-parentheses -Wpointer-arith -Wredundant-decls"
     WARN_CFLAGS="-Werror-implicit-function-declaration"
     extra_ldflags="$extra_ldflags -ffast-math"
   fi
-- 
1.7.10.2.552.gaa3bb87

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20140226/9380a8e6/attachment.asc>