Re: [PATCH 1/1] gdb: Introduce new setting to filter out shadowed variables.

On 8/11/26 4:27 PM, Stephan Rohr wrote:
> From: "Sargsyan, Eduard" <[email protected]>
> 
> Add a new setting 'print shadowed on|off' to control the printing of
> shadowed variables in 'info locals'.
> 
> When shadowed variables are present, each variable involved in a
> shadowing relationship is annotated with its declaration location
> '<file:line>'; the ones hidden by an inner declaration are additionally
> marked ', shadowed'.
> 
> Given this code, stopped at line 10:
> 
>    1 int num = 1;
>    2 int
>    3 main ()
>    4 {
>    5   const char *str = "main";
>    6   int num = 3;
>    7   {
>    8     const char *str = "nested";
>    9     int num = 5;
>   10     num = 0; //  break here
>   11   }
>   12   return num;
>   13 }
> 
> By default, 'info locals' prints shadowed variables:
> 
>   (gdb) info locals
>   str = 0x555555556009 "nested"  <main.c:8>
>   num = 5  <main.c:9>
>   str = 0x555555556004 "main"  <main.c:5, shadowed>
>   num = 3  <main.c:6, shadowed>
>   Use 'set print shadowed off' to hide shadowed variables.
> 
> The user may not want to print shadowed variables:
> 
>   (gdb) set print shadowed off
>   (gdb) info locals
>   str = 0x555555556009 "nested"
>   num = 5
>   Use 'set print shadowed on' to include shadowed variables.
> ---
>  gdb/NEWS                                 |  8 ++++++++
>  gdb/doc/gdb.texinfo                      | 15 +++++++++++++-
>  gdb/printcmd.c                           |  8 +++++++-
>  gdb/stack.c                              | 25 +++++++++++++++++++++++-
>  gdb/testsuite/gdb.ada/var_shadowing.exp  |  1 +
>  gdb/testsuite/gdb.base/options.exp       |  1 +
>  gdb/testsuite/gdb.base/var-shadowing.exp | 15 ++++++++++++++
>  gdb/valprint.c                           | 20 +++++++++++++++++++
>  gdb/valprint.h                           |  3 +++
>  9 files changed, 93 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 10c182067f9..a3348436984 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -3,6 +3,14 @@
>  
>  *** Changes since GDB 18
>  
> +* New commands
> +
> +set print shadowed on|off
> +show print shadowed
> +  This controls the output of the "info locals" command for C/C++/Fortran.  If
> +  the option is 'off' shadowed variables will be omitted in output.  The
> +  default is to print shadowed variables.
> +
>  *** Changes in GDB 18
>  
>  * Support for the Common Trace Format (CTF) has been removed.  GDB now
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 0030698dcee..232a9901a2c 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -9212,7 +9212,8 @@ 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.
> +to indicate that they are not the active variable.  Printing shadowed
> +variables can be controlled by @ref{set print shadowed}.
>  
>  @item info locals [-q] [-t @var{type_regexp}] [@var{regexp}]
>  Like @kbd{info locals}, but only print the local variables selected
> @@ -12829,6 +12830,18 @@ Do not pretty print C@t{++} virtual function tables.
>  
>  @item show print vtbl
>  Show whether C@t{++} virtual function tables are pretty printed, or not.
> +
> +@anchor{set print shadowed}
> +@item set print shadowed
> +@itemx set print shadowed on
> +@cindex shadowed variables in C/C@t{++}/Fortran
> +Print shadowed variables for C/C@t{++}/Fortran.  The default is on.
> +
> +@item set print shadowed off
> +Do not print shadowed variables for C/C@t{++}/Fortran.
> +
> +@item show print shadowed
> +Show whether C/C@t{++}/Fortran shadowed variables are printed or not.
>  @end table

Is there a reason you specify C/C++/Fortran everywhere?  Apparently, Ada
supports shadowing too (although I haven't verified).  But in any case,
I don't think it's necessary to specify the languages everywhere like
this.  If you want to mention the languages that support it, then
perhaps one mention in the section about shadowed variables should be
enough (but even then, I feel like it's bound to become outdated at some
point).

> diff --git a/gdb/stack.c b/gdb/stack.c
> index 954ef6b11a4..cd21aa3a40c 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -2258,6 +2258,7 @@ struct print_variable_and_value_data
>    int num_tabs;
>    struct ui_file *stream;
>    int values_printed;
> +  bool has_shadowed_variables;

I think it would be good to initialize the field directly in here,
instead of doing it at the call site.  I don't know if it matters, but
print_frame_arg_vars uses this type without initializing the field.  It
would just be simpler and safer to do it here.



>  
>    void operator() (const char *print_name, struct symbol *sym,
>  		   var_shadowing shadow_status);
> @@ -2292,6 +2293,8 @@ print_variable_and_value_data::operator() (const char *print_name,
>  			    shadow_status);
>  
>    values_printed = 1;

Claude found this corner case:


  int main (void) {
    double x = 1.5;
    { int x = 2; x = x + 1; /* bp */ return x; }
  }

  (gdb) set print shadowed off
  (gdb) info locals -t double
  Use 'set print shadowed on' to include shadowed variables.

What do we want to print here?  I think it's a case where it would
probably make sense to print the "No matching locals" message, but also
the "Use 'set print shadowed on'..." message to indicate that some
variables were hidden due to shadowing.

The point it made was that here, values_printed is set even if the call
to print_variable_and_value above didn't actually print a variable.


> @@ -2360,7 +2364,26 @@ print_frame_local_vars (const frame_info_ptr &frame,
>  
>    iterate_over_block_local_vars_printing (block, cb_data);
>  
> -  if (!cb_data.values_printed && !quiet)
> +  if (quiet)
> +    return;
> +
> +  if (cb_data.values_printed)
> +    {
> +      if (!cb_data.has_shadowed_variables)
> +	return;
> +
> +      value_print_options opts;
> +      get_user_print_options (&opts);
> +      if (opts.print_shadowed)
> +	gdb_printf (stream,
> +		    _("Use 'set print shadowed off' "
> +		      "to hide shadowed variables.\n"));
> +      else
> +	gdb_printf (stream,
> +		    _("Use 'set print shadowed on' "
> +		      "to include shadowed variables.\n"));

I would find it a bit heavy to see this message printed every time, but
at least having shadowed variables is not that common.  And the positive
side is that it makes the setting discoverable.

Nit about the message: if there are some shadowed variables printed,
it's quite clear why you would see the message "Use '...' to hide
shadowed variables".  However, if they were hidden, it's not
obvious why GDB prints "Use '...' to include shadowed variable".  The
user has to infer that GDB prints this because there are hidden shadowed
variable.  I think the message should be more direct, saying something
like "Some shadowed variables were omitted, use '...'  to include them".

Printing this here means that it will be visible in `bt -full`, like
this:

(gdb) bt -full
#0  func () at test.c:9
        x = 4   <test.c:8>
        x = 2   <test.c:5, shadowed>
        y = 3
Use 'set print shadowed off' to hide shadowed variables.
#1  0x000055555555517b in main () at test.c:28
        str = 0x555555556009 "nested"   <test.c:24>
        num = 0 <test.c:25>
        str = 0x555555556004 "main"     <test.c:21, shadowed>
        num = 3 <test.c:22, shadowed>
Use 'set print shadowed off' to hide shadowed variables.

Just wondering if you expected it to work like this or not.

> diff --git a/gdb/testsuite/gdb.base/options.exp b/gdb/testsuite/gdb.base/options.exp
> index 35487ead6a7..34e4b642181 100644
> --- a/gdb/testsuite/gdb.base/options.exp
> +++ b/gdb/testsuite/gdb.base/options.exp
> @@ -198,6 +198,7 @@ proc_with_prefix test-print {{prefix ""}} {
>  	"-pretty"
>  	"-raw-values"
>  	"-repeats"
> +	"-shadowed"
>  	"-static-members"
>  	"-symbol"
>  	"-union"

This looks wrong, I don't think the print command accepts a "-shadowed"
option, so it shouldn't exist in the completion.

On the other hand, it would be nice for "info locals" to accept
"-shadowed on|off".

Simon