[AVR] Fix unused argument warning

Message ID 20210930192723.hbh2nemcsm55bxig@lug-owl.de
State Committed
Commit 370374c4d91d83fb69d4b70dd4e48103114a903d
Headers
Series [AVR] Fix unused argument warning |

Commit Message

Jan-Benedict Glaw Sept. 30, 2021, 7:27 p.m. UTC
  Hi!

Configuring GCC with --target=avr-elf --enable-werror-always, I see
this warning that's easy to fix. The options are parsed with a lot of
#ifdefs and it may actually just be unused. Let's just mark it as such.

[all 2021-09-30 00:43:46] /usr/lib/gcc-snapshot/bin/g++  -fno-PIE -c   -g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE   -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wno-error=format-diag -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -I. -I. -I../../gcc/gcc -I../../gcc/gcc/. -I../../gcc/gcc/../include -I../../gcc/gcc/../libcpp/include -I../../gcc/gcc/../libcody  -I../../gcc/gcc/../libdecnumber -I../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../gcc/gcc/../libbacktrace   -o avr-common.o -MT avr-common.o -MMD -MP -MF ./.deps/avr-common.TPo ../../gcc/gcc/common/config/avr/avr-common.c
[all 2021-09-30 00:43:47] ../../gcc/gcc/common/config/avr/avr-common.c: In function 'bool avr_handle_option(gcc_options*, gcc_options*, const cl_decoded_option*, location_t)':
[all 2021-09-30 00:43:47] ../../gcc/gcc/common/config/avr/avr-common.c:80:72: error: unused parameter 'loc' [-Werror=unused-parameter]
[all 2021-09-30 00:43:47]    80 |                    const struct cl_decoded_option *decoded, location_t loc)
[all 2021-09-30 00:43:47]       |                                                             ~~~~~~~~~~~^~~
[all 2021-09-30 00:43:47] cc1plus: all warnings being treated as errors
[all 2021-09-30 00:43:47] make[1]: *** [Makefile:2420: avr-common.o] Error 1

gcc/ChangeLog:

	* common/config/avr/avr-common.c (avr_handle_option): Mark
	argument as ATTRIBUTE_UNUSED.





  Ok for trunk?

Thanks,
  Jan-Benedict

--
  

Comments

Jan-Benedict Glaw Oct. 5, 2021, 2:46 p.m. UTC | #1
Hi,

On Thu, 2021-09-30 21:27:23 +0200, Jan-Benedict Glaw <jbglaw@lug-owl.de> wrote:
> gcc/ChangeLog:
> 
> 	* common/config/avr/avr-common.c (avr_handle_option): Mark
> 	argument as ATTRIBUTE_UNUSED.
> 
> diff --git a/gcc/common/config/avr/avr-common.c b/gcc/common/config/avr/avr-common.c
> index 6486659d27c..a6939ad03d3 100644
> --- a/gcc/common/config/avr/avr-common.c
> +++ b/gcc/common/config/avr/avr-common.c
> @@ -77,7 +77,8 @@ static const struct default_options avr_option_optimization_table[] =
>  
>  static bool
>  avr_handle_option (struct gcc_options *opts, struct gcc_options*,
> -                   const struct cl_decoded_option *decoded, location_t loc)
> +                   const struct cl_decoded_option *decoded,
> +                   location_t loc ATTRIBUTE_UNUSED)
>  {
>    int value = decoded->value;
>  
> 
> 
>   Ok for trunk?

Wanted to give this a ping.

Thanks,
  Jan-Benedict

--
  
Jeff Law Oct. 5, 2021, 2:53 p.m. UTC | #2
On 10/5/2021 8:46 AM, Jan-Benedict Glaw wrote:
> Hi,
>
> On Thu, 2021-09-30 21:27:23 +0200, Jan-Benedict Glaw <jbglaw@lug-owl.de> wrote:
>> gcc/ChangeLog:
>>
>> 	* common/config/avr/avr-common.c (avr_handle_option): Mark
>> 	argument as ATTRIBUTE_UNUSED.
>>
>> diff --git a/gcc/common/config/avr/avr-common.c b/gcc/common/config/avr/avr-common.c
>> index 6486659d27c..a6939ad03d3 100644
>> --- a/gcc/common/config/avr/avr-common.c
>> +++ b/gcc/common/config/avr/avr-common.c
>> @@ -77,7 +77,8 @@ static const struct default_options avr_option_optimization_table[] =
>>   
>>   static bool
>>   avr_handle_option (struct gcc_options *opts, struct gcc_options*,
>> -                   const struct cl_decoded_option *decoded, location_t loc)
>> +                   const struct cl_decoded_option *decoded,
>> +                   location_t loc ATTRIBUTE_UNUSED)
>>   {
>>     int value = decoded->value;
>>   
>>
>>
>>    Ok for trunk?
> Wanted to give this a ping.
I thought I sent a reply a few days ago.  Instead of using 
ATTRIBUTE_UNUSED, just drop the parameter's name.  You should consider 
that pre-approved for this and any other cases you run into.

jeff
  
Jan-Benedict Glaw Oct. 5, 2021, 3:03 p.m. UTC | #3
Hi Jeff,

On Tue, 2021-10-05 08:53:00 -0600, Jeff Law <jeffreyalaw@gmail.com> wrote:
> On 10/5/2021 8:46 AM, Jan-Benedict Glaw wrote:
> > On Thu, 2021-09-30 21:27:23 +0200, Jan-Benedict Glaw <jbglaw@lug-owl.de> wrote:
> > > gcc/ChangeLog:
> > > 
> > > 	* common/config/avr/avr-common.c (avr_handle_option): Mark
> > > 	argument as ATTRIBUTE_UNUSED.
> > > 
> > > diff --git a/gcc/common/config/avr/avr-common.c b/gcc/common/config/avr/avr-common.c
> > > index 6486659d27c..a6939ad03d3 100644
> > > --- a/gcc/common/config/avr/avr-common.c
> > > +++ b/gcc/common/config/avr/avr-common.c
> > > @@ -77,7 +77,8 @@ static const struct default_options avr_option_optimization_table[] =
> > >   static bool
> > >   avr_handle_option (struct gcc_options *opts, struct gcc_options*,
> > > -                   const struct cl_decoded_option *decoded, location_t loc)
> > > +                   const struct cl_decoded_option *decoded,
> > > +                   location_t loc ATTRIBUTE_UNUSED)
> > >   {
> > >     int value = decoded->value;
> > > 
> > > 
> > >    Ok for trunk?
> > Wanted to give this a ping.
> I thought I sent a reply a few days ago.  Instead of using ATTRIBUTE_UNUSED,
> just drop the parameter's name.  You should consider that pre-approved for
> this and any other cases you run into.

Not quite I think. The `loc`ation parameter is used under some
circumstances, though it's dependant on some #ifdefs:

 78 static bool
 79 avr_handle_option (struct gcc_options *opts, struct gcc_options*,
 80                    const struct cl_decoded_option *decoded,
 81                    location_t loc ATTRIBUTE_UNUSED)
 82 {
 83   int value = decoded->value;
 84 
 85   switch (decoded->opt_index)
 86     {
 87     case OPT_mdouble_:
 88       if (value == 64)
 89         {
 90 #if !defined (HAVE_DOUBLE64)
 91           error_at (loc, "option %<-mdouble=64%> is only available if "
 92                     "configured %<--with-double={64|64,32|32,64}%>");
 93 #endif
 94           opts->x_avr_long_double = 64;
 95         }
 96       else if (value == 32)
 97         {
 98 #if !defined (HAVE_DOUBLE32)
 99           error_at (loc, "option %<-mdouble=32%> is only available if "
100                     "configured %<--with-double={32|32,64|64,32}%>");
101 #endif
102         }

Also, this implements the TARGET_HANDLE_OPTION function, so changing
the arguments wouldn't really work and as the `loc` argument is used
under certain circumstances, I kept it as is and marked it unused.
(Adding an #ifdef in front doesn't make the code any more readable.)

Thanks,
  Jan-Benedict

--
  
Jeff Law Oct. 5, 2021, 3:06 p.m. UTC | #4
On 10/5/2021 9:03 AM, Jan-Benedict Glaw wrote:
> Hi Jeff,
>
> On Tue, 2021-10-05 08:53:00 -0600, Jeff Law <jeffreyalaw@gmail.com> wrote:
>> On 10/5/2021 8:46 AM, Jan-Benedict Glaw wrote:
>>> On Thu, 2021-09-30 21:27:23 +0200, Jan-Benedict Glaw <jbglaw@lug-owl.de> wrote:
>>>> gcc/ChangeLog:
>>>>
>>>> 	* common/config/avr/avr-common.c (avr_handle_option): Mark
>>>> 	argument as ATTRIBUTE_UNUSED.
>>>>
>>>> diff --git a/gcc/common/config/avr/avr-common.c b/gcc/common/config/avr/avr-common.c
>>>> index 6486659d27c..a6939ad03d3 100644
>>>> --- a/gcc/common/config/avr/avr-common.c
>>>> +++ b/gcc/common/config/avr/avr-common.c
>>>> @@ -77,7 +77,8 @@ static const struct default_options avr_option_optimization_table[] =
>>>>    static bool
>>>>    avr_handle_option (struct gcc_options *opts, struct gcc_options*,
>>>> -                   const struct cl_decoded_option *decoded, location_t loc)
>>>> +                   const struct cl_decoded_option *decoded,
>>>> +                   location_t loc ATTRIBUTE_UNUSED)
>>>>    {
>>>>      int value = decoded->value;
>>>>
>>>>
>>>>     Ok for trunk?
>>> Wanted to give this a ping.
>> I thought I sent a reply a few days ago.  Instead of using ATTRIBUTE_UNUSED,
>> just drop the parameter's name.  You should consider that pre-approved for
>> this and any other cases you run into.
> Not quite I think. The `loc`ation parameter is used under some
> circumstances, though it's dependant on some #ifdefs:
Sigh.  We rooted out most of those kinds of conditional compilation from 
the core compiler years ago for precisely these kinds of reasons.  Go 
ahead with the ATTRIBUTE_UNUSED in this case.  But generally dropping 
the argument's name is better.  Extra points if you killed the 
conditional compilation in the avr backend, but I don't think that 
should be a requirement to move forward.

Jeff
  

Patch

diff --git a/gcc/common/config/avr/avr-common.c b/gcc/common/config/avr/avr-common.c
index 6486659d27c..a6939ad03d3 100644
--- a/gcc/common/config/avr/avr-common.c
+++ b/gcc/common/config/avr/avr-common.c
@@ -77,7 +77,8 @@  static const struct default_options avr_option_optimization_table[] =
 
 static bool
 avr_handle_option (struct gcc_options *opts, struct gcc_options*,
-                   const struct cl_decoded_option *decoded, location_t loc)
+                   const struct cl_decoded_option *decoded,
+                   location_t loc ATTRIBUTE_UNUSED)
 {
   int value = decoded->value;