[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

[Ivan Kalvachev]

On 2/26/14, Alexander Strasser <eclipse7 at gmx.net> wrote:
> 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.

I do prefer this solution.
The custom mp_unused() would make the code look quite strange:

int foo(void* ctx, int mp_unused(param1), mp_unused(int param2)){
    return ctx != NULL;
}

Also, the unused variables are mostly useless for finding and fixing bugs.

Best Regards
   iive