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

Hi Tom,

thanks for the feedback.  I will follow up with a v3 of the patch.  I have one
question regarding your feedback, please see inline below.

Thanks
Stephan

> -----Original Message-----
> From: Tom Tromey <[email protected]>
> Sent: Friday, 21 August 2026 21:03
> To: Rohr, Stephan <[email protected]>
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH v2 1/1] gdb: Introduce new setting to filter out shadowed
> variables.
> 
> >>>>> "Stephan" == Stephan Rohr <[email protected]> writes:
> 
> Stephan> +* New commands
> Stephan> +
> Stephan> +set print shadowed on|off
> Stephan> +show print shadowed
> Stephan> +This controls the output of the "info locals" command.  If the
> option is 'off'
> Stephan> +shadowed variables will be omitted in output.  The default is to
> print
> Stephan> +shadowed variables.
> 
> Needs a doc review but I think the explanation lines should be indented.
> 
> Stephan> +/* Option for printing shadowed variables.  */
> Stephan> +
> Stephan> +struct shadowed_print_options
> Stephan> +{
> Stephan> +  bool print_shadowed = true;
> 
> I don't really follow why this type is needed.

Do you mean why we need the flag or why it is encapsulated into a new
"shadowed_print_options" type? Or why it is not included in any other
print related options struct?

I removed the "-shadowed" option from the "value_print_options" to
avoid the unnecessary "print -shadowed" option in v2.  The "shadowed"
option is used to store the user's setting for the session and can be overwritten
by the "info locals" command.  Thus, I added a new type here similar to
"maintenance_selftest_options" in maint.c.

I separated this from the "info_print_options" as these are per invocation and I
need to store the "print_shadowed" option for the lifetime of the session.  Also,
the "-shadowed" option is only used by the "info locals" command.

> 
> Stephan>    print_variable_and_value (print_name, sym, frame, stream,
> num_tabs,
> Stephan> -			    shadow_status);
> Stephan> +			    print_shadowed ? shadow_status
> Stephan> +					   : var_shadowing::NONE);
> 
> Lining things up like this isn't really the gdb style; you could break
> before the "?" instead.
> 
> Stephan> +  if (cb_data.printed_shadowed_variables)
> Stephan> +    gdb_printf (stream,
> Stephan> +		_("Use 'set print shadowed off' "
> Stephan> +		  "to hide shadowed variables.\n"));
> Stephan> +  else if (cb_data.omitted_shadowed_variables)
> Stephan> +    gdb_printf (stream,
> Stephan> +		_("Some shadowed variables were omitted, use "
> Stephan> +		  "'set print shadowed on' to include them.\n"));
> 
> I'm slowly trying to transition gdb to using double quotes rather than
> the current mishmash of quoting styles.
> 
> Also command names should be styled using command_style.
> 
> thanks,
> Tom
________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.