RE: [PATCH v14 1/2] gdb: add annotation in 'info locals' command for variables shadowing case

["Ijaz, Abdul B" <abdul.b.ijaz@intel.com>]

Thanks Andrew for the feedback. I will fix them as per the feedback before mering.

>> +
>> +

Andrew>Double blank line here, there should be only one.
Abdul>Will remove one blank line.

>> +method, or inner class).  For such cases, its location for the 
>> +outermost scope is followed by @samp{shadowed}.  The location can 
>> +help to locate the instances of shadowed variables.  So, location 
>> +information is only added for shadowed variables.

Andrew>I found the second sentence of this paragraph a little hard to parse.
Andrew>Also the justification for the location seems unnecessary (to me).  How about this instead:

Abdul>Will use the description from your suggestion.

>> +typedef gdb::function_view<void (const char *print_name, struct symbol *sym,
>> +				 var_shadowing shadow_status)>
>> +    iterate_over_block_arg_local_vars_cb_printing;
>> +

Abdrew>After the recent commit:

Abdrew>  commit ef5907287aec1dc7f5323c66042a4a26f275c5cf
Abdrew>  Date:   Mon Jul 13 15:06:35 2026 +0200

Abdrew>    [gdb] Convert template typedefs to using

Abdrew>the previous 'typedef' has been updated to 'using'.  Could you update this to match please.

Abdul> Will fix it before merging.

Best Regards,
Abdul Basit


-----Original Message-----
From: Andrew Burgess <aburgess@redhat.com> 
Sent: Friday, July 17, 2026 2:57 PM
To: Ijaz, Abdul B <abdul.b.ijaz@intel.com>; gdb-patches@sourceware.org
Cc: pedro@palves.net; philippe.waroquiers@skynet.be; Schimpe, Christina <christina.schimpe@intel.com>; lsix@lancelotsix.com; eliz@gnu.org; Ijaz, Abdul B <abdul.b.ijaz@intel.com>; guinevere@redhat.com
Subject: Re: [PATCH v14 1/2] gdb: add annotation in 'info locals' command for variables shadowing case


Hi, I have a last couple of super minor nits, see inline below.  If you're happy to fix these as I suggest then I don't think you need to post another version of the patch:

Approved-By: Andrew Burgess <aburgess@redhat.com>

But please wait for approval on patch #2 before merging both together.

Thanks,
Andrew

Abdul Basit Ijaz <abdul.b.ijaz@intel.com> writes:

> From: "Ijaz, Abdul B" <abdul.b.ijaz@intel.com>
>
> For C/C++/Fortran/Ada languages GDB prints same name variable multiple 
> times in case of variable shadowing and it is confusing for user to 
> identify which variable belongs to the current scope.  So for such 
> cases add location info to the innermost listed variables and for 
> super block variables add "shadowed" annotation in the form of "<file.c:line, shadowed>".
>
> Suppose we have
>
> 1:int x = 3;
> 2:  {
> 3:    int x = 4;
> 4:    int y = 52;
> 5:    x = 99; /* break here */
> 6:  }
>
> Currently:
>
> (gdb) info locals
> x = 4
> y = 52
> x = 3
>
> After applying this patch, we obtain:
>
> (gdb) info locals
> x = 4  <file.c:3>
> y = 52
> x = 3  <file.c:1, shadowed>
>
> The patch adds the location annotations by keeping track of inner 
> block and already printed variables to identify shadowing.  So, GDB 
> now prints "<file.c:line, shadowed>" for shadowed super-block 
> variables and "<file.c:line>" for innermost declarations of such variables only.
>
> The location annotations are printed for shadowed variables in case of 
> C/C++/Fortran/Ada languages.  In Rust, it is possible to declare a 
> variable with the same name many times.  So in this case, just the 
> first instance of the variable is printed.  RUST language test "var_reuse.exp"
> fails with rustc compiler version >= 1.73 so XFAIL is added accordingly.
>
> Fix regex expression in the gdb.opt/inline-locals.exp test according 
> to this change.  The test update is only required due to the existing 
> gdb known ticket gdb/25695 where this issue is seen with 7.5.0 version 
> on
> sles15sp6 but it is not seen anymore on the newer gcc versions e.g.
> gcc-11.4.0.
>
> The symtab()/filename() nullptr check was added specifically to avoid 
> the crash seen in gdb.dwarf2/missing-type-name-for-templates.exp where 
> template symbols may have no associated source file.
>
> Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
> ---
>  gdb/NEWS                                      |   5 +
>  gdb/c-typeprint.c                             |   3 +-
>  gdb/doc/gdb.texinfo                           |  26 ++++
>  gdb/language.c                                |  20 +++
>  gdb/language.h                                |  18 +++
>  gdb/printcmd.c                                |  44 ++++++-
>  gdb/stack.c                                   | 115 ++++++++++++++++--
>  gdb/stack.h                                   |  22 ++++
>  gdb/testsuite/gdb.ada/var_shadowing.exp       |  39 ++++++
>  .../gdb.ada/var_shadowing/var_shadowing.adb   |  30 +++++
>  gdb/testsuite/gdb.base/var-shadowing.c        |  51 ++++++++
>  gdb/testsuite/gdb.base/var-shadowing.exp      |  92 ++++++++++++++
>  gdb/testsuite/gdb.base/var-shadowing2.c       |  16 +++
>  gdb/testsuite/gdb.opt/inline-locals.exp       |  21 ++--
>  gdb/testsuite/gdb.rust/var_reuse.exp          |  36 ++++++
>  gdb/testsuite/gdb.rust/var_reuse.rs           |  20 +++
>  gdb/value.h                                   |  26 +++-
>  17 files changed, 561 insertions(+), 23 deletions(-)  create mode 
> 100644 gdb/testsuite/gdb.ada/var_shadowing.exp
>  create mode 100644 
> gdb/testsuite/gdb.ada/var_shadowing/var_shadowing.adb
>  create mode 100755 gdb/testsuite/gdb.base/var-shadowing.c
>  create mode 100755 gdb/testsuite/gdb.base/var-shadowing.exp
>  create mode 100644 gdb/testsuite/gdb.base/var-shadowing2.c
>  create mode 100755 gdb/testsuite/gdb.rust/var_reuse.exp
>  create mode 100755 gdb/testsuite/gdb.rust/var_reuse.rs
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index ec9b5a33787..2c8adf05b43 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -201,6 +201,11 @@ New command class for help
>    commands that we, as developers, believe would be close to a minimal
>    set of commands for a new user of GDB.
>  
> +info locals
> +  GDB now shows the "shadowed" annotation and the location 
> +information
> +  for variables that are shadowed, or which are shadowing.
> +
> +

Double blank line here, there should be only one.

>  * Removed commands
>  
>  target ctf

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 
> a698b2b8451..ee33bfdba13 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -9188,6 +9188,32 @@ The optional flag @samp{-q}, which stands for 
> @samp{quiet}, disables  printing header information and messages 
> explaining why no local variables  have been printed.
>  
> +@smallexample
> +@group
> +1: int x = 3;
> +2: @{
> +3:       int x = 4;
> +4:       int y = 52;
> +5:       x = 99; // breakpoint-line
> +6: @}
> +@end group
> +@group
> +(gdb) info locals
> +x = 4	<file.c:3>
> +y = 52
> +x = 3	<file.c:1, shadowed>
> +@end group
> +@end smallexample
> +
> +@anchor{shadowed variables}
> +@cindex shadowed variables
> +A variable is @dfn{shadowed} when there's another variable with the 
> +same name which is declared within an inner scope (decision block, 
> +method, or inner class).  For such cases, its location for the 
> +outermost scope is followed by @samp{shadowed}.  The location can 
> +help to locate the instances of shadowed variables.  So, location 
> +information is only added for shadowed variables.

I found the second sentence of this paragraph a little hard to parse.
Also the justification for the location seems unnecessary (to me).  How about this instead:

  A variable is @dfn{shadowed} when there's another variable with the
  same name which is declared within an inner scope (decision block,
  method, or inner class).  When shadowing is detected, location
  information is added to all instances of the shadowed variable name.
  The outermost instances are additionally followed by @samp{shadowed}
  to indicate that they are not the active variable.

> +
>  @item info locals [-q] [-t @var{type_regexp}] [@var{regexp}]  Like 
> @kbd{info locals}, but only print the local variables selected  with 
> the provided regexp(s).


> diff --git a/gdb/stack.h b/gdb/stack.h index ad2700b59a7..a03103b9847 
> 100644
> --- a/gdb/stack.h
> +++ b/gdb/stack.h
> @@ -20,6 +20,8 @@
>  #ifndef GDB_STACK_H
>  #define GDB_STACK_H
>  
> +enum class var_shadowing;
> +
>  gdb::unique_xmalloc_ptr<char> find_frame_funname (const frame_info_ptr &frame,
>  						  enum language *funlang,
>  						  struct symbol **funcp);
> @@ -27,12 +29,32 @@ gdb::unique_xmalloc_ptr<char> find_frame_funname 
> (const frame_info_ptr &frame,  typedef gdb::function_view<void (const char *print_name, struct symbol *sym)>
>       iterate_over_block_arg_local_vars_cb;
>  
> +typedef gdb::function_view<void (const char *print_name, struct symbol *sym,
> +				 var_shadowing shadow_status)>
> +    iterate_over_block_arg_local_vars_cb_printing;
> +

After the recent commit:

  commit ef5907287aec1dc7f5323c66042a4a26f275c5cf
  Date:   Mon Jul 13 15:06:35 2026 +0200

    [gdb] Convert template typedefs to using

the previous 'typedef' has been updated to 'using'.  Could you update this to match please.

>  void iterate_over_block_arg_vars (const struct block *block,
>  				  iterate_over_block_arg_local_vars_cb cb);
>  
>  void iterate_over_block_local_vars (const struct block *block,
>  				    iterate_over_block_arg_local_vars_cb cb);
>  

Intel Deutschland GmbH

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
Tel: +49 89 991 430, www.intel.de
Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
Chairperson of the Supervisory Board: Nicole Lau
Registered Seat: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928