Hello,

I am curious what value enabling the warning for an unused argument adds.  Can 
someone provide some examples where enabling this warning and adding the UNUSED 
tag addresses real issues?  I am concerned that we are enabling a warning that 
would impose a large number of source changes with little or no value.  I can 
be convinced otherwise, but would like to see some supporting data.

Also, what are the requirements for maintenance of code once if this new UNUSED 
macro is used?  We have function implementations where an argument may not be 
used today, but a bug fix/feature enhancement later may use the UNUSED 
argument.  Do we get a different warning because it was tagged as UNUSED, but 
it is actually used now? Or is that silent?

I think there are a few categories of functions to consider here:

1) Protocol/PPI functions.  The parameters for functions are defined for a wide 
usage sometimes with optional behavior.  Specific implementations may choose to 
not implement some optional behavior, and hence may not require some of the 
parameters.  I do not think a warning or an error should be generated by a 
conformant implementation that does not use a parameter.

2) Library Class functions.  The parameters for functions are also defined for 
a wide usage sometimes with optional behavior.  Specific implementations may 
choose to not implement optional behavior, and hence may not require some of 
the parameters.  I do not think a warning or an error should be generated by a 
specific library instance that does not use a parameter in a public library 
class API.

3) Internal functions.  If an internal function does not use a parameter, then 
remove the parameter from the function call.

Can we enable /Wall and disable this specific warning?  Or is there a concern 
that category (3) cannot be detected without enabling this warning?  Maybe it 
would be better if we could find a way to disable this warning for all APIs 
that are decorated with EFIAPI?

Thanks,

Mike

> -----Original Message-----
> From: Leif Lindholm [mailto:[email protected]]
> Sent: Thursday, March 17, 2016 8:53 AM
> To: [email protected]
> Cc: Kinney, Michael D <[email protected]>; Gao, Liming
> <[email protected]>
> Subject: [PATCH] MdePkg: add UNUSED notation to Base.h
> 
> To permit many existing platforms to build with -Wunused-parameter, on
> GCC and CLANG, the unused parameters need to be annotated as such.
> Existing regexp code already uses ARG_UNUSED for this, but it is really
> needed across the codebase - so add a version called UNUSED in Base.h.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Leif Lindholm <[email protected]>
> ---
> 
> Change since RFC: rename ARG_UNUSED -> UNUSED.
> 
>  MdePkg/Include/Base.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
> index 89b2aed..a68b209 100644
> --- a/MdePkg/Include/Base.h
> +++ b/MdePkg/Include/Base.h
> @@ -189,6 +189,15 @@ struct _LIST_ENTRY {
>  ///
>  #define OPTIONAL
> 
> +///
> +/// Function argument intentionally unused in function.
> +///
> +#if defined(__GNUC__) || defined(__clang__)
> +  #define UNUSED  __attribute__ ((unused))
> +#else
> +  #define UNUSED
> +#endif
> +
>  //
>  //  UEFI specification claims 1 and 0. We are concerned about the
>  //  complier portability so we did it this way.
> --
> 2.1.4

_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to