> -----Original Message----- > From: Philippe Mathieu-Daudé <phi...@linaro.org> > Sent: Monday, October 9, 2023 1:43 AM > To: Brian Cain <bc...@quicinc.com>; qemu-devel@nongnu.org > Cc: arm...@redhat.com; richard.hender...@linaro.org; > peter.mayd...@linaro.org; Matheus Bernardino (QUIC) > <quic_mathb...@quicinc.com>; stefa...@redhat.com; a...@rev.ng; > a...@rev.ng; Marco Liebel (QUIC) <quic_mlie...@quicinc.com>; > ltaylorsimp...@gmail.com; Thomas Huth <th...@redhat.com>; Daniel P. > Berrangé <berra...@redhat.com> > 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- > b2b58d351...@redhat.com/ > > Regards, > > Phil.
