Re: [PATCH 1/1] meson: Enable -Wshadow=local

[Philippe Mathieu-Daudé]

On 26/10/23 07:31, Markus Armbruster wrote:
> Local variables shadowing other local variables or parameters make the
> code needlessly hard to understand.  Bugs love to hide in such code.
> Evidence: commit bbde656263d (migration/rdma: Fix save_page method to
> fail on polling error).
>
> Enable -Wshadow=local to prevent such issues.  Possible thanks to
> recent cleanups.  Enabling -Wshadow would prevent more issues, but
> we're not yet ready for that.
>
> As usual, the warning is only enabled when the compiler recognizes it.
> GCC does, Clang doesn't.
>
> Some shadowed locals remain in bsd-user.  Since BSD prefers Clang,
> let's not wait for its cleanup.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   meson.build | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/meson.build b/meson.build
> index dcef8b1e79..89220443b8 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -462,6 +462,7 @@ warn_flags = [
>     '-Wno-tautological-type-limit-compare',
>     '-Wno-psabi',
>     '-Wno-gnu-variable-sized-type-not-at-end',
> +  '-Wshadow=local',
>   ]
>   
>   if targetos != 'darwin'

Using Clang on Darwin:

$ ../configure
The Meson build system
Version: 1.2.1
Build type: native build
Project name: qemu
Project version: 8.1.50
C compiler for the host machine: cc (clang 15.0.0 "Apple clang version 
15.0.0 (clang-1500.0.40.1)")
C linker for the host machine: cc ld64 1015.7
Host machine cpu family: aarch64
Host machine cpu: aarch64
Program sh found: YES (/bin/sh)
Objective-C compiler for the host machine: clang (clang 15.0.0)
Objective-C linker for the host machine: clang ld64 1015.7
Program bzip2 found: YES (/usr/bin/bzip2)
Program iasl found: YES (/opt/homebrew/bin/iasl)
Compiler for C supports arguments -fno-pie: YES
Compiler for C supports arguments -no-pie: NO
Compiler for C supports link arguments -Wl,-z,relro: NO
Compiler for C supports link arguments -Wl,-z,now: NO
Compiler for C supports link arguments -Wl,--warn-common: NO
Compiler for C supports arguments -Wundef: YES
Compiler for C supports arguments -Wwrite-strings: YES
Compiler for C supports arguments -Wmissing-prototypes: YES
Compiler for C supports arguments -Wstrict-prototypes: YES
Compiler for C supports arguments -Wredundant-decls: YES
Compiler for C supports arguments -Wold-style-declaration: NO
Compiler for C supports arguments -Wold-style-definition: YES
Compiler for C supports arguments -Wtype-limits: YES
Compiler for C supports arguments -Wformat-security: YES
Compiler for C supports arguments -Wformat-y2k: YES
Compiler for C supports arguments -Winit-self: YES
Compiler for C supports arguments -Wignored-qualifiers: YES
Compiler for C supports arguments -Wempty-body: YES
Compiler for C supports arguments -Wnested-externs: YES
Compiler for C supports arguments -Wendif-labels: YES
Compiler for C supports arguments -Wexpansion-to-defined: YES
Compiler for C supports arguments -Wimplicit-fallthrough=2: NO
Compiler for C supports arguments -Wmissing-format-attribute: YES
Compiler for C supports arguments -Wno-initializer-overrides: YES
Compiler for C supports arguments -Wno-missing-include-dirs: YES
Compiler for C supports arguments -Wno-shift-negative-value: YES
Compiler for C supports arguments -Wno-string-plus-int: YES
Compiler for C supports arguments -Wno-typedef-redefinition: YES
Compiler for C supports arguments -Wno-tautological-type-limit-compare: YES
Compiler for C supports arguments -Wno-psabi: YES
Compiler for C supports arguments 
-Wno-gnu-variable-sized-type-not-at-end: YES
Compiler for C supports arguments -Wshadow=local: NO
Compiler for Objective-C supports arguments -Wundef: YES
Compiler for Objective-C supports arguments -Wwrite-strings: YES
Compiler for Objective-C supports arguments -Wmissing-prototypes: YES
Compiler for Objective-C supports arguments -Wstrict-prototypes: YES
Compiler for Objective-C supports arguments -Wredundant-decls: YES
Compiler for Objective-C supports arguments -Wold-style-declaration: NO
Compiler for Objective-C supports arguments -Wold-style-definition: YES
Compiler for Objective-C supports arguments -Wtype-limits: YES
Compiler for Objective-C supports arguments -Wformat-security: YES
Compiler for Objective-C supports arguments -Wformat-y2k: YES
Compiler for Objective-C supports arguments -Winit-self: YES
Compiler for Objective-C supports arguments -Wignored-qualifiers: YES
Compiler for Objective-C supports arguments -Wempty-body: YES
Compiler for Objective-C supports arguments -Wnested-externs: YES
Compiler for Objective-C supports arguments -Wendif-labels: YES
Compiler for Objective-C supports arguments -Wexpansion-to-defined: YES
Compiler for Objective-C supports arguments -Wimplicit-fallthrough=2: NO
Compiler for Objective-C supports arguments -Wmissing-format-attribute: YES
Compiler for Objective-C supports arguments -Wno-initializer-overrides: YES
Compiler for Objective-C supports arguments -Wno-missing-include-dirs: YES
Compiler for Objective-C supports arguments -Wno-shift-negative-value: YES
Compiler for Objective-C supports arguments -Wno-string-plus-int: YES
Compiler for Objective-C supports arguments -Wno-typedef-redefinition: YES
Compiler for Objective-C supports arguments 
-Wno-tautological-type-limit-compare: YES
Compiler for Objective-C supports arguments -Wno-psabi: YES
Compiler for Objective-C supports arguments 
-Wno-gnu-variable-sized-type-not-at-end: YES
Compiler for Objective-C supports arguments -Wshadow=local: NO

So:

Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Now don't blame me for posting patches with trigger shadow=local
warnings because I am not testing that locally.

I find it a bit unfair to force me rely on CI or other machines
rather than my host machine to check for warnings. I'd have
rather waited this option support lands first in Clang before
enabling this flag.

Regards,

Phil.