> -----Original Message----- > From: Philippe Mathieu-Daudé <[email protected]> > Sent: Monday, October 9, 2023 1:43 AM > To: Brian Cain <[email protected]>; [email protected] > Cc: [email protected]; [email protected]; > [email protected]; Matheus Bernardino (QUIC) > <[email protected]>; [email protected]; [email protected]; > [email protected]; Marco Liebel (QUIC) <[email protected]>; > [email protected]; Thomas Huth <[email protected]>; Daniel P. > Berrangé <[email protected]> > Subject: Re: [PATCH v2 3/3] target/hexagon: avoid shadowing globals > > WARNING: This email originated from outside of Qualcomm. Please be wary of > any links or attachments, and do not enable macros. > > On 9/10/23 08:09, Philippe Mathieu-Daudé wrote: > > Hi Brian, > > > > On 6/10/23 00:22, Brian Cain wrote: > >> The typedef `vaddr` is shadowed by `vaddr` identifiers, so we rename the > >> identifiers to avoid shadowing the type name. > > > > This one surprises me, since we have other occurences: > > > > include/exec/memory.h:751:bool memory_get_xlat_addr(IOMMUTLBEntry > > *iotlb, void **vaddr, > > include/qemu/plugin.h:199:void qemu_plugin_vcpu_mem_cb(CPUState > > *cpu, uint64_t vaddr, > > target/arm/internals.h:643:G_NORETURN void > > arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, > > target/i386/tcg/helper-tcg.h:70:G_NORETURN void > > handle_unaligned_access(CPUX86State *env, vaddr vaddr, > > ... > > > > $ git grep -w vaddr, | wc -l > > 207 > > > > What is the error/warning like? > > OK I could reproduce, I suppose you are building with Clang which > doesn't support shadow-local so you get global warnings too (as > mentioned in this patch subject...):
No -- I generally build with gcc and only double-check the clang results to make sure I don't see any new failures there. But I've not tested "-Wshadow" with clang yet. I found these by adding "-Wshadow=global" to "-Wshadow=local". I thought it might be useful to address these too while we're here. > In file included from ../../gdbstub/trace.h:1, > from ../../gdbstub/softmmu.c:30: > trace/trace-gdbstub.h: In function '_nocheck__trace_gdbstub_hit_watchpoint': > trace/trace-gdbstub.h:903:106: error: declaration of 'vaddr' shadows a > global declaration [-Werror=shadow] > 903 | static inline void _nocheck__trace_gdbstub_hit_watchpoint(const > char * type, int cpu_gdb_index, uint64_t vaddr) > | > ~~~~~~~~~^~~~~ > In file included from include/sysemu/accel-ops.h:13, > from include/sysemu/cpus.h:4, > from ../../gdbstub/softmmu.c:21: > include/exec/cpu-common.h:21:18: note: shadowed declaration is here > 21 | typedef uint64_t vaddr; > | ^~~~~ > trace/trace-gdbstub.h: In function 'trace_gdbstub_hit_watchpoint': > trace/trace-gdbstub.h:923:96: error: declaration of 'vaddr' shadows a > global declaration [-Werror=shadow] > 923 | static inline void trace_gdbstub_hit_watchpoint(const char * > type, int cpu_gdb_index, uint64_t vaddr) > | > ~~~~~~~~~^~~~~ > include/exec/cpu-common.h:21:18: note: shadowed declaration is here > 21 | typedef uint64_t vaddr; > | ^~~~~ > > Clang users got confused by this, IIUC Markus and Thomas idea is > to only enable these warnings for GCC, enforcing them for Clang > users via CI (until Clang get this option supported). Personally > I'd rather enable the warning once for all, waiting for Clang > support (or clean/enable global shadowing for GCC too). Hopefully it's helpful or at least benign if we address the shadowed globals under target/hexagon/ for now, even if "-Wshadow=global" is not enabled. > See this thread: > https://lore.kernel.org/qemu-devel/11abc551-188e-85c0-fe55- > [email protected]/ > > Regards, > > Phil.
