Re: [PATCH v2 3/3] target/hexagon: avoid shadowing globals

[Philippe Mathieu-Daudé]

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...):

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).

See this thread:
11abc551-188e-85c0-fe55-b2b58d35105d@redhat.com/">https://lore.kernel.org/qemu-devel/11abc551-188e-85c0-fe55-b2b58d35105d@redhat.com/

Regards,

Phil.