On Thu, Oct 26, 2023 at 07:58:42AM +0200, 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 <arm...@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' > > 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.
QEMU has never required regular contributors to submit code that compiles perfectly on every supported platform. Only that they make a fair effort to have it compile on their platform, and respond to feedback if a reviewer points out a problem for a different platform. Subsystem maintainers though should be ensuring code is warning free on every platform by running through CI before submitting a pull request. This centralization of CI repsonsibilities on maintainers is one of the downsides of our mailing list workflow, as compared to gitforges where the regular contributors would immediately trigger & see CI reports from every merge request they open. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
