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

[Thomas Huth <thuth@redhat.com>]

On 26/10/2023 07.58, Philippe Mathieu-Daudé wrote:
> 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.

Huh, that situation is already pre-existing, e.g. with 
-Wimplicit-fallthrough=2 ... and if you're too afraid, you can always 
install gcc via homebrew to check.

  Thomas