Re: [edk2-devel] [PATCH] DebugLib: Allow -Wunneeded-internal-declaration with clang

["Ard Biesheuvel" <ardb@kernel.org>]

On Tue, 12 Dec 2023 at 09:49, Mike Beaton <mjsbeaton@gmail.com> wrote:
>
> From: Mike Beaton <mjsbeaton@gmail.com>
>
> Provides a variant of the DEBUG macro for clang when MDEPKG_NDEBUG is defined,
> which uses but discards the contained expression, this means clang can tell
> that it has optimised away variable usage, therefore we can keep
> -Wunneeded-internal-declaration (as part of -Wall) to warn about any
> mistakenly genuinely unused variables.
>
> Signed-off-by: Mike Beaton <mjsbeaton@gmail.com>

You failed to mention where you found this patch.

> ---
>  BaseTools/Conf/tools_def.template |  2 +-
>  MdePkg/Include/Library/DebugLib.h | 10 ++++++++++
>  2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
> index c34ecfd557..eaccf0b698 100755
> --- a/BaseTools/Conf/tools_def.template
> +++ b/BaseTools/Conf/tools_def.template
> @@ -1859,7 +1859,7 @@ DEFINE CLANGDWARF_X64_DLINK2_FLAGS        = -Wl,--defsym=PECOFF_HEADER_SIZE=0x22
>  DEFINE CLANGDWARF_IA32_TARGET             = -target i686-pc-linux-gnu
>  DEFINE CLANGDWARF_X64_TARGET              = -target x86_64-pc-linux-gnu
>
> -DEFINE CLANGDWARF_WARNING_OVERRIDES    = -Wno-parentheses-equality -Wno-empty-body -Wno-unused-const-variable -Wno-varargs -Wno-unknown-warning-option -Wno-unused-but-set-variable -Wno-unused-const-variable -Wno-unaligned-access -Wno-unneeded-internal-declaration
> +DEFINE CLANGDWARF_WARNING_OVERRIDES    = -Wno-parentheses-equality -Wno-empty-body -Wno-unused-const-variable -Wno-varargs -Wno-unknown-warning-option -Wno-unused-but-set-variable -Wno-unused-const-variable -Wno-unaligned-access
>  DEFINE CLANGDWARF_ALL_CC_FLAGS         = DEF(GCC48_ALL_CC_FLAGS) DEF(CLANGDWARF_WARNING_OVERRIDES) -fno-stack-protector -mms-bitfields -Wno-address -Wno-shift-negative-value -Wno-unknown-pragmas -Wno-incompatible-library-redeclaration -fno-asynchronous-unwind-tables -mno-sse -mno-mmx -msoft-float -mno-implicit-float  -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -funsigned-char -fno-ms-extensions -Wno-null-dereference
>
>  ###########################
> diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h
> index f0c9f64487..e2158b1a3d 100644
> --- a/MdePkg/Include/Library/DebugLib.h
> +++ b/MdePkg/Include/Library/DebugLib.h
> @@ -425,6 +425,16 @@ UnitTestDebugAssert (
>          _DEBUG (Expression);       \
>        }                            \
>      } while (FALSE)
> +#elif defined (__clang__)
> +#define DEBUG(Expression)                                \
> +    do {                                                   \
> +      _Pragma("GCC diagnostic push")                       \
> +      _Pragma("GCC diagnostic ignored \"-Wunused-value\"") \

This seems redundant to me. Either we set the pragma and the compiler
does not care, or we don't, and rely on the fact that the compiler can
infer that 'Expression' will never be evaluated at runtime, but won't
complain about symbols that are only referenced via 'Expression' and
nowhere else.


> +      if ((FALSE)) {                                       \
> +        (VOID) Expression;                                 \
> +      }                                                    \
> +      _Pragma("GCC diagnostic pop")                        \
> +    } while (FALSE)
>  #else
>  #define DEBUG(Expression)
>  #endif
> --
> 2.39.2
>
>
>
> ------------
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#112363): https://edk2.groups.io/g/devel/message/112363
> Mute This Topic: https://groups.io/mt/103126777/5717338
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ardb+tianocore@kernel.org]
> ------------
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112366): https://edk2.groups.io/g/devel/message/112366
Mute This Topic: https://groups.io/mt/103126777/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-