On 9/10/23 22:53, Brian Cain wrote:
On 9/10/23 08:09, Philippe Mathieu-Daudé wrote:
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;
| ^~~~~
If we have to clean that for -Wshadow=global, I'm tempted to rename
the typedef as 'vaddr_t' and keep the 'vaddr' variable names.
Richard, Anton, what do you think?
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.
